From 75b3fb3cb42385612d446ee1907b87e64d988c2f Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Thu, 17 Aug 2023 19:18:50 -0700 Subject: [PATCH] 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 -- 2.39.2