bug#57955: 29.0.50; Allow session-local ERC modules

["J.P." <jp@neverwas.me>]

[-- Attachment #1: Type: text/plain, Size: 5064 bytes --]

Tags: patch

Hi people,

Since its inception, ERC has aimed to support local modules, that is,
modules local to a connection. (If you need convincing of this, take a
look at `define-erc-module'.) This makes sense for a good many reasons,
chief among them simplified semantics when arranging for buffer-local
variables and hooks. Ancillary benefits include let-binding
`erc-modules' around entry-point invocations and selectively disabling
modules for particular sessions (e.g., after capability negotiation).

Unfortunately, this dream of ERC's authors was never fully realized.
Take a look at `erc-open', where you'll find would-be local vars being
set too early and thus clobbered when `erc-mode' (the major mode) is
activated. Various kludges have come along to circumvent this. For
example, the log module uses `erc-connect-pre-hook' to conduct its
buffer-local business. But it shouldn't have to. ERC can do better.

This patch aims to address this problem by partially changing the
purpose of the function `erc-update-modules' in a backward compatible
way. Instead of activating local modules immediately, it now returns
them in a list to be activated at a time and place of the caller's
choosing. The most opportune place for this, in terms of `erc-open', is
after all the core local variables have been determined, which exposes
them to module setup code. As a bonus, the major mode hook is likewise
deferred to this point.

This patch also reworks the module-to-features map and preferred-name
migration logic, which was incomplete. As far as third-party packages
are concerned, it's only been tested with erc-hl-nicks, thus far, but it
"should" work with others too. (Please let me know if that's a lie.)

Thanks,
J.P.

P.S. BTW, if anyone is friendly with the hl-nicks author, please ask
them to reach out regarding an unrelated custom.el issue; attempts to
contact them via GitHub have so far proven unsuccessful.


In GNU Emacs 29.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version
 3.24.34, cairo version 1.17.6) of 2022-09-19 built on localhost
Repository revision: 132d5cb0a3ec94afbb49772631861e00160ffffb
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12014000
System Description: Fedora Linux 36 (Workstation Edition)

Configured using:
 'configure --enable-check-lisp-object-type --enable-checking=yes,glyphs
 'CFLAGS=-O0 -g3'
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date subr-x mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils rmc iso-transl tooltip eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
faces cus-face macroexp files window text-properties overlay sha1 md5
base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process
emacs)

Memory information:
((conses 16 36059 6198)
 (symbols 48 5107 0)
 (strings 32 13115 1641)
 (string-bytes 1 372299)
 (vectors 16 9247)
 (vector-slots 8 146583 10252)
 (floats 8 21 25)
 (intervals 56 220 0)
 (buffers 1000 10))

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Support-local-ERC-modules-in-erc-mode-buffers.patch --]
[-- Type: text/x-patch, Size: 10741 bytes --]

From b88bcadffba84b64ae91d45b84736313ac49dfef Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 12 Jul 2021 03:44:28 -0700
Subject: [PATCH 2/4] Support local ERC modules in erc-mode buffers

* lisp/erc/erc.el (erc-migrate-modules): add some missing mappings.
(erc--module-name-migrations, erc--features-to-modules,
erc--modules-to-features): add alists to support simplified
module-name migrations.
(erc-update-modules): Change return value to a list of minor-mode
commands for local modules that need deferred activation, if any.  Use
`custom-variable-p' to detect flavor.  Currently, all modules are
global, meaning so are their accompanying minor modes.
(erc-open): Defer enabling of local modules via `erc-update-modules'
until after buffer is initialized with other local vars.  Also defer
major mode hooks so they can detect things like whether the buffer is
a server or target buffer.
(define-erc-modules): Don't enable local modules (minor modes) unless
`erc-mode' is the major mode. And don't disable them unless the minor
mode is actually active.  Also, don't mutate `erc-modules' when
dealing with a local module.  It's believed that the original authors
wanted this functionality.
---
 lisp/erc/erc.el            | 108 ++++++++++++++++++++++++-------------
 test/lisp/erc/erc-tests.el |  47 ++++++++++++++++
 2 files changed, 119 insertions(+), 36 deletions(-)

diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 20f22c896f..8fa9d0c8a3 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1390,7 +1390,9 @@ define-erc-module
 
 This will define a minor mode called erc-NAME-mode, possibly
 an alias erc-ALIAS-mode, as well as the helper functions
-erc-NAME-enable, and erc-NAME-disable.
+erc-NAME-enable, and erc-NAME-disable.  Beware that for global
+modules, these helpers, as well as the minor-mode toggle, all mutate
+the user option `erc-modules'.
 
 Example:
 
@@ -1426,16 +1428,21 @@ define-erc-module
          ,(format "Enable ERC %S mode."
                   name)
          (interactive)
-         (add-to-list 'erc-modules (quote ,name))
-         (setq ,mode t)
-         ,@enable-body)
+         (unless ,local-p
+           (cl-pushnew (erc--normalize-module-symbol ',name) erc-modules))
+         (when (or ,(not local-p) (eq major-mode 'erc-mode))
+           (setq ,mode t)
+           ,@enable-body))
        (defun ,disable ()
          ,(format "Disable ERC %S mode."
                   name)
          (interactive)
-         (setq erc-modules (delq (quote ,name) erc-modules))
-         (setq ,mode nil)
-         ,@disable-body)
+         (unless ,local-p
+           (setq erc-modules (delq (erc--normalize-module-symbol ',name)
+                                   erc-modules)))
+         (when (or ,(not local-p) ,mode)
+           (setq ,mode nil)
+           ,@disable-body))
        ,(when (and alias (not (eq name alias)))
           `(defalias
              ',(intern
@@ -2030,14 +2037,40 @@ erc-default-nicks
 (defvar-local erc-nick-change-attempt-count 0
   "Used to keep track of how many times an attempt at changing nick is made.")
 
+(defconst erc--features-to-modules
+  '((erc-pcomplete completion pcomplete)
+    (erc-capab capab-identify)
+    (erc-join autojoin)
+    (erc-page page ctcp-page)
+    (erc-sound sound ctcp-sound)
+    (erc-stamp stamp timestamp)
+    (erc-services services nickserv))
+  "Migration alist mapping a library feature to module names.
+Keys need not be unique: a library may define more than one
+module.")
+
+(defconst erc--modules-to-features
+  (cl-loop for (feature . names) in erc--features-to-modules
+           append (mapcar (lambda (name) (cons name feature)) names))
+  "Migration alist mapping a module's name to library feature.")
+
+(defconst erc--module-name-migrations
+  (let (pairs)
+    (pcase-dolist (`(,_ ,canonical . ,rest) erc--features-to-modules)
+      (dolist (obsolete rest)
+        (push (cons obsolete canonical) pairs)))
+    pairs)
+  "Association list of obsolete module names to canonical names.")
+
+(defun erc--normalize-module-symbol (module)
+  "Canonicalize symbol MODULE for `erc-modules'."
+  (or (cdr (assq module erc--module-name-migrations)) module))
+
 (defun erc-migrate-modules (mods)
   "Migrate old names of ERC modules to new ones."
   ;; modify `transforms' to specify what needs to be changed
   ;; each item is in the format '(old . new)
-  (let ((transforms '((pcomplete . completion))))
-    (delete-dups
-     (mapcar (lambda (m) (or (cdr (assoc m transforms)) m))
-             mods))))
+  (delete-dups (mapcar #'erc--normalize-module-symbol mods)))
 
 (defcustom erc-modules '(netsplit fill button match track completion readonly
                                   networks ring autojoin noncommands irccontrols
@@ -2116,27 +2149,22 @@ erc-modules
   :group 'erc)
 
 (defun erc-update-modules ()
-  "Run this to enable erc-foo-mode for all modules in `erc-modules'."
-  (let (req)
+  "Enable global minor mode for all global modules in `erc-modules'.
+Return minor-mode commands for all local modules, possibly for
+deferred invocation, as done by `erc-open' whenever a new ERC
+buffer is created.  Local modules were introduced in ERC 5.6."
+  (let (local-modules)
     (dolist (mod erc-modules)
-      (setq req (concat "erc-" (symbol-name mod)))
-      (cond
-       ;; yuck. perhaps we should bring the filenames into sync?
-       ((string= req "erc-capab-identify")
-        (setq req "erc-capab"))
-       ((string= req "erc-completion")
-        (setq req "erc-pcomplete"))
-       ((string= req "erc-pcomplete")
-        (setq mod 'completion))
-       ((string= req "erc-autojoin")
-        (setq req "erc-join")))
-      (condition-case nil
-          (require (intern req))
-        (error nil))
+      (require (or (alist-get mod erc--modules-to-features)
+                   (intern (concat "erc-" (symbol-name mod))))
+               nil 'noerror) ; some modules don't have a corresponding feature
       (let ((sym (intern-soft (concat "erc-" (symbol-name mod) "-mode"))))
-        (if (fboundp sym)
+        (unless (and sym (fboundp sym))
+          (error "`%s' is not a known ERC module" mod))
+        (if (custom-variable-p sym)
             (funcall sym 1)
-          (error "`%s' is not a known ERC module" mod))))))
+          (push sym local-modules))))
+    local-modules))
 
 (defun erc-setup-buffer (buffer)
   "Consults `erc-join-buffer' to find out how to display `BUFFER'."
@@ -2192,18 +2220,22 @@ erc-open
   (let* ((target (and channel (erc--target-from-string channel)))
          (buffer (erc-get-buffer-create server port nil target id))
          (old-buffer (current-buffer))
-         old-point
+         (old-recon-count erc-server-reconnect-count)
+         (old-point nil)
+         (delayed-modules nil)
          (continued-session (and erc--server-reconnecting
                                  (with-suppressed-warnings
                                      ((obsolete erc-reuse-buffers))
                                    erc-reuse-buffers))))
     (when connect (run-hook-with-args 'erc-before-connect server port nick))
-    (erc-update-modules)
     (set-buffer buffer)
     (setq old-point (point))
-    (let ((old-recon-count erc-server-reconnect-count))
-      (erc-mode)
-      (setq erc-server-reconnect-count old-recon-count))
+    (setq delayed-modules (erc-update-modules))
+
+    (delay-mode-hooks (erc-mode))
+
+    (setq erc-server-reconnect-count old-recon-count)
+
     (when (setq erc-server-connected (not connect))
       (setq erc-server-announced-name
             (buffer-local-value 'erc-server-announced-name old-buffer)))
@@ -2266,6 +2298,12 @@ erc-open
     (setq erc-dbuf
           (when erc-log-p
             (get-buffer-create (concat "*ERC-DEBUG: " server "*"))))
+
+    (erc-determine-parameters server port nick full-name user passwd)
+
+    (save-excursion (run-mode-hooks))
+    (dolist (mod delayed-modules) (funcall mod +1))
+
     ;; set up prompt
     (unless continued-session
       (goto-char (point-max))
@@ -2277,8 +2315,6 @@ erc-open
       (erc-display-prompt)
       (goto-char (point-max)))
 
-    (erc-determine-parameters server port nick full-name user passwd)
-
     ;; Saving log file on exit
     (run-hook-with-args 'erc-connect-pre-hook buffer)
 
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index b2ed29e80e..d3d319ab22 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -975,4 +975,51 @@ erc-message
     (kill-buffer "ExampleNet")
     (kill-buffer "#chan")))
 
+(ert-deftest erc-migrate-modules ()
+  (should (equal (erc-migrate-modules '(autojoin timestamp button))
+                 '(autojoin stamp button)))
+  ;; Default unchanged
+  (should (equal (erc-migrate-modules erc-modules) erc-modules)))
+
+(ert-deftest erc-update-modules ()
+  (let* (calls
+         (erc-modules '(fake-foo fake-bar)))
+    (cl-letf (((symbol-function 'require)
+               (lambda (s &rest _) (push s calls)))
+              ((symbol-function 'erc-fake-foo-mode)
+               (lambda (n) (push (cons 'fake-foo n) calls)))
+              ;; Here, foo is a global module (minor mode)
+              ((get 'erc-fake-foo-mode 'standard-value) #'ignore)
+              ((symbol-function 'erc-fake-bar-mode)
+               (lambda (n) (push (cons 'fake-bar n) calls)))
+              ((symbol-function 'erc-autojoin-mode)
+               (lambda (n) (push (cons 'autojoin n) calls)))
+              ((get 'erc-autojoin-mode 'standard-value) #'ignore)
+              ((symbol-function 'erc-networks-mode)
+               (lambda (n) (push (cons 'networks n) calls)))
+              ((symbol-function 'erc-completion-mode)
+               (lambda (n) (push (cons 'completion n) calls)))
+              ((get 'erc-completion-mode 'standard-value) #'ignore))
+
+      (ert-info ("Locals")
+        (should (equal (erc-update-modules)
+                       '(erc-fake-bar-mode)))
+        ;; Bar still required
+        (should (equal (nreverse calls) '(erc-fake-foo
+                                          (fake-foo . 1)
+                                          erc-fake-bar)))
+        (setq calls nil))
+
+      (ert-info ("Module name overrides")
+        (setq erc-modules '(completion autojoin networks))
+        (should-not (erc-update-modules)) ; no locals
+        (should (equal (nreverse calls)
+                       '(erc-pcomplete
+                         (completion . 1)
+                         erc-join
+                         (autojoin . 1)
+                         erc-networks
+                         (networks . 1))))
+        (setq calls nil)))))
+
 ;;; erc-tests.el ends here
-- 
2.37.2