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.]

Emanuel Berg <incal@dataswamp.org> writes:

> Tags: patch
>
>> Anyway here is the patch, note that this is different from
>> the one I just posted, it includes a note in etc/ERC-NEWS
>
> Okay, so it doesn't?
>
> Okay, but I have now verified that the patch includes that
> change, even tho it still has the same number and filename.
>
>> From 737777da67f78a50d50b416b3ebba0e343c907aa Mon Sep 17 00:00:00 2001
> From: Emanuel Berg <incal@dataswamp.org>
> Date: Tue, 23 Jan 2024 14:21:49 +0100
> Subject: [PATCH 301/301] 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
                                            ^                     ^
>From CONTRIBUTE:

  Some commenting rules in the GNU coding standards also apply to
  ChangeLog entries: they must be in English, and be complete sentences
  starting with a capital and ending with a period (except the summary
  line should not end in a period).

>
> Fixed bug in erc-cmd-GME
>
> * lisp/erc/erc.el (erc-cmd-GME): should be #'erc--connected-and-joined-p,
> not (erc--connected-and-joined-p)
> (Bug#68401)

This last item appears to describe an incremental patch revision only
known to this discussion thread on the tracker. These log messages are
instead meant to reflect the entirety of the proposed changeset relative
to what's currently on master. However, something like this might still
be helpful in the body of a discussion post (email).

>
> Test and files added
>
> * test/lisp/erc/erc-scenarios-misc-commands.el
> (erc-scenarios-misc-commands--AMSG-GMSG-AME-GME): New test.
> * test/lisp/erc/resources/commands/amsg-barnet.eld: New file.
> * test/lisp/erc/resources/commands/amsg-foonet.eld: New file.
>
> Added a note on `erc-cmd-AMGS' and its three new friends
>
> * etc/ERC-NEWS: Mentioned here.

FWIW, other Emacs commit messages don't seem to have these interspersed
sub-headers, like "Fixed bug in erc-cmd-GME" and "Test and files added".
And based on

  Each entry in a change log describes either an individual change or
  the smallest batch of changes that belong together, also known as a
  changeset. It is a good idea to start the change log entry with a
  header line: a single line that ... [1]

and

  Separate unrelated change log entries with blank lines. Don’t put
  blank lines between individual changes of an entry. [1]

it's my impression that's only done for "unrelated" entries in the same
patch. As it would seem your changes all affect the same (logical) area
of the code base and provide similar functionality as part of the same
feature set, I'd sooner remove them. Still, I do often see somewhat
arbitrary blanks breaking up groups of items belonging to the same
entry, just without their own header lines. So who really knows?

[1] https://www.gnu.org/prep/standards/html_node/Change-Log-Concepts.html
[2] https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html

> ---
>  etc/ERC-NEWS                                  | 10 ++-
>  lisp/erc/erc.el                               | 38 +++++++--
>  test/lisp/erc/erc-scenarios-misc-commands.el  | 84 +++++++++++++++++++
>  .../erc/resources/commands/amsg-barnet.eld    | 52 ++++++++++++
>  .../erc/resources/commands/amsg-foonet.eld    | 52 ++++++++++++
>  5 files changed, 228 insertions(+), 8 deletions(-)
>  create mode 100644 test/lisp/erc/resources/commands/amsg-barnet.eld
>  create mode 100644 test/lisp/erc/resources/commands/amsg-foonet.eld
>
> diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS
> index 1e88500d169..a4ea1573d64 100644
> --- a/etc/ERC-NEWS
> +++ b/etc/ERC-NEWS
> @@ -14,6 +14,12 @@ GNU Emacs since Emacs version 22.1.
>
>  * Changes in ERC 5.6
>  
> +** Made `erc-cmd-AMSG' session local so it only affects the
> +current connection, this is now consistent with its docstring.

>From CONTRIBUTE:

  Try to start each NEWS entry with a sentence that summarizes the entry
  and takes just one line -- this will allow reading NEWS in Outline
  mode after hiding the body of each entry.

Also, bug fixes aren't really announced unless they threaten to cause
widespread churn or introduce potentially debilitating breakage, so you
can probably just leave that out and only mention the new commands.

> +Also, the new IRC slash commands `erc-cmd-GMSG',
> +`erc-cmd-AME', and `erc-cmd-GME' were added and are available
   ^~~~~~~~~~~~^

For etc/*NEWS, I believe they prefer single 'quotes' for literal text
rather than traditional ones with an opening backtick.

> +
>  ** Module 'keep-place' has a more decorative cousin.
>  Remember your place in ERC buffers a bit more easily with the help of
>  a configurable, visible indicator.  Optionally sync the indicator to
> @@ -1367,7 +1373,7 @@ reconnection attempts that ERC will make per server.
>  in seconds, that ERC will wait between successive reconnect attempts.
>  
>  *** erc-server-send-ping-timeout: Determines when to consider a connection
> -stalled and restart it.  The default is      after 120 seconds.
> +stalled and restart it.  The default is   after 120 seconds.
                                          ~~~

Please collapse the expanded TAB into a single space.

>  
>  *** erc-system-name: Determines the system name to use when logging in.
>  The default is to figure this out by calling `system-name'.
> @@ -2386,5 +2392,5 @@ Local variables:
>  coding: utf-8
>  mode: outline
>  mode: emacs-news
> -paragraph-separate: "[       ]*$"
> +paragraph-separate: "[  ]*$"
                          ^
I believe the TAB here is intentional, so please undo this hunk.

>  end:
> diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
> index 08dfa4b8f1b..3bd13f4a1a0 100644
> --- a/lisp/erc/erc.el
> +++ b/lisp/erc/erc.el
> @@ -4047,16 +4047,42 @@ 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))
> +
> +(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--connected-and-joined-p
> +    (erc-send-message line)))
> +(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 (string-remove-prefix " " line))
> +  (erc-with-all-buffers-of-server erc-server-process
> +      #'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
> +      #'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
> +      #'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.
>  
> diff --git a/test/lisp/erc/erc-scenarios-misc-commands.el 
> b/test/lisp/erc/erc-scenarios-misc-commands.el
> index d6ed53b5358..c6bb610b9df 100644
> --- a/test/lisp/erc/erc-scenarios-misc-commands.el
> +++ b/test/lisp/erc/erc-scenarios-misc-commands.el
> @@ -123,4 +123,88 @@ erc-scenarios-misc-commands--VHOST
>          (should (string= (erc-server-user-host (erc-get-server-user 
> "tester"))
>                           "some.host.test.cc"))))))
>  

[...]

Emanuel Berg <incal@dataswamp.org> writes:

> J.P. wrote:
>
>> What's not great is that the test still passes in spite of
>> this. It seems /GME is the only variant not covered, which
>> I guess is my fault. Perhaps you should improve the test so
>> it fails with the current patch applied and passes once
>> it's fixed.
>
> I'm not familiar with those tests so it is better you do
> that part, I think.

For reference, I have attached the revised test covering /GME. Don't
bother reincorporating it if you don't know how; I can do so when
installing.

>
>> Is this FIXME comment regarding your paperwork accurate?
>
> That had to do with my package on GNU ELPA [1] but that
> package has appeared so I suppose the paperwork issue has
> been solved.

I have confirmed this with the copyright clerk. Thanks.

0001-5.x-Add-tests-for-ERC-slash-commands-AMSG-GMSG-etc.patch
Description: Text Data