Subject: Re: bug#56449: 29.0.50; Revisit target shortcuts in erc-message

From: "J.P." <[email-masked]>
Date: Tue, 12 Jul 2022 01:10:58 -0700

"J.P." <[email-masked]> writes:

> IOW, issuing a
>
>   ERC> /MSG , hi
>
> recalls the last nick to speak over a given connection. And
>
>   ERC> /MSG . hi
>
> reuses the target (channel or query) from the last `erc-message'
> invocation

Actually, thinking on this a bit has revealed that these two state items
(last received vs. last sent) may be less symmetrical than originally
believed in terms of use case and function: the "." form always expands
to a substring of something already typed, IOW "previous input", while
the "," form may well produce something novel.

So, if the dot form is more akin to everyday command-history recall,
then ERC may have an easier time providing a useful substitute by
improving upon its existing offering, namely, erc-ring. Indeed, adopting
in-house variants of the more powerful history-traversing commands seen
elsewhere in Emacs, such as `previous-complete-history-element' and (in
some sense) `previous-matching-history-element', wouldn't be the worst
call (IMO).

> For now, I propose temporarily adopting (1) by confining shared values
> to server buffers.

... and stopping there rather than further complicating the situation
for little perceived gain, convenience wise, by fiddling with imperfect
alternatives to niche features.

As for the comma form, in case the above wasn't clear, I'm in favor of
dialing things back and shelving the notion of enhanced recipient
recall. (In the future, we could conceivably retrieve candidates by
searching backward through text-properties to gather the most recent
speakers in a target buffer. These could be presented via completing
read, perhaps anchored by a transient to offer unified access to various
scopes, like buffer, network, etc.)

For the sake of completeness, I have attached the surviving changes,
minor as they are, and will be adding them or similar to ERC by week's
end unless anyone has additional insights. Thanks.

>From 9e0a581227ea94d97833056d8735b280ed753e52 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email-masked]>
Date: Wed, 6 Jul 2022 00:40:42 -0700
Subject: [PATCH] Avoid mutating default value of erc-server-last-peers

* lisp/erc/erc-backend.el (erc-server-last-peers): Leave default as
nil instead of a quoted constant.
(erc-server-connect): Initialize `erc-server-last-peers' to a new
value local to a server buffer.
(erc-message): Operate on server's local `erc-server-last-peers' value
instead of the global default.  Prefer replacing value instead of
mutating CDR to make for easier testing.
(erc-server-PRIVMSG): Create a new `erc-server-last-peers' instead of
mutating to make for easier testing.
---
 lisp/erc/erc-backend.el    | 23 +++++------
 test/lisp/erc/erc-tests.el | 82 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+), 12 deletions(-)

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index bc7a7d14dc..5af63e976c 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -230,7 +230,7 @@ erc-server-error-occurred
 (defvar-local erc-server-lines-sent nil
   "Line counter.")
 
-(defvar-local erc-server-last-peers '(nil . nil)
+(defvar-local erc-server-last-peers nil
   "Last peers used, both sender and receiver.
 Those are used for /MSG destination shortcuts.")
 
@@ -562,7 +562,7 @@ erc-server-connect
         (setq erc-server-last-received-time time))
       (setq erc-server-lines-sent 0)
       ;; last peers (sender and receiver)
-      (setq erc-server-last-peers '(nil . nil)))
+      (setq erc-server-last-peers (cons nil nil)))
     ;; we do our own encoding and decoding
     (when (fboundp 'set-process-coding-system)
       (set-process-coding-system process 'raw-text))
@@ -939,21 +939,20 @@ erc-message
 to will be used."
   (cond
    ((string-match "^\\s-*\\(\\S-+\\) ?\\(.*\\)" line)
-    (let ((tgt (match-string 1 line))
-          (s (match-string 2 line)))
+    (let* ((tgt (match-string 1 line))
+           (s (match-string 2 line))
+           (server-buffer (erc-server-buffer))
+           (peers (buffer-local-value 'erc-server-last-peers server-buffer)))
       (erc-log (format "cmd: MSG(%s): [%s] %s" message-command tgt s))
       (cond
        ((string= tgt ",")
-        (if (car erc-server-last-peers)
-            (setq tgt (car erc-server-last-peers))
-          (setq tgt nil)))
+        (setq tgt (car peers)))
        ((string= tgt ".")
-        (if (cdr erc-server-last-peers)
-            (setq tgt (cdr erc-server-last-peers))
-          (setq tgt nil))))
+        (setq tgt (cdr peers))))
       (cond
        (tgt
-        (setcdr erc-server-last-peers tgt)
+        (with-current-buffer server-buffer
+          (setq erc-server-last-peers (cons (car peers) tgt)))
         (erc-server-send (format "%s %s :%s" message-command tgt s)
                          force))
        (t
@@ -1550,7 +1549,7 @@ define-erc-response-handler
                     (erc-process-ctcp-reply proc parsed nick login host
                                             (match-string 1 msg)))))
          (t
-          (setcar erc-server-last-peers nick)
+          (setq erc-server-last-peers (cons nick (cdr erc-server-last-peers)))
           (setq s (erc-format-privmessage
                    (or fnick nick) msg
                    ;; If buffer is a query buffer,
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 4971d0e194..0f222edacf 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -893,4 +893,86 @@ erc-process-input-line
 
           (should-not calls))))))
 
+;; Note: if adding an erc-backend-tests.el, please relocate this there.
+
+(ert-deftest erc-message ()
+  (should-not erc-server-last-peers)
+  (let (server-proc
+        calls
+        erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook)
+    (cl-letf (((symbol-function 'erc-display-message)
+               (lambda (_ _ _ line) (push line calls)))
+              ((symbol-function 'erc-server-send)
+               (lambda (line _) (push line calls)))
+              ((symbol-function 'erc-server-buffer)
+               (lambda () (process-buffer server-proc))))
+      (with-current-buffer (get-buffer-create "ExampleNet")
+        (erc-mode)
+        (setq erc-server-current-nick "tester"
+              server-proc (start-process "sleep" (current-buffer) "sleep" "1")
+              erc-server-process server-proc
+              erc-server-last-peers (cons nil nil)
+              erc-server-users (make-hash-table :test 'equal)
+              erc-network 'ExampleNet)
+        (set-process-query-on-exit-flag erc-server-process nil))
+
+      (with-current-buffer (get-buffer-create "#chan")
+        (erc-mode)
+        (setq erc-server-process (buffer-local-value 'erc-server-process
+                                                     (get-buffer "ExampleNet"))
+              erc-default-recipients '("#chan")
+              erc-channel-users (make-hash-table :test 'equal)
+              erc-network 'ExampleNet)
+        (erc-update-current-channel-member "alice" "alice")
+        (erc-update-current-channel-member "tester" "tester"))
+
+      (with-current-buffer "ExampleNet"
+        (erc-server-PRIVMSG erc-server-process
+                            (make-erc-response
+                             :sender "[email-masked]"
+                             :command "PRIVMSG"
+                             :command-args '("#chan" "hi")
+                             :unparsed ":[email-masked] PRIVMSG #chan :hi"))
+        (should (equal erc-server-last-peers '("alice")))
+        (should (string-match "<alice>" (pop calls))))
+
+      (with-current-buffer "#chan"
+        (ert-info ("Shortcuts usable in target buffers")
+          (should-not (local-variable-p 'erc-server-last-peers))
+          (should-not erc-server-last-peers)
+          (erc-message "PRIVMSG" ". hi")
+          (should-not erc-server-last-peers)
+          (should (eq 'no-target (pop calls)))
+          (erc-message "PRIVMSG" ", hi")
+          (should-not erc-server-last-peers)
+          (should (string-match "alice :hi" (pop calls)))))
+
+      (with-current-buffer "ExampleNet"
+        (ert-info ("Shortcuts local in server bufs")
+          (should (equal erc-server-last-peers '("alice" . "alice")))
+          (erc-message "PRIVMSG" ", hi")
+          (should (equal erc-server-last-peers '("alice" . "alice")))
+          (should (string-match "PRIVMSG alice :hi" (pop calls)))
+          (setcdr erc-server-last-peers "bob")
+          (erc-message "PRIVMSG" ". hi")
+          (should (equal erc-server-last-peers '("alice" . "bob")))
+          (should (string-match "PRIVMSG bob :hi" (pop calls)))))
+
+      (with-current-buffer "#chan"
+        (ert-info ("Non-shortcuts are local to server buffer")
+          (should-not (local-variable-p 'erc-server-last-peers))
+          (should-not erc-server-last-peers)
+          (erc-message "PRIVMSG" "#chan hola")
+          (should-not erc-server-last-peers)
+          (should-not (default-value 'erc-server-last-peers))
+          (should (equal (buffer-local-value 'erc-server-last-peers
+                                             (get-buffer "ExampleNet"))
+                         '("alice" . "#chan")))
+          (should (string-match "hola" (pop calls))))))
+
+    (should-not erc-server-last-peers)
+    (should-not calls)
+    (kill-buffer "ExampleNet")
+    (kill-buffer "#chan")))
+
 ;;; erc-tests.el ends here
-- 
2.36.1