More clearly define local module behavior in ERC

* doc/misc/erc.texi (Modules): Label all local modules as being such.
Move `querypoll' to the auxiliary section.  Rework entire "Local
Modules" portion.
* lisp/erc/erc-goodies.el (erc-keep-place-indicator-mode)
(erc-command-indicator-mode): Mention what buffer types they operate in.
* lisp/erc/erc-nicks.el (erc-nicks-mode): Mention the mode is enabled in
all buffers.
* lisp/erc/erc-notify.el (erc-querypoll-mode): Mention which buffers it
operates in.
* lisp/erc/erc-sasl.el (erc-sasl-mode): Disable completely in target
buffers so its mode variable is nil.
* lisp/erc/erc-services.el (erc-services-regain-mode): Disable in target
buffers.
* lisp/erc/erc.el (erc-open): When activating local modules, skip those
that have just been enabled by a fellow module.  Do this even though
their setup code is meant to be idempotent.
* test/lisp/erc/erc-scenarios-base-local-modules.el
(erc-scenarios-base-local-modules--toggle-helpers): Revise to assert
current behavior.  (Bug#57955)

8 files changed, 116 insertions, 71 deletions

diff --git a/doc/misc/erc.texi b/doc/misc/erc.texi
index 7c934d8bb3a..1c0afa3b300 100644
--- a/doc/misc/erc.texi
+++ b/doc/misc/erc.texi
@@ -452,7 +452,7 @@ Buttonize URLs, nicknames, and other text
 Mark unidentified users on freenode and other servers supporting CAPAB.
 
 @cindex modules, command-indicator
-@item command-indicator
+@item command-indicator (local)
 Echo command lines for ``slash commands'', like @kbd{/JOIN #erc} and
 @kbd{/HELP join}
 
@@ -494,7 +494,7 @@ Display a menu in ERC buffers
 Detect netsplits
 
 @cindex modules, nicks
-@item nicks
+@item nicks (local)
 Automatically colorize nicks
 
 @cindex modules, nickbar
@@ -519,10 +519,6 @@ or your nickname is mentioned
 @item page
 Process CTCP PAGE requests from IRC
 
-@cindex modules, querypoll
-@item querypoll
-Update query participant data by continually polling the server
-
 @cindex modules, readonly
 @item readonly
 Make displayed lines read-only
@@ -536,7 +532,7 @@ Replace text in messages
 Enable an input history
 
 @cindex modules, sasl
-@item sasl
+@item sasl (local)
 Enable SASL authentication
 
 @cindex modules, scrolltobottom
@@ -583,22 +579,26 @@ Translate morse code in messages
 
 For various reasons, the following modules aren't currently listed in
 the Custom interface for @code{erc-modules}, but feel free to add them
-explicitly.  They may be managed by another module or considered more
-useful when toggled interactively or just deemed experimental.
+explicitly.  They may be managed by another module or just deemed too
+niche or experimental.
 
 @table @code
 
 @cindex modules, fill-wrap
-@item fill-wrap
+@item fill-wrap (local)
 Wrap long lines using @code{visual-line-mode}
 
 @cindex modules, keep-place-indicator
-@item keep-place-indicator
+@item keep-place-indicator (local)
 Remember your place in buffers with a visible reminder; activated
 interactively or via something like @code{erc-join-hook}
 
+@cindex modules, querypoll
+@item querypoll (local)
+Update query participant data by continually polling the server
+
 @cindex modules, services-regain
-@item services-regain
+@item services-regain (local)
 Automatically ask NickServ to reclaim your nick when reconnecting;
 experimental as of ERC 5.6
 
@@ -618,51 +618,84 @@ always loads anyway.
 @subheading Local Modules
 @cindex local modules
 
-All modules operate as minor modes under the hood, and some newer ones
-may be defined as buffer-local.  These so-called ``local modules'' are
-a work in progress and their behavior and interface are subject to
-change.  As of ERC 5.5, the only practical differences are as follows:
+@c Earlier language in code comments, commit messages, and tracker
+@c discussions used to describe a local module as being "active" in a
+@c buffer if it had a local binding but "disabled" if that binding's
+@c value was nil.  For better or worse, ERC has since abandoned that
+@c distinction and now considers "active" to be synonymous with
+@c "enabled".
+
+All modules operate as minor modes under the hood, and newer ones are
+mostly defined as buffer-local.  These so-called @dfn{local modules} are
+a work in progress, and their behavior and interface are subject to
+change.  As of ERC 5.6, the only practical differences are as follows:
 
 @enumerate
 @item
-``Control variables,'' like @code{erc-sasl-mode}, retain their values
-across IRC sessions and override @code{erc-module} membership when
-influencing module activation.
+@dfn{Mode variables}, a.k.a. @dfn{control variables}, like
+@code{erc-sasl-mode}, retain their values across IRC sessions.
 @item
 Removing a local module from @code{erc-modules} via Customize not only
-disables its mode but also kills its control variable in all ERC
-buffers.
+disables its mode but also kills its mode variable in all ERC buffers.
 @item
-``Mode toggles,'' like @code{erc-sasl-mode} and the complementary
-@code{erc-sasl-enable}/@code{erc-sasl-disable} pairing, behave
-differently than their global counterparts.
+@dfn{Mode commands}, like @code{erc-sasl-mode} and its one-way variants
+@code{erc-sasl-enable} and @code{erc-sasl-disable}, behave differently
+than their global counterparts.
 @end enumerate
 
-In target buffers, a local module's activation state survives
-``reassociation'' by default, but modules themselves always have the
-final say.  For example, a module may reset all instances of itself in
-its network context upon reconnecting.  Moreover, the value of a mode
-variable may be meaningless in buffers that its module has no interest
-in.  For example, the value of @code{erc-sasl-mode} doesn't matter in
-target buffers and may even remain non-@code{nil} after SASL has been
-disabled for the current connection (and vice versa).
-
-When it comes to server buffers, a module's activation state only
-persists for sessions revived via the automatic reconnection mechanism
-or a manual @samp{/reconnect} issued at the prompt.  In other words,
-this doesn't apply to sessions revived by an entry-point command, such
-as @code{erc-tls}, because such commands always ensure a clean slate
-by looking only to @code{erc-modules}.  Although a session revived in
-this manner may indeed harvest other information from a previous
-server buffer, it simply doesn't care which modules might have been
-active during that connection.
-
-Lastly, a local mode's toggle command, like @code{erc-sasl-mode}, only
-affects the current buffer, but its ``non-mode'' cousins, like
+To detect whether a module is local, examine its mode variable.  For
+example, if you run @kbd{C-h v erc-sasl-mode @key{RET}}, you'll notice
+it says ``Automatically becomes buffer-local when set''.  You can do the
+same in Lisp code with @code{(local-variable-if-set-p 'erc-sasl-mode)}.
+
+In an ERC buffer, a local module is either enabled or disabled if its
+mode variable has a local binding.  This @dfn{activation state} may
+contradict a module's presence in @code{erc-modules}, namely, in buffers
+where it isn't applicable or has otherwise been disabled.  In fact, a
+local module's membership in @code{erc-modules} does nothing more than
+guarantee
+
+@enumerate
+@item
+its setup code runs in @emph{new} buffers
+@item
+its mode variable has a local binding in all affected buffers
+@end enumerate
+
+In keeping with this, all built-in local modules disable themselves in
+nonapplicable buffers rather than remain no-ops.  Some also take strides
+to enable themselves elsewhere when needed or at least emit a helpful
+error.  For example, the @samp{nicks} module does both in server
+buffers, where it shares resources among the target buffers it primarily
+services.  ERC expects third-party local modules to mimic this pattern
+and to document what buffer types they operate in: server, query, or
+channel.  (In the case of @samp{nicks}, it would be all three: it's
+@dfn{session-local}.)
+
+In ERC, you can think of an IRC session as a group of buffers sharing
+the same connection to a server.  After a connection ends, this
+association endures so that ERC can revive the session when
+reconnecting.  As it does with connection parameters, ERC therefore
+persists a local module's activation state through reconnections,
+reenabling modules that were previously active while ensuring others are
+disabled.  A couple related things to note here are
+
+@enumerate
+@item
+each module must manage its own application data and restore or reset
+its environment accordingly
+@item
+session persistence is less predictable if a user changes the makeup of
+@code{erc-modules} between sessions
+@end enumerate
+
+When it comes to a local module's various activation commands, the
+primary mode command, like @code{erc-sasl-mode}, for example, only
+affects the current buffer, but its unidirectional cousins, like
 @code{erc-sasl-enable} and @code{erc-sasl-disable}, operate on all
-buffers belonging to their connection (when called interactively).
-And unlike global toggles, none of these ever mutates
-@code{erc-modules}.
+buffers belonging to their connection (when called interactively).  And
+unlike global toggles, none of these ever mutates @code{erc-modules}.
+
 
 @c FIXME add section to Advanced chapter for creating modules, and
 @c move this there.
diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el
index cfd0d3415ea..24a3205e0cd 100644
--- a/lisp/erc/erc-goodies.el
+++ b/lisp/erc/erc-goodies.el
@@ -372,7 +372,9 @@ than the indicator's position."
   "Buffer-local `keep-place' with fringe arrow and/or highlighted face.
 Play nice with global module `keep-place' but don't depend on it.
 Expect that users may want different combinations of `keep-place'
-and `keep-place-indicator' in different buffers."
+and `keep-place-indicator' in different buffers.
+
+This module is local to individual buffers."
   ((cond (erc-keep-place-mode)
          ((memq 'keep-place erc-modules)
           (erc-keep-place-mode +1))
@@ -589,7 +591,9 @@ message's speaker."
 Skip those appearing in `erc-noncommands-list'.
 
 Users can run \\[erc-command-indicator-toggle-hidden] to hide and
-reveal echoed command lines after they've been inserted."
+reveal echoed command lines after they've been inserted.
+
+This module is local to individual buffers."
   ((add-hook 'erc--input-review-functions
              #'erc--command-indicator-permit-insertion 80 t)
    (erc-command-indicator-toggle-hidden -1))
diff --git a/lisp/erc/erc-nicks.el b/lisp/erc/erc-nicks.el
index b0629f7754c..a3e9b1f7b46 100644
--- a/lisp/erc/erc-nicks.el
+++ b/lisp/erc/erc-nicks.el
@@ -541,7 +541,9 @@ Abandon search after examining LIMIT faces."
   nick-object)
 
 (define-erc-module nicks nil
-  "Uniquely colorize nicknames in target buffers."
+  "Uniquely colorize nicknames in target buffers.
+
+This module is local per connection."
   ((if erc--target
        (progn
          (erc-with-server-buffer
diff --git a/lisp/erc/erc-notify.el b/lisp/erc/erc-notify.el
index 82754cb1989..1e04c90177e 100644
--- a/lisp/erc/erc-notify.el
+++ b/lisp/erc/erc-notify.el
@@ -299,7 +299,8 @@ like `nickbar', to provide UI feedback when changes occur.
 Once ERC implements the `monitor' extension, this module will serve as
 an optional fallback for keeping query-participant rolls up to date on
 servers that lack support or are stingy with their allotments.  Until
-such time, this module should be considered experimental.
+such time, this module should be considered experimental and only really
+useful for bots and other non-interactive Lisp programs.
 
 This is a local ERC module, so selectively polling only a subset of
 query targets is possible but cumbersome.  To do so, ensure
@@ -307,7 +308,8 @@ query targets is possible but cumbersome.  To do so, ensure
 as appropriate in desired query buffers.  To stop polling for the
 current connection, toggle off the command \\[erc-querypoll-mode] from a
 server buffer, or run \\`M-x C-u erc-querypoll-disable RET' from a
-target buffer."
+target buffer.  Note that this module's minor mode must remain active in
+at least the server buffer."
   ((if erc--target
        (if (erc-query-buffer-p)
            (progn ; accommodate those who eschew `erc-modules'
diff --git a/lisp/erc/erc-sasl.el b/lisp/erc/erc-sasl.el
index a16f554f2d1..5228fc5e5aa 100644
--- a/lisp/erc/erc-sasl.el
+++ b/lisp/erc/erc-sasl.el
@@ -34,13 +34,6 @@
 ;;
 ;; - Implement a proxy mechanism that chooses the strongest available
 ;;   mechanism for you.  Requires CAP 3.2 (see bug#49860).
-;;
-;; - Integrate with whatever solution ERC eventually settles on to
-;;   handle user options for different network contexts.  At the
-;;   moment, this does its own thing for stashing and restoring
-;;   session options, but ERC should make abstractions available for
-;;   all local modules to use, possibly based on connection-local
-;;   variables.
 
 ;;; Code:
 (require 'erc)
@@ -315,9 +308,10 @@ If necessary, pass PROMPT to `read-passwd'."
 
 (define-erc-module sasl nil
   "Non-IRCv3 SASL support for ERC.
-This doesn't solicit or validate a suite of supported mechanisms."
-  ;; See bug#49860 for a CAP 3.2-aware WIP implementation.
-  ((unless erc--target
+This local module only enables its minor mode in server buffers, and it
+doesn't currently solicit or validate supported mechanisms."
+  ((if erc--target
+       (erc-sasl-mode -1)
      (setq erc-sasl--state (make-erc-sasl--state))
      ;; If the previous attempt failed during registration, this may be
      ;; non-nil and contain erroneous values, but how can we detect that?
diff --git a/lisp/erc/erc-services.el b/lisp/erc/erc-services.el
index 429424117eb..25da873076e 100644
--- a/lisp/erc/erc-services.el
+++ b/lisp/erc/erc-services.el
@@ -613,8 +613,10 @@ In practical terms, this means that this module, which is still
 somewhat experimental, is likely only useful in conjunction with
 SASL authentication or CertFP rather than the traditional approach
 provided by the `services' module it shares a library with (see Info
-node `(erc) SASL' for more)."
-  nil nil localp)
+node `(erc) SASL' for more).
+
+This local module's minor mode is only active in server buffers."
+  ((when erc--target (erc-services-regain-mode -1))) nil localp)
 
 (cl-defmethod erc--nickname-in-use-make-request
   ((want string) temp &context (erc-server-connected null)
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index afa8e0a7b72..0d72b46360e 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -2662,7 +2662,9 @@ side effect of setting the current buffer to the one it returns.  Use
     (erc--initialize-markers old-point continued-session)
     (erc-determine-parameters server port nick full-name user passwd)
     (save-excursion (run-mode-hooks)
-                    (dolist (mod (car delayed-modules)) (funcall mod +1))
+                    (dolist (mod (car delayed-modules))
+                      (unless (and (boundp mod) (symbol-value mod))
+                        (funcall mod +1)))
                     (dolist (var (cdr delayed-modules)) (set var nil)))
 
     ;; Saving log file on exit
diff --git a/test/lisp/erc/erc-scenarios-base-local-modules.el b/test/lisp/erc/erc-scenarios-base-local-modules.el
index 71923cc11f2..1d670b8f508 100644
--- a/test/lisp/erc/erc-scenarios-base-local-modules.el
+++ b/test/lisp/erc/erc-scenarios-base-local-modules.el
@@ -117,20 +117,25 @@
         (erc-cmd-QUIT "")
         (funcall expect 10 "finished")))
 
-    (ert-info ("Disabling works from a target buffer")
+    (ert-info ("Explicit disabling affects entire session")
+      ;; Even though the mode variable is nil (but locally bound) in
+      ;; this target buffer, disabling interactively with
+      ;; `erc-sasl-disable', deactivates the module session-wide.
       (with-current-buffer "#chan"
-        (should erc-sasl-mode)
-        (call-interactively #'erc-sasl-disable)
         (should-not erc-sasl-mode)
         (should (local-variable-p 'erc-sasl-mode))
+        (should (buffer-local-value 'erc-sasl-mode (get-buffer "foonet")))
+        (call-interactively #'erc-sasl-disable)
         (should-not (buffer-local-value 'erc-sasl-mode (get-buffer "foonet")))
+        (should-not erc-sasl-mode)
         (erc-cmd-RECONNECT)
         (funcall expect 10 "Some enigma, some riddle")
-        (should-not erc-sasl-mode) ; regression
+        (should-not erc-sasl-mode)
         (should (local-variable-p 'erc-sasl-mode)))
 
       (with-current-buffer "foonet"
         (should (local-variable-p 'erc-sasl-mode))
+        (should-not erc-sasl-mode)
         (funcall expect 10 "User modes for tester`")
         (erc-cmd-QUIT "")
         (funcall expect 10 "finished")))
@@ -139,7 +144,8 @@
       (with-current-buffer "#chan"
         (call-interactively #'erc-sasl-enable)
         (should (local-variable-p 'erc-sasl-mode))
-        (should erc-sasl-mode)
+        (should-not erc-sasl-mode)
+        (should (buffer-local-value 'erc-sasl-mode (get-buffer "foonet")))
         (erc-cmd-RECONNECT)
         (funcall expect 10 "Well met; good morrow, Titus and Hortensius.")
         (erc-cmd-QUIT ""))