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

J.P. wrote:

>>> 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'?

>>>>    (erc-with-all-buffers-of-server nil
>>>> -    (lambda ()
>>>> -      (erc-channel-p (erc-default-target)))
>>>> +    (lambda () (erc-channel-p (erc-default-target)))
>>>> +    (erc-send-message line)))
>>>
>>> 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))))
>>>
>>> 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))))
>>>
>>> One minor downside of this second method is that IRC
>>> adjacent protocols and aberrant proxy servers that happen
>>> to skip 376/422 and also provide some (possibly &local)
>>> "control channel" won't be detected. (BTW, you won't be
>>> needing the `erc--target' in either example if you rebase
>>> atop the latest master.)
>>
>> Okay, but instead of having these checks embedded and
>> hopefully correctly repeated four times, shouldn't we have
>> two functions, say "erc-connected-physical-p" and
>> "erc-connected-logical-p" and call either of those (or
>> both) from the functions?
>
> If you want to factor out a common helper function, fine by
> me. AFAICT such a thing would need to include
> `erc-with-all-buffers-of-server' to be effective unless the
> predicates you've named alone result in meaningful code
> reuse. (Not sure how an `erc-connected-physical-p' would be
> any different than the existing 'erc-server-process-alive',
> though I suppose an `erc-connected-logical-p' could be
> useful if it just returns `erc-server-connected'.)

Okay, I'm on it, stay tuned.

> If you applied the unit-test patch atop your commit, you
> won't be able to "git commit --amend" your previous changes.
> See the "-i, --interactive" option for git-rebase(1), then
> maybe rearrange things so your patch comes *after* the test.
> You can always "git rebase --abort" if you mess up.
> And there's always #git on Libera.

No, I didn't do the unit test, let's agree on the source
first, after that it will be interesting to try it.

-- 
underground experts united
https://dataswamp.org/~incal