master 75b3fb3cb42 1/3: Ignore erc-response objects in error-notice helper

[F. Jason Park]

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

    Ignore erc-response objects in error-notice helper
    
    * lisp/erc/erc-button.el (erc-button--display-error-notice-with-keys):
    Remove `parsed' `erc-response' positional parameter, and don't pass it
    to `erc-display-message' because the latter adds text properties
    derived from such an object.  These properties can confuse other code
    operating on an inserted error-notice message into thinking it
    originated from the server.
    * lisp/erc/erc-common.el (erc--with-dependent-type-match): Make macro
    more readable.
    * lisp/erc/erc-networks.el (erc-networks--set-name,
    erc-networks--ensure-announced, erc-networks-on-MOTD-end): Remove
    `erc-response' objects from inserted error-notices.
    * test/lisp/erc/erc-button-tests.el
    (erc-button--display-error-notice-with-keys): Add assertions for
    overloaded first parameter.
    * test/lisp/erc/erc-tests.el (erc--with-dependent-type-match): Update
    expected expansion.
---
 lisp/erc/erc-button.el            | 36 +++++++++++++++++-------------------
 lisp/erc/erc-common.el            |  9 ++++-----
 lisp/erc/erc-networks.el          | 29 ++++++++++++++---------------
 test/lisp/erc/erc-button-tests.el | 28 ++++++++++++++++++++++++++++
 test/lisp/erc/erc-tests.el        | 10 +++++-----
 5 files changed, 68 insertions(+), 44 deletions(-)

diff --git a/lisp/erc/erc-button.el b/lisp/erc/erc-button.el
index bfaf4fa821a..8c1188e64a2 100644
--- a/lisp/erc/erc-button.el
+++ b/lisp/erc/erc-button.el
@@ -787,33 +787,31 @@ and `apropos' for other symbols."
     (erc-button-add-button from (point) fun nick-p data regexp)))
 
 ;;;###autoload
-(defun erc-button--display-error-notice-with-keys (&optional parsed buffer
-                                                             &rest strings)
+(defun erc-button--display-error-notice-with-keys (maybe-buffer &rest strings)
   "Add help keys to STRINGS for configuration-related admonishments.
-Return inserted result.  Expect PARSED to be an `erc-response'
-object, a string, or nil.  Expect BUFFER to be a buffer, a string,
-or nil.  As a special case, allow PARSED to be a buffer as long
-as BUFFER is a string or nil.  If STRINGS contains any trailing
+Return inserted result.  Expect MAYBE-BUFFER to be an ERC buffer,
+a string, or nil.  When it's a buffer, specify the `buffer'
+argument when calling `erc-display-message'.  Otherwise, add it
+to STRINGS.  If STRINGS contains any trailing non-nil
 non-strings, concatenate leading string members before applying
 `format'.  Otherwise, just concatenate everything."
-  (when (stringp buffer)
-    (push buffer strings)
-    (setq buffer nil))
-  (when (stringp parsed)
-    (push parsed strings)
-    (setq parsed nil))
-  (when (bufferp parsed)
-    (cl-assert (null buffer))
-    (setq buffer parsed
-          parsed nil))
-  (let* ((op (if (seq-every-p #'stringp (cdr strings))
+  (let* ((buffer (if (bufferp maybe-buffer)
+                     maybe-buffer
+                   (when (stringp maybe-buffer)
+                     (push maybe-buffer strings))
+                   'active))
+         (op (if (seq-every-p (lambda (o) (or (not o) (stringp o)))
+                              (cdr strings))
                  #'concat
                (let ((head (pop strings)))
-                 (while (stringp (car strings))
+                 (while (or (stringp (car strings))
+                            (and strings (not (car strings))))
                    (setq head (concat head (pop strings))))
                  (push head strings))
                #'format))
          (string (apply op strings))
+         (erc-insert-modify-hook (remove 'erc-add-timestamp
+                                         erc-insert-modify-hook))
          (erc-insert-post-hook
           (cons (lambda ()
                   (setq string (buffer-substring (point-min)
@@ -824,7 +822,7 @@ non-strings, concatenate leading string members before 
applying
              erc-button--display-error-with-buttons
              erc-button-describe-symbol 1)
             ,@erc-button-alist)))
-    (erc-display-message parsed '(t notice error) (or buffer 'active) string)
+    (erc-display-message nil '(t notice error) buffer string)
     string))
 
 ;;;###autoload
diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index 08c11d518a8..85971797c2f 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -475,12 +475,11 @@ Use the CASEMAPPING ISUPPORT parameter to determine the 
style."
 
 (defmacro erc--with-dependent-type-match (type &rest features)
   "Massage Custom :type TYPE with :match function that pre-loads FEATURES."
-  `(backquote (,(car type)
-               :match
-               ,(list '\, `(lambda (w v)
+  `(backquote-list* ',(car type)
+                    :match (lambda (w v)
                              ,@(mapcar (lambda (ft) `(require ',ft)) features)
-                             (,(widget-get (widget-convert type) :match) w v)))
-               ,@(cdr type))))
+                             (,(widget-get (widget-convert type) :match) w v))
+                    ',(cdr type)))
 
 (provide 'erc-common)
 
diff --git a/lisp/erc/erc-networks.el b/lisp/erc/erc-networks.el
index bf4ef1d35a9..ba7990e87d6 100644
--- a/lisp/erc/erc-networks.el
+++ b/lisp/erc/erc-networks.el
@@ -66,7 +66,7 @@
 (declare-function erc-set-active-buffer "erc" (buffer))
 
 (declare-function erc-button--display-error-notice-with-keys
-                  (parsed &rest strings))
+                  (maybe-buffer &rest strings))
 
 ;; Variables
 
@@ -1295,17 +1295,16 @@ shutting down the connection."
                          erc-network)))
      (erc-display-message parsed 'notice nil m)
      nil)
-    ((and
-      (guard (eq erc-network erc-networks--name-missing-sentinel))
-      ;; This can happen theoretically, e.g., when adjusting settings
-      ;; on a proxy service that partially impersonates IRC but isn't
-      ;; currently conveying anything through to a real network.  The
-      ;; service may send a 422 but no NETWORK param (or *any* 005s).
-      (let m (concat "Failed to determine network.  Please set entry for \""
-                     erc-server-announced-name "\" in `erc-networks-alist'"
-                     " or consider calling `erc-tls' with the keyword `:id'."
-                     "  See Info:\"(erc) Network Identifier\" for more.")))
-     (erc-display-error-notice parsed m)
+    ((guard (eq erc-network erc-networks--name-missing-sentinel))
+     ;; This can happen theoretically, e.g., when adjusting settings
+     ;; on a proxy service that partially impersonates IRC but isn't
+     ;; currently conveying anything through to a real network.  The
+     ;; service may send a 422 but no NETWORK param (or *any* 005s).
+     (erc-button--display-error-notice-with-keys
+      "Failed to determine network. Please set entry for \""
+      erc-server-announced-name "\" in `erc-networks-alist' or consider"
+      " calling `erc-tls' with the keyword `:id'."
+      " See Info:\"(erc) Network Identifier\" for more.")
      (if erc-networks--allow-unknown-network
          (progn
            (erc-display-error-notice
@@ -1325,9 +1324,9 @@ Copy source (prefix) from MOTD-ish message as a last 
resort."
   (unless erc-server-announced-name
     (require 'erc-button)
     (erc-button--display-error-notice-with-keys
-     parsed "Failed to determine server name.  Using \""
+     "Failed to determine server name. Using \""
      (setq erc-server-announced-name (erc-response.sender parsed)) "\" instead"
-     ".  If this was unexpected, consider reporting it via \\[erc-bug]" "."))
+     ". If this was unexpected, consider reporting it via \\[erc-bug]."))
   nil)
 
 (defun erc-unset-network-name (_nick _ip _reason)
@@ -1508,7 +1507,7 @@ to be a false alarm.  If `erc-reuse-buffers' is nil, let
                                proc)
       (require 'erc-button)
       (erc-button--display-error-notice-with-keys
-       parsed "Unexpected state detected.  Please report via \\[erc-bug].")))
+       "Unexpected state detected. Please report via \\[erc-bug].")))
 
   ;; For now, retain compatibility with erc-server-NNN-functions.
   (or (erc-networks--ensure-announced proc parsed)
diff --git a/test/lisp/erc/erc-button-tests.el 
b/test/lisp/erc/erc-button-tests.el
index 3dacf95a59f..34ad06b7eb8 100644
--- a/test/lisp/erc/erc-button-tests.el
+++ b/test/lisp/erc/erc-button-tests.el
@@ -274,6 +274,34 @@
                         "abc" " %d def" " 45%s" 123 '\6)
                        "*** abc 123 def 456")))
 
+      (ert-info ("Respects buffer as first argument when given")
+        (should (equal (erc-button--display-error-notice-with-keys
+                        (make-erc-response) "abc") ; compat
+                       "*** abc"))
+        (should (equal (erc-button--display-error-notice-with-keys
+                        (current-buffer) "abc")
+                       "*** abc")))
+
+      (ert-info ("Accounts for nil members when concatenating")
+        (should (equal (erc-button--display-error-notice-with-keys
+                        "a" nil)
+                       "*** a"))
+        (should (equal (erc-button--display-error-notice-with-keys
+                        "a" nil " b")
+                       "*** a b"))
+        (should (equal (erc-button--display-error-notice-with-keys
+                        "a: %d" nil 1)
+                       "*** a: 1"))
+        (should (equal (erc-button--display-error-notice-with-keys
+                        "a: %d %s" 1 nil)
+                       "*** a: 1 nil"))
+        (should (equal (erc-button--display-error-notice-with-keys
+                        "a: " "%d %s" 1 nil)
+                       "*** a: 1 nil"))
+        (should (equal (erc-button--display-error-notice-with-keys
+                        "a: " nil "%d %s" 1 nil)
+                       "*** a: 1 nil")))
+
       (when noninteractive
         (unless mode
           (erc-button-mode -1))
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 327ee46a736..9fdad823059 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -132,11 +132,11 @@
 (ert-deftest erc--with-dependent-type-match ()
   (should (equal (macroexpand-1
                   '(erc--with-dependent-type-match (repeat face) erc-match))
-                 '(backquote
-                   (repeat :match ,(lambda (w v)
-                                     (require 'erc-match)
-                                     (widget-editable-list-match w v))
-                           face)))))
+                 '(backquote-list*
+                   'repeat :match (lambda (w v)
+                                    (require 'erc-match)
+                                    (widget-editable-list-match w v))
+                   '(face)))))
 
 (defun erc-tests--send-prep ()
   ;; Caller should probably shadow `erc-insert-modify-hook' or