From 226d4371e0d022f5080859736fa9161966049f4f Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Sun, 2 Jul 2023 20:57:46 -0700 Subject: [PATCH 1/2] Revert "Account for leading timestamps in erc-match" This reverts commit 99d74dcd45938e2686d93eb5649800e14a88cd84 but keeps the test file test/lisp/erc/erc-scenarios-match.el. This also implements a partial alternative solution by undoing the reordering of insert hooks owned by the `stamp' and `match' modules. The reordering was performed as part of d880a08f9592e51ada5749d10b472396683fb6ee "Cement ordering of essential hook members in ERC". The intent was to address the problem of timestamps not being hidden in matched "fool" messages. However, a better approach is to incorporate timestamps into hidden messages by merging `invisible' properties. This will be handled by a future change, most likely lumped in with bug#64301. * erc/ERC-NEWS: Fix erroneous claim about relative hook ordering pre-5.6, which somewhat informs the confusion belying the original wrongheaded change. * lisp/erc/erc-match.el (erc-match-mode, erc-match-enable): Change hook depth for `erc-insert-modify-hook' member from 60 to 50. (erc-text-matched-hook): Retain portion of updated doc string instead of reverting. * lisp/erc/erc-stamp.el (erc-stamp-mode, erc-stamp-enable): Change depth for insert and send-hook members from 50 to 60. * test/lisp/erc/erc-scenarios-match.el (erc-scenarios-match--stamp-left-current-nick erc-scenarios-match--stamp-left-fools-invisible): Temporarily disable the latter and fix expected hook ordering. * test/lisp/erc/erc-tests.el (erc--essential-hook-ordering): Fix expected order of default insert hooks. (Bug#60936) --- etc/ERC-NEWS | 2 +- lisp/erc/erc-match.el | 36 ++++++++-------------------- lisp/erc/erc-stamp.el | 4 ++-- test/lisp/erc/erc-scenarios-match.el | 11 +++++---- test/lisp/erc/erc-tests.el | 4 ++-- 5 files changed, 22 insertions(+), 35 deletions(-) diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS index 2f465e247d7..5665b760ea9 100644 --- a/etc/ERC-NEWS +++ b/etc/ERC-NEWS @@ -183,7 +183,7 @@ Luckily, ERC now leverages a feature introduced in Emacs 27, "hook depth," to secure the positions of a few key members of 'erc-insert-modify-hook' and 'erc-send-modify-hook'. So far, this includes the functions 'erc-button-add-buttons', 'erc-fill', -'erc-add-timestamp', and 'erc-match-message', which now appear in that +'erc-match-message', and 'erc-add-timestamp', which now appear in that order, when present, at depths beginning at 20 and ending below 80. Of most interest to module authors is the new relative positioning of the first two, 'erc-button-add-buttons' and 'erc-fill', which have diff --git a/lisp/erc/erc-match.el b/lisp/erc/erc-match.el index 204bf14a1cf..2b7fff87ff0 100644 --- a/lisp/erc/erc-match.el +++ b/lisp/erc/erc-match.el @@ -52,7 +52,7 @@ match `erc-current-nick-highlight-type'. For all these highlighting types, you can decide whether the entire message or only the sending nick is highlighted." - ((add-hook 'erc-insert-modify-hook #'erc-match-message 60) + ((add-hook 'erc-insert-modify-hook #'erc-match-message 50) (add-hook 'erc-mode-hook #'erc-match--modify-invisibility-spec) (unless erc--updating-modules-p (erc-buffer-do #'erc-match--modify-invisibility-spec)) @@ -237,10 +237,7 @@ erc-text-matched-hook ERC calls members with the arguments (MATCH-TYPE NUH MESSAGE), where MATCH-TYPE is one of the symbols `current-nick', `keyword', `pal', `dangerous-host', `fool', and NUH is an `erc-response' -sender, like bob!~bob@example.org. Users should keep in mind -that MESSAGE may not include decorations, such as white space or -time stamps, preceding the same text as inserted in the narrowed -buffer." +sender, like bob!~bob@example.org." :options '(erc-log-matches erc-hide-fools erc-beep-on-match) :type 'hook) @@ -462,19 +459,8 @@ erc-match-directed-at-fool-p (erc-list-match fools-end msg)))) (defun erc-match-message () - "Add faces to matching text in inserted message." - ;; Exclude leading whitespace, stamps, etc. - (let ((omin (point-min)) - (beg (or (and (not (get-text-property (point-min) 'erc-command)) - (next-single-property-change (point-min) 'erc-command)) - (point-min)))) - ;; FIXME when ERC no longer supports 28, use `with-restriction' - ;; with `:label' here instead of passing `omin'. - (save-restriction - (narrow-to-region beg (point-max)) - (erc-match--message omin)))) - -(defun erc-match--message (unrestricted-point-min) + "Mark certain keywords in a region. +Use this defun with `erc-insert-modify-hook'." ;; This needs some refactoring. (goto-char (point-min)) (let* ((to-match-nick-dep '("pal" "fool" "dangerous-host")) @@ -576,14 +562,12 @@ erc-match--message 'font-lock-face match-face))) ;; Else twiddle your thumbs. (t nil)) - ;; FIXME use `without-restriction' after dropping 28. - (save-restriction - (narrow-to-region unrestricted-point-min (point-max)) - (run-hook-with-args - 'erc-text-matched-hook (intern match-type) - (or nickuserhost - (concat "Server:" (erc-get-parsed-vector-type vector))) - message))))) + (run-hook-with-args + 'erc-text-matched-hook + (intern match-type) + (or nickuserhost + (concat "Server:" (erc-get-parsed-vector-type vector))) + message)))) (if nickuserhost (append to-match-nick-dep to-match-nick-indep) to-match-nick-indep))))) diff --git a/lisp/erc/erc-stamp.el b/lisp/erc/erc-stamp.el index aac51135a07..5035e60a87d 100644 --- a/lisp/erc/erc-stamp.el +++ b/lisp/erc/erc-stamp.el @@ -163,8 +163,8 @@ erc-timestamp-face (define-erc-module stamp timestamp "This mode timestamps messages in the channel buffers." ((add-hook 'erc-mode-hook #'erc-munge-invisibility-spec) - (add-hook 'erc-insert-modify-hook #'erc-add-timestamp 50) - (add-hook 'erc-send-modify-hook #'erc-add-timestamp 50) + (add-hook 'erc-insert-modify-hook #'erc-add-timestamp 60) + (add-hook 'erc-send-modify-hook #'erc-add-timestamp 60) (add-hook 'erc-mode-hook #'erc-stamp--recover-on-reconnect) (add-hook 'erc--pre-clear-functions #'erc-stamp--reset-on-clear) (unless erc--updating-modules-p diff --git a/test/lisp/erc/erc-scenarios-match.el b/test/lisp/erc/erc-scenarios-match.el index 49e6a3370fc..61368919d31 100644 --- a/test/lisp/erc/erc-scenarios-match.el +++ b/test/lisp/erc/erc-scenarios-match.el @@ -49,8 +49,9 @@ erc-scenarios-match--stamp-left-current-nick :port port :full-name "tester" :nick "tester") - (should (memq 'erc-match-message - (memq 'erc-add-timestamp erc-insert-modify-hook))) + ;; Module `timestamp' precedes `match' in insertion hooks. + (should (memq 'erc-add-timestamp + (memq 'erc-match-message erc-insert-modify-hook))) ;; The "match type" is `current-nick'. (funcall expect 5 "tester") (should (eq (get-text-property (1- (point)) 'font-lock-face) @@ -60,6 +61,7 @@ erc-scenarios-match--stamp-left-current-nick ;; some non-nil invisibility property spans the entire message. (ert-deftest erc-scenarios-match--stamp-left-fools-invisible () :tags '(:expensive-test) + (ert-skip "WIP: fix included in bug#64301") (erc-scenarios-common-with-cleanup ((erc-scenarios-common-dialog "join/legacy") (dumb-server (erc-d-run "localhost" t 'foonet)) @@ -84,8 +86,9 @@ erc-scenarios-match--stamp-left-fools-invisible :full-name "tester" :password "changeme" :nick "tester") - (should (memq 'erc-match-message - (memq 'erc-add-timestamp erc-insert-modify-hook))) + ;; Module `timestamp' precedes `match' in insertion hooks. + (should (memq 'erc-add-timestamp + (memq 'erc-match-message erc-insert-modify-hook))) (funcall expect 5 "This server is in debug mode"))) (ert-info ("Ensure lines featuring \"bob\" are invisible") diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el index b751ef50520..80c7c708fc5 100644 --- a/test/lisp/erc/erc-tests.el +++ b/test/lisp/erc/erc-tests.el @@ -1851,8 +1851,8 @@ erc--essential-hook-ordering '( :erc-insert-modify-hook (erc-controls-highlight ; 0 erc-button-add-buttons ; 30 erc-fill ; 40 - erc-add-timestamp ; 50 - erc-match-message) ; 60 + erc-match-message ; 50 + erc-add-timestamp) ; 60 :erc-send-modify-hook ( erc-controls-highlight ; 0 erc-button-add-buttons ; 30 -- 2.41.0