master 9aa2806fdc3 13/37: Modify erc-mode-map in module definitions

[F. Jason Park]

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

    Modify erc-mode-map in module definitions
    
    * lisp/erc/erc-button.el (erc-button-mode, erc-button-enable,
    erc-button-disable): Replace call to `erc-button-setup' with one to
    `erc--modify-local-map'.  This means `erc-button-setup' is now dead
    code from a client perspective.
    * lisp/erc/erc-goodies.el (erc-irccontrols-enable,
    erc-irccontrols-disable, erc-irccontrols-mode): Bind
    `erc-toggle-interpret-controls' in module definition so it's only
    available when the module is active.
    * lisp/erc/erc-log.el (erc-log-mode, erc-log-enable, erc-log-disable):
    Move top-level `define-key' into module definition.
    * lisp/erc/erc-match.el (erc-match-mode, erc-match-enable,
    erc-match-disable): Move top-level `define-key' into module
    definition.
    * lisp/erc/erc.el (erc-mode-map): Remove C-c C-c binding for
    `erc-toggle-interpret-controls'.
    (erc--modify-local-map): Add helper for global modules to use when
    modifying `erc-mode-map'.
    * test/lisp/erc/erc-tests.el (erc--modify-local-map): Add test.
    Ensure modifications to `erc-mode-map' on loading `erc' and via
    `erc-mode-hook' still work.  (Bug#60954.)
---
 lisp/erc/erc-button.el     |  6 ++++--
 lisp/erc/erc-goodies.el    |  6 ++++--
 lisp/erc/erc-log.el        |  8 ++++----
 lisp/erc/erc-match.el      |  8 ++++----
 lisp/erc/erc.el            | 14 +++++++++++++-
 test/lisp/erc/erc-tests.el | 44 ++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 73 insertions(+), 13 deletions(-)

diff --git a/lisp/erc/erc-button.el b/lisp/erc/erc-button.el
index c28dddefa0e..1be56f5dc21 100644
--- a/lisp/erc/erc-button.el
+++ b/lisp/erc/erc-button.el
@@ -55,11 +55,11 @@
   ((add-hook 'erc-insert-modify-hook #'erc-button-add-buttons 'append)
    (add-hook 'erc-send-modify-hook #'erc-button-add-buttons 'append)
    (add-hook 'erc-complete-functions #'erc-button-next-function)
-   (add-hook 'erc-mode-hook #'erc-button-setup))
+   (erc--modify-local-map t "<backtab>" #'erc-button-previous))
   ((remove-hook 'erc-insert-modify-hook #'erc-button-add-buttons)
    (remove-hook 'erc-send-modify-hook #'erc-button-add-buttons)
    (remove-hook 'erc-complete-functions #'erc-button-next-function)
-   (remove-hook 'erc-mode-hook #'erc-button-setup)))
+   (erc--modify-local-map nil "<backtab>" #'erc-button-previous)))
 
 ;;; Variables
 
@@ -233,6 +233,8 @@ constituents.")
   "Internal variable used to keep track of whether we've added the
 global-level ERC button keys yet.")
 
+;; Maybe deprecate this function and `erc-button-keys-added' if they
+;; continue to go unused for a another version (currently 5.6).
 (defun erc-button-setup ()
   "Add ERC mode-level button movement keys.  This is only done once."
   ;; Add keys.
diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el
index 7ca155ef9d0..7ff5b1aecdf 100644
--- a/lisp/erc/erc-goodies.el
+++ b/lisp/erc/erc-goodies.el
@@ -377,9 +377,11 @@ The value `erc-interpret-controls-p' must also be t for 
this to work."
 (define-erc-module irccontrols nil
   "This mode enables the interpretation of IRC control chars."
   ((add-hook 'erc-insert-modify-hook #'erc-controls-highlight)
-   (add-hook 'erc-send-modify-hook #'erc-controls-highlight))
+   (add-hook 'erc-send-modify-hook #'erc-controls-highlight)
+   (erc--modify-local-map t "C-c C-c" #'erc-toggle-interpret-controls))
   ((remove-hook 'erc-insert-modify-hook #'erc-controls-highlight)
-   (remove-hook 'erc-send-modify-hook #'erc-controls-highlight)))
+   (remove-hook 'erc-send-modify-hook #'erc-controls-highlight)
+   (erc--modify-local-map nil "C-c C-c" #'erc-toggle-interpret-controls)))
 
 (defun erc-controls-interpret (str)
    "Return a copy of STR after dealing with IRC control characters.
diff --git a/lisp/erc/erc-log.el b/lisp/erc/erc-log.el
index 2cb9031640d..a44437ddcf7 100644
--- a/lisp/erc/erc-log.el
+++ b/lisp/erc/erc-log.el
@@ -230,7 +230,8 @@ also be a predicate function.  To only log when you are not 
set away, use:
    ;; append, so that 'erc-initialize-log-marker runs first
    (add-hook 'erc-connect-pre-hook #'erc-log-setup-logging 'append)
    (dolist (buffer (erc-buffer-list))
-     (erc-log-setup-logging buffer)))
+     (erc-log-setup-logging buffer))
+   (erc--modify-local-map t "C-c C-l" #'erc-save-buffer-in-logs))
   ;; disable
   ((remove-hook 'erc-insert-post-hook #'erc-save-buffer-in-logs)
    (remove-hook 'erc-send-post-hook #'erc-save-buffer-in-logs)
@@ -241,9 +242,8 @@ also be a predicate function.  To only log when you are not 
set away, use:
    (remove-hook 'erc-part-hook #'erc-conditional-save-buffer)
    (remove-hook 'erc-connect-pre-hook #'erc-log-setup-logging)
    (dolist (buffer (erc-buffer-list))
-     (erc-log-disable-logging buffer))))
-
-(define-key erc-mode-map "\C-c\C-l" #'erc-save-buffer-in-logs)
+     (erc-log-disable-logging buffer))
+   (erc--modify-local-map nil "C-c C-l" #'erc-save-buffer-in-logs)))
 
 ;;; functionality referenced from erc.el
 (defun erc-log-setup-logging (buffer)
diff --git a/lisp/erc/erc-match.el b/lisp/erc/erc-match.el
index 52ee5c855f3..7ec9078d493 100644
--- a/lisp/erc/erc-match.el
+++ b/lisp/erc/erc-match.el
@@ -52,8 +52,10 @@ they are hidden or highlighted.  This is controlled via the 
variables
 `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 'append))
-  ((remove-hook 'erc-insert-modify-hook #'erc-match-message)))
+  ((add-hook 'erc-insert-modify-hook #'erc-match-message 'append)
+   (erc--modify-local-map t "C-c C-k" #'erc-go-to-log-matches-buffer))
+  ((remove-hook 'erc-insert-modify-hook #'erc-match-message)
+   (erc--modify-local-map nil "C-c C-k" #'erc-go-to-log-matches-buffer)))
 
 ;; Remaining customizations
 
@@ -647,8 +649,6 @@ See `erc-log-match-format'."
                                        (get-buffer (car buffer-cons))))))
     (switch-to-buffer buffer-name)))
 
-(define-key erc-mode-map "\C-c\C-k" #'erc-go-to-log-matches-buffer)
-
 (defun erc-hide-fools (match-type _nickuserhost _message)
  "Hide foolish comments.
 This function should be called from `erc-text-matched-hook'."
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 017f72476b1..60fe0480412 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1209,7 +1209,6 @@ which the local user typed."
     (define-key map [home] #'erc-bol)
     (define-key map "\C-c\C-a" #'erc-bol)
     (define-key map "\C-c\C-b" #'erc-switch-to-buffer)
-    (define-key map "\C-c\C-c" #'erc-toggle-interpret-controls)
     (define-key map "\C-c\C-d" #'erc-input-action)
     (define-key map "\C-c\C-e" #'erc-toggle-ctcp-autoresponse)
     (define-key map "\C-c\C-f" #'erc-toggle-flood-control)
@@ -1233,6 +1232,19 @@ which the local user typed."
     map)
   "ERC keymap.")
 
+(defun erc--modify-local-map (mode &rest bindings)
+  "Modify `erc-mode-map' on behalf of a global module.
+Add or remove `key-valid-p' BINDINGS when toggling MODE."
+  (declare (indent 1))
+  (while (pcase-let* ((`(,key ,def . ,rest) bindings)
+                      (existing (keymap-lookup erc-mode-map key)))
+           (if mode
+               (when (or (not existing) (eq existing #'undefined))
+                 (keymap-set erc-mode-map key def))
+             (when (eq existing def)
+               (keymap-unset erc-mode-map key t)))
+           (setq bindings rest))))
+
 ;; Faces
 
 ; Honestly, I have a horrible sense of color and the "defaults" below
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 81381a0c800..0c7b06da436 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -492,6 +492,50 @@
     (should (equal (erc--target-from-string "&Bitlbee")
                    #s(erc--target-channel-local "&Bitlbee" &bitlbee)))))
 
+(ert-deftest erc--modify-local-map ()
+  (when (and (bound-and-true-p erc-irccontrols-mode)
+             (fboundp 'erc-irccontrols-mode))
+    (erc-irccontrols-mode -1))
+  (when (and (bound-and-true-p erc-match-mode)
+             (fboundp 'erc-match-mode))
+    (erc-match-mode -1))
+  (let* (calls
+         (inhibit-message noninteractive)
+         (cmd-foo (lambda () (interactive) (push 'foo calls)))
+         (cmd-bar (lambda () (interactive) (push 'bar calls))))
+
+    (ert-info ("Add non-existing")
+      (erc--modify-local-map t "C-c C-c" cmd-foo "C-c C-k" cmd-bar)
+      (with-temp-buffer
+        (set-window-buffer (selected-window) (current-buffer))
+        (use-local-map erc-mode-map)
+        (execute-kbd-macro "\C-c\C-c")
+        (execute-kbd-macro "\C-c\C-k"))
+      (should (equal calls '(bar foo))))
+    (setq calls nil)
+
+    (ert-info ("Add existing") ; Attempt to swap definitions fails
+      (erc--modify-local-map t "C-c C-c" cmd-bar "C-c C-k" cmd-foo)
+      (with-temp-buffer
+        (set-window-buffer (selected-window) (current-buffer))
+        (use-local-map erc-mode-map)
+        (execute-kbd-macro "\C-c\C-c")
+        (execute-kbd-macro "\C-c\C-k"))
+      (should (equal calls '(bar foo))))
+    (setq calls nil)
+
+    (ert-info ("Remove existing")
+      (ert-with-message-capture messages
+        (erc--modify-local-map nil "C-c C-c" cmd-foo "C-c C-k" cmd-bar)
+        (with-temp-buffer
+          (set-window-buffer (selected-window) (current-buffer))
+          (use-local-map erc-mode-map)
+          (execute-kbd-macro "\C-c\C-c")
+          (execute-kbd-macro "\C-c\C-k"))
+        (should (string-search "C-c C-c is undefined" messages))
+        (should (string-search "C-c C-k is undefined" messages))
+        (should-not calls)))))
+
 (ert-deftest erc-ring-previous-command-base-case ()
   (ert-info ("Create ring when nonexistent and do nothing")
     (let (erc-input-ring