Re: bug#68401: 30.0.50; ERC 5.6-git: `erc-cmd-GMSG', `erc-cmd-AMSG', `erc-cmd-GME', `erc-cmd-AME'. 2nd attempt

Emanuel Berg <[email protected]> writes:

>>>>> It might be nice to remove at most one space, for cases
>>>>> where a user wants to send preformatted text. OTOH,
>>>>> normal /MSG doesn't do this, so perhaps we shouldn't
>>>>> here either.
>>>>
>>>> Again, this is in the original `erc-cmd-AMSG'. I have no
>>>> opinion, so you can decide it.
>>>>
>>>> "At most one space", what space should that be?
>>>> Leading or trailing?
>>>
>>> Leading. See the test for `erc-extract-command-from-line'
>>> to understand the behavior of `do-not-parse-args', which
>>> determines LINE. Actually, if we're doing away with
>>> `commandp', there should be no reason for "at most one,"
>>> only "exactly one" (IIRC).
>>
>> So if and only if the initial char is a whitespace, it, and
>> only it, should be dropped. E.g. " line string " should be
>> transformed into " line string ".
>>
>> Also, at this point, only "erc-cmd-AMSG" and "erc-cmd-GMSG"
>> has the trim line, and the reason is it is present in the
>> original `erc-cmd-AMSG'.
>>
>> Should we also have the new one in `erc-cmd-AME' and
>> `erc-cmd-GME'?

Please look at the "source" of `erc-cmd-ME', which these commands both
call, and you will (hopefully) have your answer.

> Andreas Schwab wrote:

> >> (defun erc-drop-leading-whitespace (str)
> >>   (if (string-match " \\(.*\\)" str)
> >>       (match-string 1 str)
> >>     str) )
> >
> > (erc-drop-leading-whitespace "foo bar") => "bar"

> Ah, forgot the ^ and $, thank you.

> (defun erc-drop-leading-whitespace (str)
>   (if (string-match "^ \\(.*\\)$" str)
>       (match-string 1 str)
>     str) )

Why not use `string-prefix-p' and `substring' to accomplish this? Also,
please name this using the internal "--" convention, something like
`erc--drop-initial-space-char' or `erc--drop-single-leading-space'.

>>>>> Without first checking for connectivity, we run into
>>>>> another situation in which messages may be inserted but
>>>>> not sent, similar to the bit about commands being
>>>>> potentially "misleading," above. The most obvious way to
>>>>> solve this is to check for "physical" connectivity with
>>>>> something like:
>>>>>
>>>>>   (erc-with-all-buffers-of-server nil #'erc-server-process-alive
>>>>>     (when (and erc--target (erc--current-buffer-joined-p))
>>>>>       (erc-send-message line))))
>
> If we can drop `erc--target' in the latest Emacs source as you
> say that means there is only this left
>
>   (and (erc-server-process-alive) (erc--current-buffer-joined-p))

Correct.

>>>>> Alternatively, you can check for "logical" connectivity,
>>>>> which is probably more in keeping with traditional design
>>>>> principles:
>>>>>
>>>>>   (erc-with-all-buffers-of-server nil nil
>>>>>     (when (and erc-server-connected erc--target (erc--current-buffer-joined-p))
>>>>>       (erc-send-message line))))
>
> If we again can drop `erc--target' and want
> `erc-server-connected' to be returned upon success we can do
>
>   (and (erc--current-buffer-joined-p) erc-server-connected)

Correct.

> If we unify those two tests it will be
>
> (defun erc-connected-and-joined-p ()
>   (and (erc-server-process-alive)
>        (erc--current-buffer-joined-p)
>        erc-server-connected))

No! Well... it's superfluous and hand wavy, and we ought not conflate
the two unless we're going into the chimera breeding business, in which
case we'd want

  (and (erc--current-buffer-joined-p)
       (or erc-server-connected (erc-server-process-alive)))

Regardless, the confusion here is my fault because I should never have
mentioned `erc-server-process-alive' to begin with. The corner cases in
which

  (when (erc-server-process-alive)
    (cl-assert erc-server-connected))

actually signals are rare, and we can safely ignore them, at least in
this context. Although, it's worth noting that the reverse of

  (when erc-server-connected
    (cl-assert (erc-server-process-alive)))

is an operating invariant that must never signal unless we're tearing
down a session. But let's just drop the `erc-server-process-alive'
business if you don't mind. Also, `erc-connected-and-joined-p' should be
`erc--connected-and-joined-p' unless you can find at least one more
person to +1 exporting it.