master 1767b0bd7e 19/25: Don't call erc-auto-query twice on PRIVMSG

[F. Jason Park]

branch: master
commit 1767b0bd7e2883c9467ebf0e0aedee1fd90bcf92
Author: F. Jason Park <jp@neverwas.me>
Commit: F. Jason Park <jp@neverwas.me>

    Don't call erc-auto-query twice on PRIVMSG
    
    * lisp/erc/erc-backend.el (erc-server-JOIN): Use `erc--open-target'
    instead of `erc-join'.
    (erc-server-PRIVMSG): Don't call `erc-auto-query' at all, and instead
    borrow the portion of its logic that detects when a query buffer
    should be created instead of a channel buffer.
    
    * lisp/erc/erc.el (erc-cmd-QUERY): Update the mode line explicitly
    after calling `erc-query' in case it's needed after `erc-setup-buffer'
    runs. Simplify.
    (erc-query, erc--open-target): Replace uses of `erc-query'
    with `erc--open-target' and make the former obsolete.  Don't call
    `erc-update-mode-line' because `erc-open' already does that.
    (erc-auto-query): Make this function obsolete.  It was previously only
    used in erc-backend.el and only sewed confusion.
    (erc-query-on-unjoined-chan-privmsg): Add note questioning its role.
    It was previously only used by the now deprecated `erc-auto-query'.
    
    * test/lisp/erc/erc-scenarios-misc.el
    (erc-scenarios-base-mask-target-routing): Add test for server masks.
    * test/lisp/erc/resources/base/mask-target-routing/foonet.eld: New file.
---
 lisp/erc/erc-backend.el                            | 26 ++++------
 lisp/erc/erc.el                                    | 56 ++++++++++++----------
 test/lisp/erc/erc-scenarios-misc.el                | 34 +++++++++++++
 .../resources/base/mask-target-routing/foonet.eld  | 45 +++++++++++++++++
 4 files changed, 120 insertions(+), 41 deletions(-)

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index 6fb581ca7c..bee2551d76 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -1334,16 +1334,7 @@ add things to `%s' instead."
         (let* ((str (cond
                      ;; If I have joined a channel
                      ((erc-current-nick-p nick)
-                      (setq buffer (erc-open erc-session-server 
erc-session-port
-                                             nick erc-session-user-full-name
-                                             nil nil
-                                             (list chnl) chnl
-                                             erc-server-process
-                                             nil
-                                             erc-session-username
-                                             (erc-networks--id-given
-                                              erc-networks--id)))
-                      (when buffer
+                      (when (setq buffer (erc--open-target chnl))
                         (set-buffer buffer)
                         (with-suppressed-warnings
                             ((obsolete erc-add-default-channel))
@@ -1534,6 +1525,13 @@ add things to `%s' instead."
              fnick)
         (setf (erc-response.contents parsed) msg)
         (setq buffer (erc-get-buffer (if privp nick tgt) proc))
+        ;; Even worth checking for empty target here? (invalid anyway)
+        (unless (or buffer noticep (string-empty-p tgt) (eq ?$ (aref tgt 0)))
+          (if (and privp msgp (not (erc-is-message-ctcp-and-not-action-p msg)))
+              (when erc-auto-query
+                (let ((erc-join-buffer erc-auto-query))
+                  (setq buffer (erc--open-target nick))))
+            (setq buffer (erc--open-target tgt))))
         (when buffer
           (with-current-buffer buffer
             (when privp (erc--unhide-prompt))
@@ -1569,13 +1567,7 @@ add things to `%s' instead."
                                     s parsed buffer nick)
                 (run-hook-with-args-until-success
                  'erc-echo-notice-hook s parsed buffer nick))
-            (erc-display-message parsed nil buffer s)))
-        (when (string= cmd "PRIVMSG")
-          (erc-auto-query proc parsed))))))
-
-;; FIXME: need clean way of specifying extra hooks in
-;; define-erc-response-handler.
-(add-hook 'erc-server-PRIVMSG-functions #'erc-auto-query)
+            (erc-display-message parsed nil buffer s)))))))
 
 (define-erc-response-handler (QUIT)
   "Another user has quit IRC." nil
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index cbb0c9f4b4..4b24f953dd 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -3791,13 +3791,14 @@ on the value of `erc-query-display'."
   ;; `kill-buffer'?  If it makes sense, re-add it.  -- SK @ 2021-11-11
   (interactive
    (list (read-string "Start a query with: ")))
-  (let ((session-buffer (erc-server-buffer))
-        (erc-join-buffer erc-query-display))
-    (if user
-        (erc-query user session-buffer)
+  (unless user
       ;; currently broken, evil hack to display help anyway
                                         ;(erc-delete-query))))
-      (signal 'wrong-number-of-arguments ""))))
+    (signal 'wrong-number-of-arguments ""))
+  (let ((erc-join-buffer erc-query-display))
+    (erc-with-server-buffer
+     (erc--open-target user))))
+
 (defalias 'erc-cmd-Q #'erc-cmd-QUERY)
 
 (defun erc-quit/part-reason-default ()
@@ -4473,28 +4474,30 @@ See `erc-default-server-hook'."
   (nconc erc-server-vectors (list parsed))
   nil)
 
-(defun erc-query (target server)
-  "Open a query buffer on TARGET, using SERVER.
+(defun erc--open-target (target)
+  "Open an ERC buffer on TARGET."
+  (erc-open erc-session-server
+            erc-session-port
+            (erc-current-nick)
+            erc-session-user-full-name
+            nil
+            nil
+            (list target)
+            target
+            erc-server-process
+            nil
+            erc-session-username
+            (erc-networks--id-given erc-networks--id)))
+
+(defun erc-query (target server-buffer)
+  "Open a query buffer on TARGET using SERVER-BUFFER.
 To change how this query window is displayed, use `let' to bind
 `erc-join-buffer' before calling this."
-  (unless (and server
-               (buffer-live-p server)
-               (set-buffer server))
+  (declare (obsolete "bind `erc-cmd-query' and call `erc-cmd-QUERY'" "29.1"))
+  (unless (buffer-live-p server-buffer)
     (error "Couldn't switch to server buffer"))
-  (let ((buf (erc-open erc-session-server
-                       erc-session-port
-                       (erc-current-nick)
-                       erc-session-user-full-name
-                       nil
-                       nil
-                       (list target)
-                       target
-                       erc-server-process
-                       erc-session-username)))
-    (unless buf
-      (error "Couldn't open query window"))
-    (erc-update-mode-line)
-    buf))
+  (with-current-buffer server-buffer
+    (erc--open-target target)))
 
 (defcustom erc-auto-query 'window-noselect
   "If non-nil, create a query buffer each time you receive a private message.
@@ -4513,6 +4516,9 @@ a new window, but not to select it.  See the 
documentation for
                  (const :tag "Use current buffer" buffer)
                  (const :tag "Use current buffer" t)))
 
+;; FIXME either retire this or put it to use or more clearly explain
+;; what it's supposed to do.  It's currently only used by the obsolete
+;; function `erc-auto-query'.
 (defcustom erc-query-on-unjoined-chan-privmsg t
   "If non-nil create query buffer on receiving any PRIVMSG at all.
 This includes PRIVMSGs directed to channels.  If you are using an IRC
@@ -4635,6 +4641,8 @@ and as second argument the event parsed as a vector."
              (erc-cmd-QUERY query))
            nil))))
 
+(make-obsolete 'erc-auto-query "try erc-cmd-QUERY instead" "29.1")
+
 (defun erc-is-message-ctcp-p (message)
   "Check if MESSAGE is a CTCP message or not."
   (string-match "^\C-a\\([^\C-a]*\\)\C-a?$" message))
diff --git a/test/lisp/erc/erc-scenarios-misc.el 
b/test/lisp/erc/erc-scenarios-misc.el
index 86cfa8b10b..9d6d5bc1d6 100644
--- a/test/lisp/erc/erc-scenarios-misc.el
+++ b/test/lisp/erc/erc-scenarios-misc.el
@@ -104,4 +104,38 @@
         (should-not erc-network)
         (should (string= erc-server-announced-name "irc.foonet.org"))))))
 
+;; Targets that are host/server masks like $*, $$*, and #* are routed
+;; to the server buffer: https://github.com/ircdocs/wooooms/issues/5
+
+(ert-deftest erc-scenarios-base-mask-target-routing ()
+  :tags '(:expensive-test)
+  (erc-scenarios-common-with-cleanup
+      ((erc-scenarios-common-dialog "base/mask-target-routing")
+       (dumb-server (erc-d-run "localhost" t 'foonet))
+       (port (process-contact dumb-server :service))
+       (expect (erc-d-t-make-expecter)))
+
+    (ert-info ("Connect to foonet")
+      (with-current-buffer (erc :server "127.0.0.1"
+                                :port port
+                                :nick "tester"
+                                :password "changeme"
+                                :full-name "tester")
+        (should (string= (buffer-name) (format "127.0.0.1:%d" port)))))
+
+    (erc-d-t-wait-for 10 (get-buffer "foonet"))
+
+    (ert-info ("Channel buffer #foo playback received")
+      (with-current-buffer (erc-d-t-wait-for 3 (get-buffer "#foo"))
+        (funcall expect 10 "Excellent workman")))
+
+    (ert-info ("Global notices routed to server buffer")
+      (with-current-buffer "foonet"
+        (funcall expect 10 "going down soon")
+        (funcall expect 10 "this is a warning")
+        (funcall expect 10 "second warning")
+        (funcall expect 10 "final warning")))
+
+    (should-not (get-buffer "$*"))))
+
 ;;; erc-scenarios-misc.el ends here
diff --git a/test/lisp/erc/resources/base/mask-target-routing/foonet.eld 
b/test/lisp/erc/resources/base/mask-target-routing/foonet.eld
new file mode 100644
index 0000000000..796d5566b6
--- /dev/null
+++ b/test/lisp/erc/resources/base/mask-target-routing/foonet.eld
@@ -0,0 +1,45 @@
+;; -*- mode: lisp-data; -*-
+((pass 1 "PASS :changeme"))
+((nick 1 "NICK tester"))
+((user 1 "USER user 0 * :tester")
+ (0 ":irc.foonet.org 001 tester :Welcome to the foonet IRC Network tester")
+ (0 ":irc.foonet.org 002 tester :Your host is irc.foonet.org, running version 
oragono-2.6.0-7481bf0385b95b16")
+ (0 ":irc.foonet.org 003 tester :This server was created Mon, 31 May 2021 
09:56:24 UTC")
+ (0 ":irc.foonet.org 004 tester irc.foonet.org oragono-2.6.0-7481bf0385b95b16 
BERTZios CEIMRUabefhiklmnoqstuv Iabefhkloqv")
+ (0 ":irc.foonet.org 005 tester AWAYLEN=390 BOT=B CASEMAPPING=ascii 
CHANLIMIT=#:100 CHANMODES=Ibe,k,fl,CEMRUimnstu CHANNELLEN=64 CHANTYPES=# 
ELIST=U EXCEPTS EXTBAN=,m FORWARD=f INVEX KICKLEN=390 :are supported by this 
server")
+ (0 ":irc.foonet.org 005 tester MAXLIST=beI:60 MAXTARGETS=4 MODES MONITOR=100 
NETWORK=foonet NICKLEN=32 PREFIX=(qaohv)~&@%+ STATUSMSG=~&@%+ 
TARGMAX=NAMES:1,LIST:1,KICK:1,WHOIS:1,USERHOST:10,PRIVMSG:4,TAGMSG:4,NOTICE:4,MONITOR:100
 TOPICLEN=390 UTF8MAPPING=rfc8265 UTF8ONLY WHOX :are supported by this server")
+ (0 ":irc.foonet.org 005 tester draft/CHATHISTORY=100 :are supported by this 
server")
+ (0 ":irc.foonet.org 251 tester :There are 0 users and 4 invisible on 1 
server(s)")
+ (0 ":irc.foonet.org 252 tester 0 :IRC Operators online")
+ (0 ":irc.foonet.org 254 tester 2 :channels formed")
+ (0 ":irc.foonet.org 255 tester :I have 4 clients and 0 servers")
+ (0 ":irc.foonet.org 265 tester 4 4 :Current local users 4, max 4")
+ (0 ":irc.foonet.org 266 tester 4 4 :Current global users 4, max 4")
+ (0 ":irc.foonet.org 422 tester :MOTD File is missing"))
+
+((mode-user 1.2 "MODE tester +i")
+ ;; No mode answer
+ (0 ":irc.znc.in 306 tester :You have been marked as being away")
+ (0 ":tester!~u@gq7yjr7gsu7nn.irc JOIN #foo")
+ (0 ":irc.foonet.org 353 tester = #foo :alice @bob rando tester")
+ (0 ":irc.foonet.org 366 tester #foo :End of /NAMES list.")
+ (0 ":***!znc@znc.in PRIVMSG #foo :Buffer Playback...")
+ (0 ":alice!~u@gq7yjr7gsu7nn.irc PRIVMSG #foo :[10:00:02] bob: All that he is 
hath reference to your highness.")
+ (0 ":bob!~u@gq7yjr7gsu7nn.irc PRIVMSG #foo :[10:00:06] alice: Excellent 
workman! Thou canst not paint a man so bad as is thyself.")
+ (0 ":***!znc@znc.in PRIVMSG #foo :Playback Complete.")
+ (0 ":irc.foonet.org NOTICE tester :[09:56:57] This server is in debug mode 
and is logging all user I/O. If you do not wish for everything you send to be 
readable by the server owner(s), please disconnect.")
+ (0 ":irc.foonet.org 305 tester :You are no longer marked as being away"))
+
+((mode 5 "MODE #foo")
+ (0 ":irc.foonet.org 324 tester #foo +nt")
+ (0 ":irc.foonet.org 329 tester #foo 1622454985")
+ ;; Invalid msg
+ (0.1 ":rando!~u@em2i467d4ejul.irc PRIVMSG :")
+ (0.1 ":alice!~u@gq7yjr7gsu7nn.irc PRIVMSG #foo :bob: Farewell, pretty lady: 
you must hold the credit of your father.")
+ (0.1 ":bob!~u@gq7yjr7gsu7nn.irc NOTICE $* :[Global notice] going down soon.")
+ (0.1 ":alice!~u@rz2v467q4rwhy.irc PRIVMSG #foo :bob: Well, this is the forest 
of Arden.")
+ (0.1 ":bob!~u@gq7yjr7gsu7nn.irc NOTICE $$* :[Global notice] this is a 
warning.")
+ (0.1 ":alice!~u@rz2v467q4rwhy.irc PRIVMSG #foo :bob: Be married under a bush, 
like a beggar ? Get you to church, and have a good priest that can tell you 
what marriage is: this fellow will but join you together as they join wainscot; 
then one of you will prove a shrunk panel, and like green timber, warp, warp.")
+ (0.1 ":bob!~u@gq7yjr7gsu7nn.irc PRIVMSG $* :[Global msg] second warning.")
+ (0.1 ":alice!~u@rz2v467q4rwhy.irc PRIVMSG #foo :bob: And will you, being a 
man of your breeding.")
+ (0.1 ":bob!~u@gq7yjr7gsu7nn.irc NOTICE #* :[Global notice] final warning."))