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 <incal@dataswamp.org>]

J.P. wrote:

> Make erc-cmd-AMSG session-local, add /GMSG /AME /GME
>
> * lisp/erc/erc.el (erc-cmd-AMSG): Make good on behavior described in
> the doc string by limiting damage to the current connection.
> (erc-cmd-GMSG, erc-cmd-GME, erc-cmd-AME): New functions, all IRC
> "slash commands".  (Bug#68401)

Okay, I'll use that instead. But let's agree on the
source first.

> I question the wisdom of having new slash commands serve
> double duty as interactive Emacs commands (at least those
> handling chat input). This reservation has nothing to do
> with M-x erc-cmd-FOO <RET> being less faithful (or whatever)
> to the traditional IRC experience than /FOO <RET>. Rather,
> it stems from a need to prioritize consistent feedback and
> promote maintainability by only having a single path for
> chat input to reach the server (except under special
> circumstances).

I made them interactive as `erc-cmd-AMSG' is interactive, but
let's remove it from the other three then.

[ As a side note, Emacs has a problem with different
  interfaces doing too much and influencing the behavior of
  their functions. Interfaces should just be different ways of
  setting the formal parameters, after that the exact same
  thing should happen for the same data. ]

>> (setq line (erc-trim-string line))
>
> 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? This is nothing `erc-trim-string' can do BTW. But we
can of course still remove whatever spaces we like.

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

>> +(put 'erc-cmd-GMSG 'do-not-parse-args t)
>> +
>> +(defun erc-cmd-AMSG (line)
>> +  "Send LINE to all channels of the current network.
>> +Interactively, prompt for the line of text to send."
>> +  (interactive "sSend to all channels on this network: ")
>> +  (setq line (erc-trim-string line))
>> +  (erc-with-all-buffers-of-server erc-server-process
>> +    (lambda () (erc-channel-p (erc-default-target)))
>
>          ^ Indentation. This macro is declared "indent 2"

Okay, fixed that.

> rebase

How do you do that, just 'git pull'?

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