Emanuel Berg writes: >> From fa8ae9dcc306d16cccdd6aa7c2bac242b90adbdb Mon Sep 17 00:00:00 2001 > From: Emanuel Berg > Date: Sat, 13 Jan 2024 03:40:05 +0100 > Subject: [PATCH] Functions for ERC. Emacs doesn't seem to be very picky about a commit's subject line, but I'd prefer those for ERC that aren't mechanical or administrative in nature to be somewhat unique and distinguishable at a glance, such as Make erc-cmd-AMSG session-local, add /GMSG /AME /GME > `erc-cmd-GMSG', `erc-cmd-GME' and `erc-cmd-AME' was added. > `erc-cmd-AMSG' was changed so that it does what the docstring says. > * lisp/erc/erc.el: functions were added/modified to/in this file. > > (bug#68401) I believe the guidelines call for a commit body to be formatted as a change log entry, for example: * 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) > --- > lisp/erc/erc.el | 38 +++++++++++++++++++++++++++++++++----- > 1 file changed, 33 insertions(+), 5 deletions(-) > > diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el > index 478683a77f5..aeb7722b563 100644 > --- a/lisp/erc/erc.el > +++ b/lisp/erc/erc.el > @@ -4016,16 +4016,44 @@ erc--split-string-shell-cmd > ;; Input commands handlers > ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; > > -(defun erc-cmd-AMSG (line) > - "Send LINE to all channels of the current server that you are on." > - (interactive "sSend to all channels you're on: ") > +(defun erc-cmd-GMSG (line) > + "Send LINE to all channels on all networks you are on. > +Interactively, prompt for the line of text to send." > + (interactive "sSend to all channels: ") 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 being less faithful (or whatever) to the traditional IRC experience than /FOO . 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). Normally, when a user submits chat input at the prompt, ERC engages in a series of validation checks before pushing a message out the door. These steps are bypassed when someone invokes what's normally a slash command via M-x. For example, if you /DISCONNECT and issue an /AMSG at the prompt, you'll see "Process not running" in the echo area, and the input will remain there for further editing, killing, etc. However, if you run M-x erc-cmd-AMSG , the message will be inserted in all target buffers, even though nothing is actually sent, which is misleading. Obviously, we can't make `erc-cmd-AMSG' non-interactive because it's been `commandp' forever. But new related commands don't have to follow its (IMO flawed) example. As far as counterarguments go, the only one that comes to mind for making these `commandp' is that doing so also makes managing interactive menus for modules like `bufbar', `nickbar', and `button' easier. For example, at first glance, making `erc-cmd-KICK' interactive would appear to streamline its inclusion in `erc-nick-popup-alist' and obviate the need for an `erc-button-cmd-KICK'. However, if you look closely at this arrangement, you'll see that even if `erc-cmd-KICK' were made a proper Emacs command, a button-specific wrapper would still be necessary because it makes special accommodations for the potential lack of a channel context from which to draw membership rolls for completion. Such a thing isn't necessary when issuing a /KICK at the prompt because the function `pcomplete/erc-mode/KICK' knows it's already running in a channel. > (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. > (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.) > +(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" > (erc-send-message line))) > (put 'erc-cmd-AMSG 'do-not-parse-args t) > > +(defun erc-cmd-GME (line) > + "Send LINE as an action to all channels on all networks you are on. > +Interactively, prompt for the line of text to send." > + (interactive "sSend action to all channels: ") This command currently fails when invoked interactively. For example, if I run M-x erc-cmd-GME hi from any ERC buffer belonging to a connected session, nothing appears in the server logs or any ERC buffer. This needs addressing if you're intent on keeping these interactive, which I'm rather against for reasons previously noted. > + (erc-with-all-buffers-of-server nil > + (lambda () (erc-channel-p (erc-default-target))) > + (erc-cmd-ME line) )) This currently suffers from the same problem as /GMSG regarding disconnected buffers. However you address this, it's probably best to use the same approach for fixing both functions. > +(put 'erc-cmd-GME 'do-not-parse-args t) > + > +(defun erc-cmd-AME (line) > + "Send LINE as an action to all channels on the current network. > +Interactively, prompt for the line of text to send." > + (interactive "sSend action to all channels on this network: ") This command also appears do to nothing when invoked via M-x. > + (erc-with-all-buffers-of-server erc-server-process > + (lambda () (erc-channel-p (erc-default-target))) ^ Indentation again. > + (erc-cmd-ME line) )) > +(put 'erc-cmd-AME 'do-not-parse-args t) > + > (defun erc-cmd-SAY (line) > "Send LINE to the current query or channel as a message, not a command. > > -- > 2.39.2 The attached patch is a unit test for all four commands. It doesn't bother asserting M-x behavior (because see above). Please try to make it pass without changing the test itself (unless there's a bug). $ git am /tmp/0002-5.x-Add...etc.patch $ make -C test lisp/erc/erc-scenarios-misc-commands.log \ SELECTOR=erc-scenarios-misc-commands--AMSG-GMSG-AME-GME