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:

> Tags: patch
>
> Here is a patch with the latest version.
>
> I'll run the unit test now.
>
>> From b4248461e88241cb81f64ea6c073dad82df8b48c Mon Sep 17 00:00:00 2001
> From: Emanuel Berg <[email protected]>
> Date: Tue, 23 Jan 2024 14:21:49 +0100
> Subject: [PATCH] Make erc-cmd-AMSG session local; add /GMSG, /AME and /GME
>
> * lisp/erc/erc.el (erc-cmd-AMSG): Make it consistent with the doc
> string by only affecting the current connection.
> (erc-cmd-GMSG, erc-cmd-AME, erc-cmd-GME): new IRC slash commands
> (Bug#68401)
> ---
>  lisp/erc/erc.el | 44 +++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 37 insertions(+), 7 deletions(-)
>
> diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
> index 767a693a52e..e89733e7871 100644
> --- a/lisp/erc/erc.el
> +++ b/lisp/erc/erc.el
> @@ -4004,16 +4004,46 @@ 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: ")
> -  (setq line (erc-trim-string line))
> +(defun erc--connected-and-joined-p ()
> +  (and (erc--current-buffer-joined-p)
> +       erc-server-connected))

I took your initial remarks advocating for a DRY helper and, in
particular, this bit about error-proneness

  Okay, but instead of having these checks embedded and
  hopefully correctly repeated four times, shouldn't we ...

to mean you wanted to factor out more than just a predicate, e.g., with
something like

  (defun erc--run-in-joined-buffers (line-function line &optional proc)
    (erc-with-all-buffers-of-server proc #'erc--connected-and-joined-p
      (funcall line-function line)))

(Not that I think such a thing is needed.)

> +
> +(defun erc-cmd-GMSG (line)
> +  "Send LINE to all channels on all networks you are on."
> +  (setq line (string-remove-prefix " " line))
>    (erc-with-all-buffers-of-server nil
> -    (lambda ()
> -      (erc-channel-p (erc-default-target)))
> -    (erc-send-message line)))
> +      (lambda () (erc-channel-p (erc-default-target)))
> +    (when (erc--connected-and-joined-p)
> +      (erc-send-message line))))
> +(put 'erc-cmd-GMSG 'do-not-parse-args t)

I don't understand the point of adding your purpose-built predicate
`erc--connected-and-joined-p' if you're not going to use it to replace

  (lambda () (erc-channel-p (erc-default-target)))

completely. IOW, having both is redundant. If you're not already doing
so, please try expanding your macro use sites to verify correctness.

> +
> +(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 (string-remove-prefix " " line))
> +  (erc-with-all-buffers-of-server erc-server-process
> +      (lambda () (erc-channel-p (erc-default-target)))
> +    (when (erc--connected-and-joined-p)
> +      (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."
> +  (erc-with-all-buffers-of-server nil
> +      (lambda () (erc-channel-p (erc-default-target)))
> +    (when (erc--connected-and-joined-p)
> +      (erc-cmd-ME line))))
> +(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."
> +  (erc-with-all-buffers-of-server erc-server-process
> +      (lambda () (erc-channel-p (erc-default-target)))
> +    (when (erc--connected-and-joined-p)
> +      (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