unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#57955: 29.0.50; Allow session-local ERC modules
@ 2022-09-20 13:05 J.P.
  2022-09-20 17:43 ` Michael Albinus
  0 siblings, 1 reply; 11+ messages in thread
From: J.P. @ 2022-09-20 13:05 UTC (permalink / raw)
  To: 57955; +Cc: emacs-erc

[-- 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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#57955: 29.0.50; Allow session-local ERC modules
  2022-09-20 13:05 bug#57955: 29.0.50; Allow session-local ERC modules J.P.
@ 2022-09-20 17:43 ` Michael Albinus
  2022-09-21 13:15   ` J.P.
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Albinus @ 2022-09-20 17:43 UTC (permalink / raw)
  To: J.P.; +Cc: 57955, emacs-erc

"J.P." <jp@neverwas.me> writes:

> Hi people,

Hi,

> 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).

Without knowing erc in general and your patch in detail: this sounds
like you could profit from connection-local variables. Did you check
this?

> Thanks,
> J.P.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#57955: 29.0.50; Allow session-local ERC modules
  2022-09-20 17:43 ` Michael Albinus
@ 2022-09-21 13:15   ` J.P.
  0 siblings, 0 replies; 11+ messages in thread
From: J.P. @ 2022-09-21 13:15 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 57955, emacs-erc

Hi Michael,

Michael Albinus <michael.albinus@gmx.de> writes:

> Without knowing erc in general and your patch in detail: this sounds
> like you could profit from connection-local variables. Did you check
> this?

Not quite yet (only superficially).

At first glance, I'm not sure they're a perfect fit for this specific
issue, but I'll definitely investigate further. Either way, I'm thinking
they'd be a great solution (or inspiration) for an initiative we have on
the horizon, namely, devising a means of applying user options in a more
granular, contextual manner [1].

Also (if you happen to recall), a few of our recent exchanges ended with
me pledging to follow through on one thing or another. And yet, most of
those promises remain unfulfilled. Please know that I do plan on
addressing them "eventually" and that I very much appreciate your help
(and your patience).

Thanks,
J.P.


[1] In case you're interested, by "context," I'm referring to various
    logical (somewhat overlapping) IRC boundaries, such as

    - message: event-local, i.e., source-wise and message-type-wise
    - channel: target-local
    - network: connection-local

    Basically, I'm looking for something akin to a "context variable"
    facility, except not so much for managing concurrency but instead
    for transparently stashing and restoring message-processing
    environments matched against headers and protocol state. More info:

    https://lists.gnu.org/archive/html/emacs-erc/2021-10/msg00003.html





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#57955: 29.0.50; Allow session-local ERC modules
       [not found] <8735cm2o2l.fsf@neverwas.me>
@ 2022-10-26 13:16 ` J.P.
  2022-11-15 15:07   ` J.P.
       [not found]   ` <877czww91p.fsf@neverwas.me>
  2023-10-09  4:02 ` J.P.
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 11+ messages in thread
From: J.P. @ 2022-10-26 13:16 UTC (permalink / raw)
  To: 57955; +Cc: emacs-erc

I'd like to propose that these changes be included in ERC 5.5 and Emacs
29. If anyone has any concerns, please speak up. Otherwise, expect this
patch to be installed at some point as part of the larger SASL change
set in bug#29108. Thanks.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#57955: 29.0.50; Allow session-local ERC modules
  2022-10-26 13:16 ` J.P.
@ 2022-11-15 15:07   ` J.P.
       [not found]   ` <877czww91p.fsf@neverwas.me>
  1 sibling, 0 replies; 11+ messages in thread
From: J.P. @ 2022-11-15 15:07 UTC (permalink / raw)
  To: 57955; +Cc: emacs-erc

"J.P." <jp@neverwas.me> writes:

> I'd like to propose that these changes be included in ERC 5.5 and Emacs
> 29. If anyone has any concerns, please speak up. Otherwise, expect this
> patch to be installed at some point as part of the larger SASL change
> set in bug#29108. Thanks.

Brief update. The scope of this change has shrunk considerably. The
deferred loading and migrations stuff still applies, but the main
user-facing aspect, namely, support for let-binding a module's options
on entry-point invocation, has been abandoned (for now).

After looking into connection-local variables a bit, following Michael's
suggestion up thread, I have come to the opinion that the let-binding
idea was not fully formed and that options granularity is worthy of more
meditation and discussion and thus not a realistic goal for Emacs 5.5.

Thanks.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#57955: 29.0.50; Allow session-local ERC modules
       [not found]   ` <877czww91p.fsf@neverwas.me>
@ 2023-05-22  4:05     ` J.P.
  0 siblings, 0 replies; 11+ messages in thread
From: J.P. @ 2023-05-22  4:05 UTC (permalink / raw)
  To: 57955; +Cc: emacs-erc

"J.P." <jp@neverwas.me> writes:

> Brief update. The scope of this change has shrunk considerably. The
> deferred loading and migrations stuff still applies, but the main
> user-facing aspect, namely, support for let-binding a module's options
> on entry-point invocation, has been abandoned (for now).
>
> After looking into connection-local variables a bit, following Michael's
> suggestion up thread, I have come to the opinion that the let-binding
> idea was not fully formed and that options granularity is worthy of more
> meditation and discussion and thus not a realistic goal for Emacs 5.5.

To help with managing local modules, I've added some (possibly
temporary) convenience functions and other supporting items as part of
bug#60936. The most useful are:

  * macro `erc--restore-initialize-priors'

    This restores local variables from a previous session on major-mode
    hook or slightly later. The effect is similar to that provided by
    the `permanent-local' property, which we may end up settling for if
    the dream of context-local user options evaporates for good. This
    may also be of interest to global modules intended primarily for
    interactive use, such as the proposed `bufbar' and `nickbar'
    (bug#63595).

  * variable `erc--updating-modules-p'

    This is non-nil when running `erc-update-modules' in `erc-open'. It
    allows global modules to suppress superfluous buffer initialization
    pre-major-mode while still making that same init code available on
    demand for interactive invocations and indirect activation by
    dependent modules.

Please keep in mind that these may change or disappear at any time.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#57955: 29.0.50; Allow session-local ERC modules
       [not found] <8735cm2o2l.fsf@neverwas.me>
  2022-10-26 13:16 ` J.P.
@ 2023-10-09  4:02 ` J.P.
       [not found] ` <87o7h8jvet.fsf@neverwas.me>
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: J.P. @ 2023-10-09  4:02 UTC (permalink / raw)
  To: 57955; +Cc: emacs-erc

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

It turns out these changes removed some behavior involving the loading
of modules that's long been part of ERC's implicit interface. Basically,
when encountering a third-party module `mymod', ERC has always attempted
to `require' the (possibly nonexistent) feature `erc-mymod', and to do
so unconditionally. However, this bug changed that policy to instead
only attempt such loading if the corresponding command `erc-mymod-mode'
is undefined. That was likely unwise because some packages do
questionable things like

  ;;;###autoload
  (eval-after-load 'erc
    '(define-erc-module mymod nil "Doc" () ()))

which fails to provide the vital `symbol-file' association between the
minor-mode command and the library (because the command itself isn't
autoloaded). Such uses ignore the decades-old example in the doc string
of `define-erc-module', which clearly recommends

  ;;;###autoload(autoload 'erc-mymode-mode "erc-mymode")
  (define-erc-module mymod nil "Doc" () ())

as the surefire approach. That said, in cases where the library's name
matches the (prefixed) module name, no autoload cookie is necessary.
Unfortunately, many of these packages are in maintenance mode, with
authors unwilling to respond to suggestions to update such aberrant
uses. Thus, I think it's in ERC's best interest to accommodate them as
it always has. Please see the second patch below. Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-5.6-Honor-nil-values-in-erc-restore-initialize-prior.patch --]
[-- Type: text/x-patch, Size: 3513 bytes --]

From 01c7045757c6109e07053dcf3a712b74a1d34d41 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Thu, 5 Oct 2023 00:16:46 -0700
Subject: [PATCH 1/2] [5.6] Honor nil values in erc--restore-initialize-priors

* lisp/erc/erc.el (erc--restore-initialize-priors): Don't produce
invalid empty `setq' when given VARS that initialize to nil.  This
function is mainly used by local modules, which were first introduced
in 5.5 (bug#57955).
* test/lisp/erc/erc-tests.el (erc--restore-initialize-priors): Fix
expected expansion.  (Bug#60936)
---
 lisp/erc/erc.el            | 17 ++++++++---------
 test/lisp/erc/erc-tests.el | 17 +++++++----------
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index fb236f1f189..16651b41eef 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1366,16 +1366,15 @@ erc--target-priors
 (defmacro erc--restore-initialize-priors (mode &rest vars)
   "Restore local VARS for MODE from a previous session."
   (declare (indent 1))
-  (let ((existing (make-symbol "existing"))
+  (let ((priors (make-symbol "priors"))
+        (initp (make-symbol "initp"))
         ;;
-        restore initialize)
-    (while-let ((k (pop vars)) (v (pop vars)))
-      (push `(,k (alist-get ',k ,existing)) restore)
-      (push `(,k ,v) initialize))
-    `(if-let* ((,existing (or erc--server-reconnecting erc--target-priors))
-               ((alist-get ',mode ,existing)))
-         (setq ,@(mapcan #'identity (nreverse restore)))
-       (setq ,@(mapcan #'identity (nreverse initialize))))))
+        forms)
+    (while-let ((k (pop vars)))
+      (push `(,k (if ,initp (alist-get ',k ,priors) ,(pop vars))) forms))
+    `(let* ((,priors (or erc--server-reconnecting erc--target-priors))
+            (,initp (and ,priors (alist-get ',mode ,priors))))
+       (setq ,@(mapcan #'identity (nreverse forms))))))
 
 (defun erc--target-from-string (string)
   "Construct an `erc--target' variant from STRING."
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 8a68eca6196..64b503832f3 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -796,18 +796,15 @@ erc--valid-local-channel-p
       (should (erc--valid-local-channel-p "&local")))))
 
 (ert-deftest erc--restore-initialize-priors ()
-  ;; This `pcase' expands to 100+k.  Guess we could do something like
-  ;; (and `(,_ ((,e . ,_) . ,_) . ,_) v) first and then return a
-  ;; (equal `(if-let* ((,e ...)...)...) v) to cut it down to < 1k.
   (should (pcase (macroexpand-1 '(erc--restore-initialize-priors erc-my-mode
                                    foo (ignore 1 2 3)
-                                   bar #'spam))
-            (`(if-let* ((,e (or erc--server-reconnecting erc--target-priors))
-                        ((alist-get 'erc-my-mode ,e)))
-                  (setq foo (alist-get 'foo ,e)
-                        bar (alist-get 'bar ,e))
-                (setq foo (ignore 1 2 3)
-                      bar #'spam))
+                                   bar #'spam
+                                   baz nil))
+            (`(let* ((,p (or erc--server-reconnecting erc--target-priors))
+                     (,q (and ,p (alist-get 'erc-my-mode ,p))))
+                (setq foo (if ,q (alist-get 'foo ,p) (ignore 1 2 3))
+                      bar (if ,q (alist-get 'bar ,p) #'spam)
+                      baz (if ,q (alist-get 'baz ,p) nil)))
              t))))
 
 (ert-deftest erc--target-from-string ()
-- 
2.41.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-5.6-Sort-and-dedupe-when-loading-modules-in-erc-open.patch --]
[-- Type: text/x-patch, Size: 16507 bytes --]

From 8e85f0748859590f2e94339e3dab300f9de9f728 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Fri, 6 Oct 2023 17:34:04 -0700
Subject: [PATCH 2/2] [5.6] Sort and dedupe when loading modules in erc-open

* doc/misc/erc.texi: Add new subheading "Module Loading" under
"Modules" chapter.
* lisp/erc/erc.el (erc--sort-modules): New utility function to sort
and dedupe modules.
(erc-modules): In `custom-set' function, factor out collation step
into separate utility `erc--sort-modules'.
(erc-update-modules): Call `erc--update-modules' with `erc-modules'.
(erc--aberrant-modules): New variable, a list of symbols whose modules
are suspected of being incorrectly defined.
(erc--find-mode): Make heuristic more robust by always checking for a
mode activation command rather than just a state variable.  This fixes
a compatibility bug, new in 5.6, affecting third-party modules that
autoload their module definition instead of its corresponding
activation-toggle command.
(erc--update-modules): Add new positional argument `modules'.
(erc--setup-buffer-hook): Add new default member,
`erc--warn-about-aberrant-modules'.
(erc-open): Pass sorted `erc-modules' to `erc--update-modules'.
* test/lisp/erc/erc-tests.el (erc--sort-modules): New test.
(erc-tests--update-modules): New fixture.
(erc--update-modules): Remove and rework as three separate tests
dedicated to specific contexts.  The existing one had poor coverage
and was difficult if not impossible to follow.
(erc--update-modules/unknown, erc--update-modules/local,
erc--update-modules/realistic): New tests.  (Bug#57955)
---
 doc/misc/erc.texi          |  35 ++++++++
 lisp/erc/erc.el            |  52 ++++++++----
 test/lisp/erc/erc-tests.el | 163 ++++++++++++++++++++++++++-----------
 3 files changed, 183 insertions(+), 67 deletions(-)

diff --git a/doc/misc/erc.texi b/doc/misc/erc.texi
index 3297d8b17f0..3bfa240cacc 100644
--- a/doc/misc/erc.texi
+++ b/doc/misc/erc.texi
@@ -653,6 +653,41 @@ Modules
 @code{erc-modules}.
 
 
+@anchor{Module Loading}
+@subheading Module Loading
+@cindex module loading
+
+ERC loads internal modules in alphabetical order and third-party
+modules as they appear in @code{erc-modules}.  When defining your own
+module, take care to ensure ERC can find it.  An easy way to do that
+is by mimicking the example in the doc string for
+@code{define-erc-module}.  For historical reasons, ERC also falls back
+to @code{require}ing features.  For example, if some module
+@code{<mymod>} in @code{erc-modules} lacks a corresponding
+@code{erc-<mymod>-mode} command, ERC will attempt to load the library
+@code{erc-<mymod>} prior to connecting.  If this fails, ERC signals an
+error.  Users wanting to define modules in an init files should
+@code{(provide 'erc-<my-mod>)} somewhere to placate ERC.  Dynamically
+generating modules on the fly is not supported.
+
+Sometimes, packages attempt to autoload a module's definition instead
+of its minor-mode command, which breaks the link between the library
+and the module.  This means that enabling the mode by invoking its
+command toggle isn't enough to load its defining library.  Such
+packages should instead only supply autoload cookies featuring an
+explicit @code{autoload} form for their module's minor-mode command.
+As mentioned above, packages can also usually avoid autoload cookies
+entirely so long as their module's prefixed name matches that of its
+defining library and the latter's provided feature.
+
+Packages have also been seen to specify unnecessary top-level
+@code{eval-after-load} forms, which end up being ineffective in most
+cases.  Another unfortunate practice is mutating @code{erc-modules}
+itself in an autoloaded form.  Doing this tricks Customize into
+displaying the widget for @code{erc-modules} incorrectly, with
+built-in modules moved from the predefined checklist to the
+user-provided free-form area.
+
 @c PRE5_4: Document every option of every module in its own subnode
 
 
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 16651b41eef..60d745e1268 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -2004,6 +2004,13 @@ erc-migrate-modules
   ;; each item is in the format '(old . new)
   (delete-dups (mapcar #'erc--normalize-module-symbol mods)))
 
+(defun erc--sort-modules (modules)
+  (let (built-in third-party)
+    (dolist (mod modules)
+      (setq mod (erc--normalize-module-symbol mod))
+      (cl-pushnew mod (if (get mod 'erc--module) built-in third-party)))
+    `(,@(sort built-in #'string-lessp) ,@(nreverse third-party))))
+
 (defcustom erc-modules '( autojoin button completion fill imenu irccontrols
                           list match menu move-to-prompt netsplit
                           networks noncommands readonly ring stamp track)
@@ -2039,16 +2046,10 @@ erc-modules
                                           (when (symbol-value f)
                                             (funcall f 0))
                                           (kill-local-variable f)))))))))
-         (let (built-in third-party)
-           (dolist (v val)
-             (setq v (erc--normalize-module-symbol v))
-             (if (get v 'erc--module)
-                 (push v built-in)
-               (push v third-party)))
-           ;; Calling `set-default-toplevel-value' complicates testing
-           (set sym (append (sort built-in #'string-lessp)
-                            (nreverse third-party))))
+         ;; Calling `set-default-toplevel-value' complicates testing.
+         (set sym (erc--sort-modules val))
          ;; this test is for the case where erc hasn't been loaded yet
+         ;; FIXME explain how this ^ can occur or remove comment.
          (when (fboundp 'erc-update-modules)
            (unless erc--inside-mode-toggle-p
              (erc-update-modules))))
@@ -2112,15 +2113,29 @@ erc-modules
 (defun erc-update-modules ()
   "Enable minor mode for every module in `erc-modules'.
 Except ignore all local modules, which were introduced in ERC 5.5."
-  (erc--update-modules)
+  (erc--update-modules erc-modules)
   nil)
 
+(defvar erc--aberrant-modules nil
+  "Modules suspected of being improperly loaded.")
+
+(defun erc--warn-about-aberrant-modules ()
+  (when erc--aberrant-modules
+    (erc-button--display-error-notice-with-keys-and-warn
+     "The following modules exhibited strange loading behavior: "
+     (mapconcat (lambda (s) (format "`%s'" s)) erc--aberrant-modules ", ")
+     ". Please contact ERC with \\[erc-bug] if you believe this to be untrue."
+     " See Info:\"(erc) Module Loading\" for more.")
+    (setq erc--aberrant-modules nil)))
+
 (defun erc--find-mode (sym)
   (setq sym (erc--normalize-module-symbol sym))
-  (if-let* ((mode (intern-soft (concat "erc-" (symbol-name sym) "-mode")))
-            ((or (boundp mode)
-                 (and (fboundp mode)
-                      (autoload-do-load (symbol-function mode) mode)))))
+  (if-let ((mode (intern-soft (concat "erc-" (symbol-name sym) "-mode")))
+           ((and (fboundp mode)
+                 (autoload-do-load (symbol-function mode) mode)))
+           ((or (get sym 'erc--module)
+                (symbol-file mode)
+                (ignore (cl-pushnew sym erc--aberrant-modules)))))
       mode
     (and (require (or (get sym 'erc--feature)
                       (intern (concat "erc-" (symbol-name sym))))
@@ -2129,9 +2144,9 @@ erc--find-mode
          (fboundp mode)
          mode)))
 
-(defun erc--update-modules ()
+(defun erc--update-modules (modules)
   (let (local-modes)
-    (dolist (module erc-modules local-modes)
+    (dolist (module modules local-modes)
       (if-let ((mode (erc--find-mode module)))
           (if (custom-variable-p mode)
               (funcall mode 1)
@@ -2158,7 +2173,7 @@ erc--updating-modules-p
 confidently call (erc-foo-mode 1) without having to learn
 anything about the dependency's implementation.")
 
-(defvar erc--setup-buffer-hook nil
+(defvar erc--setup-buffer-hook '(erc--warn-about-aberrant-modules)
   "Internal hook for module setup involving windows and frames.")
 
 (defvar erc--display-context nil
@@ -2315,7 +2330,8 @@ erc-open
     (setq old-point (point))
     (setq delayed-modules
           (erc--merge-local-modes (let ((erc--updating-modules-p t))
-                                    (erc--update-modules))
+                                    (erc--update-modules
+                                     (erc--sort-modules erc-modules)))
                                   (or erc--server-reconnecting
                                       erc--target-priors)))
 
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 64b503832f3..0b88ad9cfa9 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -2293,65 +2293,130 @@ erc--find-group--real
   (should (eq (erc--find-group 'smiley nil) 'erc))
   (should (eq (erc--find-group 'unmorse nil) 'erc)))
 
-(ert-deftest erc--update-modules ()
-  (let (calls
-        erc-modules
-        erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook)
+(ert-deftest erc--sort-modules ()
+  (should (equal (erc--sort-modules '(networks foo fill bar fill stamp bar))
+                 ;; Third-party mods appear in original order.
+                 '(fill networks stamp foo bar))))
+
+(defun erc-tests--update-modules (fn)
+  (let* ((calls nil)
+         (custom-modes nil)
+         (on-load nil)
+
+         (get-calls (lambda () (prog1 (nreverse calls) (setq calls nil))))
+
+         (add-onload (lambda (m k v)
+                       (put (intern m) 'erc--feature k)
+                       (push (cons k (lambda () (funcall v m))) on-load)))
 
-    ;; This `lbaz' module is unknown, so ERC looks for it via the
-    ;; symbol proerty `erc--feature' and, failing that, by
-    ;; `require'ing its "erc-" prefixed symbol.
-    (should-not (intern-soft "erc-lbaz-mode"))
+         (mk-cmd (lambda (module)
+                   (let ((mode (intern (format "erc-%s-mode" module))))
+                     (fset mode (lambda (n) (push (cons mode n) calls))))))
+
+         (mk-builtin (lambda (module-string)
+                       (let ((s (intern module-string)))
+                         (put s 'erc--module s))))
+
+         (mk-global (lambda (module)
+                      (push (intern (format "erc-%s-mode" module))
+                            custom-modes))))
 
     (cl-letf (((symbol-function 'require)
                (lambda (s &rest _)
-                 (when (eq s 'erc--lbaz-feature)
-                   (fset (intern "erc-lbaz-mode") ; local module
-                         (lambda (n) (push (cons 'lbaz n) calls))))
-                 (push s calls)))
-
-              ;; Local modules
-              ((symbol-function 'erc-lbar-mode)
-               (lambda (n) (push (cons 'lbar n) calls)))
-              ((get 'lbaz 'erc--feature) 'erc--lbaz-feature)
-
-              ;; Global modules
-              ((symbol-function 'erc-gfoo-mode)
-               (lambda (n) (push (cons 'gfoo n) calls)))
-              ((get 'erc-gfoo-mode 'standard-value) 'ignore)
+                 ;; Simulate library being loaded, things defined.
+                 (when-let ((h (alist-get s on-load))) (funcall h))
+                 (push (cons 'req s) calls)))
+
+              ;; Spoof global module detection.
+              ((symbol-function 'custom-variable-p)
+               (lambda (v) (memq v custom-modes))))
+
+      (funcall fn get-calls add-onload mk-cmd mk-builtin mk-global))
+    (should-not erc--aberrant-modules)))
+
+(ert-deftest erc--update-modules/unknown ()
+  (erc-tests--update-modules
+
+   (lambda (get-calls _ mk-cmd _ mk-global)
+
+     (ert-info ("Baseline")
+       (let* ((erc-modules '(foo))
+              (obarray (obarray-make))
+              (err (should-error (erc--update-modules erc-modules))))
+         (should (equal (cadr err) "`foo' is not a known ERC module"))
+         (should (equal (funcall get-calls)
+                        `((req . ,(intern-soft "erc-foo")))))))
+
+     ;; Module's mode command exists but lacks an associated file.
+     (ert-info ("Bad autoload flagged as suspect")
+       (should-not erc--aberrant-modules)
+       (let* ((erc--aberrant-modules nil)
+              (obarray (obarray-make))
+              (erc-modules (list (intern "foo"))))
+
+         ;; Create a mode activation command.
+         (funcall mk-cmd "foo")
+
+         ;; Make the mode var global.
+         (funcall mk-global "foo")
+
+         ;; No local modules to return.
+         (should-not (erc--update-modules erc-modules))
+         (should (equal (mapcar #'prin1-to-string erc--aberrant-modules)
+                        '("foo")))
+         ;; ERC requires the library via prefixed module name.
+         (should (equal (mapcar #'prin1-to-string (funcall get-calls))
+                        `("(req . erc-foo)" "(erc-foo-mode . 1)"))))))))
+
+;; A local module (here, `lo2') lacks a mode toggle, so ERC tries to
+;; load its defining library, first via the symbol property
+;; `erc--feature', and then via an "erc-" prefixed symbol.
+(ert-deftest erc--update-modules/local ()
+  (erc-tests--update-modules
+
+   (lambda (get-calls add-onload mk-cmd mk-builtin mk-global)
+
+     (let* ((obarray (obarray-make 20))
+            (erc-modules (mapcar #'intern '("glo" "lo1" "lo2"))))
+
+       ;; Create a global and a local module.
+       (mapc mk-cmd '("glo" "lo1"))
+       (mapc mk-builtin '("glo" "lo1"))
+       (funcall mk-global "glo")
+       (funcall add-onload "lo2" 'explicit-feature-lib mk-cmd)
+
+       ;; Returns local modules.
+       (should (equal (mapcar #'symbol-name (erc--update-modules erc-modules))
+                      '("erc-lo2-mode" "erc-lo1-mode")))
+
+       ;; Requiring `erc-lo2' defines `erc-lo2-mode'.
+       (should (equal (mapcar #'prin1-to-string (funcall get-calls))
+                      `("(erc-glo-mode . 1)"
+                        "(req . explicit-feature-lib)")))))))
+
+(ert-deftest erc--update-modules/realistic ()
+  (let ((calls nil)
+        ;; Module `pcomplete' "resolves" to `completion'.
+        (erc-modules '(pcomplete autojoin networks)))
+    (cl-letf (((symbol-function 'require)
+               (lambda (s &rest _) (push (cons 'req s) calls)))
+
+              ;; Spoof global module detection.
+              ((symbol-function 'custom-variable-p)
+               (lambda (v)
+                 (memq v '(erc-autojoin-mode erc-networks-mode
+                                             erc-completion-mode))))
+              ;; Mock and spy real builtins.
               ((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)))
-              ((get 'erc-networks-mode 'standard-value) 'ignore)
               ((symbol-function 'erc-completion-mode)
-               (lambda (n) (push (cons 'completion n) calls)))
-              ((get 'erc-completion-mode 'standard-value) 'ignore))
-
-      (ert-info ("Unknown module")
-        (setq erc-modules '(lfoo))
-        (should-error (erc--update-modules))
-        (should (equal (pop calls) 'erc-lfoo))
-        (should-not calls))
+               (lambda (n) (push (cons 'completion n) calls))))
 
-      (ert-info ("Local modules")
-        (setq erc-modules '(gfoo lbar lbaz))
-        ;; Don't expose the mode here
-        (should (equal (mapcar #'symbol-name (erc--update-modules))
-                       '("erc-lbaz-mode" "erc-lbar-mode")))
-        ;; Lbaz required because unknown.
-        (should (equal (nreverse calls) '((gfoo . 1) erc--lbaz-feature)))
-        (fmakunbound (intern "erc-lbaz-mode"))
-        (unintern (intern "erc-lbaz-mode") obarray)
-        (setq calls nil))
-
-      (ert-info ("Global modules") ; `pcomplete' resolved to `completion'
-        (setq erc-modules '(pcomplete autojoin networks))
-        (should-not (erc--update-modules)) ; no locals
-        (should (equal (nreverse calls)
-                       '((completion . 1) (autojoin . 1) (networks . 1))))
-        (setq calls nil)))))
+      (should-not (erc--update-modules erc-modules)) ; no locals
+      (should (equal (nreverse calls)
+                     '((completion . 1) (autojoin . 1) (networks . 1)))))))
 
 (ert-deftest erc--merge-local-modes ()
   (cl-letf (((get 'erc-b-mode 'erc-module) 'b)
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#57955: 29.0.50; Allow session-local ERC modules
       [not found] ` <87o7h8jvet.fsf@neverwas.me>
@ 2023-10-14  0:23   ` J.P.
       [not found]   ` <87edhy9hne.fsf@neverwas.me>
  1 sibling, 0 replies; 11+ messages in thread
From: J.P. @ 2023-10-14  0:23 UTC (permalink / raw)
  To: 57955; +Cc: emacs-erc

I've added something similar to the proposed change as

  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=d46c016f

If you'll recall, I initially left this bug open as a reminder that we
still lack a solution to the glaring problem of configuration scoping.
To summarize, this concerns the pressing need to allow users to specify
different values for new (and hopefully existing) global options based
on context (like network, channel, speaker, etc.). One idea bandied
about has been adapting connection-local variables to be more abstract
and eventually integrating them with Customize and `use-package'. As
thing stand, though, each local module must itself decide whether it's
session-local, buffer-local, or both. And it must contend with stashing
and restoring its configured state on its own.

I'll likely close this bug after opening another to address these
broader configuration concerns. Thanks.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#57955: 29.0.50; Allow session-local ERC modules
       [not found]   ` <87edhy9hne.fsf@neverwas.me>
@ 2023-10-18 13:36     ` J.P.
  0 siblings, 0 replies; 11+ messages in thread
From: J.P. @ 2023-10-18 13:36 UTC (permalink / raw)
  To: 57955; +Cc: emacs-erc

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

"J.P." <jp@neverwas.me> writes:

> I've added something similar to the proposed change as
>
>   https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=d46c016f

Actually, this has proved insufficient for detecting top-level and
`eval-after-load' calls to `erc-update-modules'. The attached tweak
hopefully addresses that.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-5.6-Warn-about-top-level-erc-update-modules-calls.patch --]
[-- Type: text/x-patch, Size: 5002 bytes --]

From 1e2b653b3b4af5f19f24b700bdd55cb30c198670 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Tue, 17 Oct 2023 23:36:12 -0700
Subject: [PATCH] [5.6] Warn about top-level erc-update-modules calls

* doc/misc/erc.texi (Modules): Mention risk of infinite recursion when
packages run `erc-update-modules' on load.
* lisp/erc/erc.el (erc--warn-about-aberrant-modules): Tweak warning
message.
(erc--requiring-module-mode-p): New internal variable.
(erc--find-mode): Guard against recursive `require's.  (Bug#57955)
---
 doc/misc/erc.texi | 21 ++++++++++++---------
 lisp/erc/erc.el   | 21 ++++++++++++++++-----
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/doc/misc/erc.texi b/doc/misc/erc.texi
index 3bfa240cacc..c654c97731c 100644
--- a/doc/misc/erc.texi
+++ b/doc/misc/erc.texi
@@ -666,17 +666,17 @@ Modules
 @code{<mymod>} in @code{erc-modules} lacks a corresponding
 @code{erc-<mymod>-mode} command, ERC will attempt to load the library
 @code{erc-<mymod>} prior to connecting.  If this fails, ERC signals an
-error.  Users wanting to define modules in an init files should
+error.  Users defining personal modules in an init file should
 @code{(provide 'erc-<my-mod>)} somewhere to placate ERC.  Dynamically
 generating modules on the fly is not supported.
 
-Sometimes, packages attempt to autoload a module's definition instead
-of its minor-mode command, which breaks the link between the library
-and the module.  This means that enabling the mode by invoking its
-command toggle isn't enough to load its defining library.  Such
-packages should instead only supply autoload cookies featuring an
-explicit @code{autoload} form for their module's minor-mode command.
-As mentioned above, packages can also usually avoid autoload cookies
+Some packages attempt to autoload a module's definition instead of its
+minor-mode command, which breaks the link between the library and the
+module.  This means that enabling the mode by invoking its command
+toggle isn't enough to load its defining library.  Such packages
+should instead only supply autoload cookies featuring an explicit
+@code{autoload} form for their module's minor-mode command.  As
+mentioned above, packages can also usually avoid autoload cookies
 entirely so long as their module's prefixed name matches that of its
 defining library and the latter's provided feature.
 
@@ -686,7 +686,10 @@ Modules
 itself in an autoloaded form.  Doing this tricks Customize into
 displaying the widget for @code{erc-modules} incorrectly, with
 built-in modules moved from the predefined checklist to the
-user-provided free-form area.
+user-provided free-form area.  Similarly, some packages run
+@code{erc-update-modules} in top-level and autoloaded forms or in
+@code{after-load-functions} members, which can trigger recursive
+invocations.
 
 @c PRE5_4: Document every option of every module in its own subnode
 
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 5bf6496e926..f841e385ab7 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -2129,12 +2129,17 @@ erc--aberrant-modules
 (defun erc--warn-about-aberrant-modules ()
   (when (and erc--aberrant-modules (not erc--target))
     (erc-button--display-error-notice-with-keys-and-warn
-     "The following modules exhibited strange loading behavior: "
+     "The following modules exhibit strange loading behavior: "
      (mapconcat (lambda (s) (format "`%s'" s)) erc--aberrant-modules ", ")
      ". Please contact ERC with \\[erc-bug] if you believe this to be untrue."
      " See Info:\"(erc) Module Loading\" for more.")
     (setq erc--aberrant-modules nil)))
 
+(defvar erc--requiring-module-mode-p nil
+  "Non-nil when `require'ing `erc-mymod' from `erc-update-modules'.
+Used for detecting top-level calls to `erc-update-modules' by
+third-party packages.")
+
 (defun erc--find-mode (sym)
   (setq sym (erc--normalize-module-symbol sym))
   (if-let ((mode (intern-soft (concat "erc-" (symbol-name sym) "-mode")))
@@ -2144,10 +2149,16 @@ erc--find-mode
                 (symbol-file mode)
                 (ignore (cl-pushnew sym erc--aberrant-modules)))))
       mode
-    (and (require (or (get sym 'erc--feature)
-                      (intern (concat "erc-" (symbol-name sym))))
-                  nil 'noerror)
-         (setq mode (intern-soft (concat "erc-" (symbol-name sym) "-mode")))
+    (and (or (and erc--requiring-module-mode-p
+                  ;; Also likely non-nil: (eq sym (car features))
+                  (cl-pushnew sym erc--aberrant-modules))
+             (let ((erc--requiring-module-mode-p t))
+               (require (or (get sym 'erc--feature)
+                            (intern (concat "erc-" (symbol-name sym))))
+                        nil 'noerror))
+             (memq sym erc--aberrant-modules))
+         (or mode (setq mode (intern-soft (concat "erc-" (symbol-name sym)
+                                                  "-mode"))))
          (fboundp mode)
          mode)))
 
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#57955: 29.0.50; Allow session-local ERC modules
       [not found] <8735cm2o2l.fsf@neverwas.me>
                   ` (2 preceding siblings ...)
       [not found] ` <87o7h8jvet.fsf@neverwas.me>
@ 2024-02-10 20:36 ` J.P.
       [not found] ` <87wmrcxdjf.fsf@neverwas.me>
  4 siblings, 0 replies; 11+ messages in thread
From: J.P. @ 2024-02-10 20:36 UTC (permalink / raw)
  To: 57955; +Cc: emacs-erc

A new Emacs user recently complained about the lack of an example
showing local modules being `let'-bound around calls to ERC's entry
point functions in lisp code. Perhaps we should add such an example to
the Modules chapter and also possibly update the "Multiple networks"
example in Advanced > SASL to include a non-SASL connection:

  (defun my-erc-up (network)
    (interactive "Snetwork: ")
  
    (pcase network
      ('libera
       (let ((erc-modules (cons 'sasl erc-modules))
             (erc-sasl-mechanism 'external))
         (erc-tls :server "irc.libera.chat" :port 6697
                  :client-certificate t)))
      ('example
       (let ((erc-modules (cons 'sasl erc-modules))
             (erc-sasl-auth-source-function
              #'erc-sasl-auth-source-password-as-host))
         (erc-tls :server "irc.example.net" :port 6697
                  :user "alyssa"
                  :password "Example.Net")))
      (_ (call-interactively #'erc-tls))))





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#57955: 29.0.50; Allow session-local ERC modules
       [not found] ` <87wmrcxdjf.fsf@neverwas.me>
@ 2024-03-01  0:25   ` J.P.
  0 siblings, 0 replies; 11+ messages in thread
From: J.P. @ 2024-03-01  0:25 UTC (permalink / raw)
  To: 57955; +Cc: emacs-erc

"J.P." <jp@neverwas.me> writes:

> A new Emacs user recently complained about the lack of an example
> showing local modules being `let'-bound around calls to ERC's entry
> point functions in lisp code. Perhaps we should add such an example to
> the Modules chapter and also possibly update the "Multiple networks"
> example in Advanced > SASL to include a non-SASL connection:
>
>   (defun my-erc-up (network)
>     (interactive "Snetwork: ")
>   
>     (pcase network
>       ('libera
>        (let ((erc-modules (cons 'sasl erc-modules))
>              (erc-sasl-mechanism 'external))
>          (erc-tls :server "irc.libera.chat" :port 6697
>                   :client-certificate t)))
>       ('example
>        (let ((erc-modules (cons 'sasl erc-modules))
>              (erc-sasl-auth-source-function
>               #'erc-sasl-auth-source-password-as-host))
>          (erc-tls :server "irc.example.net" :port 6697
>                   :user "alyssa"
>                   :password "Example.Net")))
>       (_ (call-interactively #'erc-tls))))

I've added an example similar to this one.





^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-03-01  0:25 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-20 13:05 bug#57955: 29.0.50; Allow session-local ERC modules J.P.
2022-09-20 17:43 ` Michael Albinus
2022-09-21 13:15   ` J.P.
     [not found] <8735cm2o2l.fsf@neverwas.me>
2022-10-26 13:16 ` J.P.
2022-11-15 15:07   ` J.P.
     [not found]   ` <877czww91p.fsf@neverwas.me>
2023-05-22  4:05     ` J.P.
2023-10-09  4:02 ` J.P.
     [not found] ` <87o7h8jvet.fsf@neverwas.me>
2023-10-14  0:23   ` J.P.
     [not found]   ` <87edhy9hne.fsf@neverwas.me>
2023-10-18 13:36     ` J.P.
2024-02-10 20:36 ` J.P.
     [not found] ` <87wmrcxdjf.fsf@neverwas.me>
2024-03-01  0:25   ` J.P.

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).