master 1c960bda912 3/3: Use smarter default for erc-server-reconnect-function

[F. Jason Park]

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

    Use smarter default for erc-server-reconnect-function
    
    * doc/misc/erc.texi (Sample Configuration): Remove customization in
    `use-package' declaration for `erc-server-reconnect-function' as well as
    related language in the customization walk-through.  Do this because the
    new default incorporates `erc-server-delayed-check-reconnect' behavior
    for compatible connect functions.
    * etc/ERC-NEWS: Announce new default for `erc-server-reconnect-function'.
    * lisp/erc/erc-backend.el (erc-server-reconnect-function): Change
    default to `erc-server-prefer-check-reconnect'.
    (erc-server-delayed-check-reconnect): Use `process-send-string' instead
    of `send-string'.
    (erc--server-delayed-check-connectors): New variable.
    (erc-server-prefer-check-reconnect): New function.
    * test/lisp/erc/erc-scenarios-base-auto-recon.el
    (erc-scenarios-base-auto-recon-unavailable)
    (erc-scenarios-base-auto-recon-no-proto): Remove unnecessary
    `erc-server-reconnect-function' binding because the new default
    incorporates the behavior being tested for.
    * test/lisp/erc/erc-scenarios-base-buffer-display.el
    (erc-scenarios-base-buffer-display--reconnect-common):
    * test/lisp/erc/erc-scenarios-base-compat-rename-bouncer.el
    (erc-scenarios-common--base-compat-no-rename-bouncer):
    * test/lisp/erc/erc-scenarios-base-netid-bouncer-recon-base.el
    (erc-scenarios-base-netid-bouncer--recon-base):
    * test/lisp/erc/erc-scenarios-base-netid-bouncer-recon-both.el
    (erc-scenarios-base-netid-bouncer--recon-both):
    * test/lisp/erc/erc-scenarios-base-netid-bouncer-recon-id.el
    (erc-scenarios-base-netid-bouncer--reconnect-id-foo)
    (erc-scenarios-base-netid-bouncer--reconnect-id-bar):
    * test/lisp/erc/erc-scenarios-base-reconnect.el
    (erc-scenarios-base-reconnect-timer)
    (erc-scenarios-base-cancel-reconnect):
    * test/lisp/erc/erc-scenarios-services-misc.el
    (erc-scenarios-services-misc--reconnect-retry-nick):
    * test/lisp/erc/erc-scenarios-stamp.el
    (erc-scenarios-stamp--date-mode/reconnect): Explicitly bind
    `erc-server-reconnect-function' to `erc-server-delayed-reconnect', the
    former default, which does not do any probing.  (Bug#62044)
---
 doc/misc/erc.texi                                  | 37 +++++++---------------
 etc/ERC-NEWS                                       |  6 ++++
 lisp/erc/erc-backend.el                            | 20 ++++++++++--
 test/lisp/erc/erc-scenarios-base-auto-recon.el     |  8 +++--
 test/lisp/erc/erc-scenarios-base-buffer-display.el |  1 +
 .../erc-scenarios-base-compat-rename-bouncer.el    |  1 +
 .../erc-scenarios-base-netid-bouncer-recon-base.el |  3 +-
 .../erc-scenarios-base-netid-bouncer-recon-both.el |  4 ++-
 .../erc-scenarios-base-netid-bouncer-recon-id.el   |  6 ++--
 test/lisp/erc/erc-scenarios-base-reconnect.el      |  2 ++
 test/lisp/erc/erc-scenarios-services-misc.el       |  1 +
 test/lisp/erc/erc-scenarios-stamp.el               |  1 +
 12 files changed, 55 insertions(+), 35 deletions(-)

diff --git a/doc/misc/erc.texi b/doc/misc/erc.texi
index 0f6b6b8c5be..b5838b3b0ce 100644
--- a/doc/misc/erc.texi
+++ b/doc/misc/erc.texi
@@ -1340,8 +1340,7 @@ settings (@pxref{Sample configuration via Customize}).
   ;; Scroll all windows to prompt when submitting input.
   (erc-scrolltobottom-all t)
 
-  ;; Reconnect automatically using a fancy strategy.
-  (erc-server-reconnect-function #'erc-server-delayed-check-reconnect)
+  ;; Wait a bit longer between automatic reconnect attempts.
   (erc-server-reconnect-timeout 30)
 
   ;; Show new buffers in the current window instead of a split.
@@ -1444,30 +1443,18 @@ descriptions just in case you want to disable them 
later.  When
 finished, hit @kbd{C-x C-s} or click @samp{[Apply and Save]} atop the
 buffer.
 
-Now do the same for another couple options, this time having to do
-with automatic reconnection.  But instead of searching for their print
-names, try running @kbd{M-x customize-option @key{RET} @samp{<option>}
-@key{RET}}, replacing @samp{<option>} with:
+Now do the same for another option, this time having to do with
+automatic reconnection.  But instead of searching for its print name,
+try running @kbd{M-x customize-option @key{RET}
+erc-server-reconnect-timeout @key{RET}}.  (If it helps, hit @key{TAB}
+for completion.)  As you may have noticed, when customizing options
+individually, each buffer displays but a single option's widget.  For
+@code{erc-server-reconnect-timeout}, you'll encounter a text field
+(instead of a button), which works like those in a typical web form.
+Enter @samp{30} and either hit @kbd{C-x C-s} to save or @key{TAB} over
+to @samp{[State]} and hit @key{RET} followed by @kbd{1} to persist your
+changes.
 
-@itemize @bullet
-@item @code{erc-server-reconnect-function}, a function
-@item @code{erc-server-reconnect-timeout}, a number
-@end itemize
-
-@noindent
-(If it helps, hit @key{TAB} for completion.)  As you may have noticed,
-when customizing options individually, each buffer displays but a
-single option's widget.  When you get to the buffer for ``Erc Server
-Reconnect Function'', you'll see that @samp{[Toggle]} has been
-replaced with @samp{[Value Menu]} and that clicking it reveals three
-choices in a pop-up window.  Enter @kbd{1} to select
-@code{erc-server-delayed-check-reconnect} before @key{TAB}'ing over to
-@samp{[State]} and hitting @key{RET}.  Enter @kbd{1} again, this time
-to persists your changes.
-
-For the final option, @code{erc-server-reconnect-timeout}, you'll
-encounter a text field (instead of a button), which works like those
-in a typical web form.  Enter @samp{30} and hit @kbd{C-x C-s} to save.
 Just for fun, click the group link for @samp{Erc Server} at the bottom
 of the buffer.  You could just as well have set the last two options
 from this ``custom group'' buffer alone, which very much resembles the
diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS
index f3c8645f02d..b7b49956d06 100644
--- a/etc/ERC-NEWS
+++ b/etc/ERC-NEWS
@@ -62,6 +62,12 @@ of concerns and the newer module's "experimental" status, 
the migration
 was deemed worth any potential disruption, despite this being a point
 release.  ERC appreciates your understanding in this matter.
 
+** Option 'erc-server-reconnect-function' has a new default.
+ERC 5.6 added 'erc-server-delayed-check-reconnect', whose "probing"
+strategy worked better for most users.  While compatibility concerns
+prevented it from becoming the new 'erc-server-reconnect-function'
+outright, a new solution has emerged that defers to it when sensible.
+
 ** Entry-point command 'erc-tls' once again considers option 'erc-port'.
 In its zeal to enforce a preference for TLS connections, ERC 5.5 went a
 bit far in disregarding the useful user option 'erc-port'.  When called
diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index 311e3a624e6..ab26a3715f1 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -429,15 +429,16 @@ this value to 120 or greater and/or exploring the option
 means of handling this situation on some servers."
   :type 'number)
 
-(defcustom erc-server-reconnect-function 'erc-server-delayed-reconnect
+(defcustom erc-server-reconnect-function 'erc-server-prefer-check-reconnect
   "Function called by the reconnect timer to create a new connection.
 Called with a server buffer as its only argument.  Potential uses
 include exponential backoff and probing for connectivity prior to
 dialing.  Use `erc-schedule-reconnect' to instead try again later
 and optionally alter the attempts tally."
-  :package-version '(ERC . "5.5")
+  :package-version '(ERC . "5.6.1")
   :type '(choice (function-item erc-server-delayed-reconnect)
                  (function-item erc-server-delayed-check-reconnect)
+                 (function-item erc-server-prefer-check-reconnect)
                  function))
 
 (defcustom erc-split-line-length 440
@@ -879,7 +880,7 @@ Expect BUFFER to be the server buffer for the current 
connection."
              (sentinel (lambda (proc event)
                          (pcase event
                            ("open\n"
-                            (run-at-time nil nil #'send-string proc
+                            (run-at-time nil nil #'process-send-string proc
                                          (format "PING %d\r\n"
                                                  (time-convert nil 'integer))))
                            ((or "connection broken by remote peer\n"
@@ -901,6 +902,19 @@ Expect BUFFER to be the server buffer for the current 
connection."
               (set-process-sentinel proc sentinel))
           (file-error (funcall reschedule nil)))))))
 
+(defvar erc--server-delayed-check-connectors
+  '(erc-open-tls-stream erc-open-network-stream)
+  "Functions compatible with `erc-server-delayed-check-reconnect'.")
+
+(defun erc-server-prefer-check-reconnect (buffer)
+  "Defer to another reconnector based on BUFFER's `erc-session-connector'.
+Prefer `erc-server-delayed-check-reconnect' if the connector is known to
+be \"check-aware\".  Otherwise, use `erc-server-delayed-reconnect'."
+  (if (memq (buffer-local-value 'erc-session-connector buffer)
+            erc--server-delayed-check-connectors)
+      (erc-server-delayed-check-reconnect buffer)
+    (erc-server-delayed-reconnect buffer)))
+
 (defun erc-server-filter-function (process string)
   "The process filter for the ERC server."
   (with-current-buffer (process-buffer process)
diff --git a/test/lisp/erc/erc-scenarios-base-auto-recon.el 
b/test/lisp/erc/erc-scenarios-base-auto-recon.el
index 808b1d8c4d4..d6a114147b3 100644
--- a/test/lisp/erc/erc-scenarios-base-auto-recon.el
+++ b/test/lisp/erc/erc-scenarios-base-auto-recon.el
@@ -24,6 +24,10 @@
   (let ((load-path (cons (ert-resource-directory) load-path)))
     (require 'erc-scenarios-common)))
 
+;; This tests `erc-server-delayed-check-reconnect', which is called by
+;; `erc-server-prefer-check-reconnect' (the default value of
+;; `erc-server-reconnect-function' as of ERC 5.6.1).
+
 (defun erc-scenarios-base-auto-recon--get-unused-port ()
   (let ((server (make-network-process :name "*erc-scenarios-base-auto-recon*"
                                       :host "localhost"
@@ -42,7 +46,6 @@
        (port (erc-scenarios-base-auto-recon--get-unused-port))
        (erc--server-reconnect-timeout-scale-function (lambda (_) 1))
        (erc-server-auto-reconnect t)
-       (erc-server-reconnect-function #'erc-server-delayed-check-reconnect)
        (expect (erc-d-t-make-expecter))
        (erc-scenarios-common-dialog "base/reconnect")
        (dumb-server nil))
@@ -89,7 +92,7 @@
         (erc-cmd-RECONNECT "cancel")
         (funcall expect 10 "canceled")))))
 
-;; In this test, a listener accepts but doesn't respond to any messages.
+;; Here, a listener accepts but doesn't respond to any messages.
 
 (ert-deftest erc-scenarios-base-auto-recon-no-proto ()
   :tags '(:expensive-test)
@@ -102,7 +105,6 @@
        (erc--server-reconnect-timeout-scale-function (lambda (_) 1))
        (erc--server-reconnect-timeout-check 0.5)
        (erc-server-auto-reconnect t)
-       (erc-server-reconnect-function #'erc-server-delayed-check-reconnect)
        (expect (erc-d-t-make-expecter)))
 
     (ert-info ("Session succeeds but cut short")
diff --git a/test/lisp/erc/erc-scenarios-base-buffer-display.el 
b/test/lisp/erc/erc-scenarios-base-buffer-display.el
index 5c3c526f86d..137db955db5 100644
--- a/test/lisp/erc/erc-scenarios-base-buffer-display.el
+++ b/test/lisp/erc/erc-scenarios-base-buffer-display.el
@@ -40,6 +40,7 @@
        (port (process-contact dumb-server :service))
        (expect (erc-d-t-make-expecter))
        (erc-server-flood-penalty 0.1)
+       (erc-server-reconnect-function #'erc-server-delayed-reconnect)
        (erc-server-auto-reconnect t)
        erc-autojoin-channels-alist)
 
diff --git a/test/lisp/erc/erc-scenarios-base-compat-rename-bouncer.el 
b/test/lisp/erc/erc-scenarios-base-compat-rename-bouncer.el
index d1124269e0a..0bba0446ce6 100644
--- a/test/lisp/erc/erc-scenarios-base-compat-rename-bouncer.el
+++ b/test/lisp/erc/erc-scenarios-base-compat-rename-bouncer.el
@@ -43,6 +43,7 @@
        (chan-buf-foo (format "#chan@127.0.0.1:%d" port))
        (chan-buf-bar (format "#chan@127.0.0.1:%d<2>" port))
        (expect (erc-d-t-make-expecter))
+       (erc-server-reconnect-function #'erc-server-delayed-reconnect)
        (erc-server-auto-reconnect auto)
        erc-server-buffer-foo erc-server-process-foo
        erc-server-buffer-bar erc-server-process-bar)
diff --git a/test/lisp/erc/erc-scenarios-base-netid-bouncer-recon-base.el 
b/test/lisp/erc/erc-scenarios-base-netid-bouncer-recon-base.el
index e5453c40e56..04ccb86643b 100644
--- a/test/lisp/erc/erc-scenarios-base-netid-bouncer-recon-base.el
+++ b/test/lisp/erc/erc-scenarios-base-netid-bouncer-recon-base.el
@@ -26,6 +26,7 @@
 
 (ert-deftest erc-scenarios-base-netid-bouncer--recon-base ()
   :tags '(:expensive-test)
-  (erc-scenarios-common--base-network-id-bouncer--reconnect nil nil))
+  (let ((erc-server-reconnect-function #'erc-server-delayed-reconnect))
+    (erc-scenarios-common--base-network-id-bouncer--reconnect nil nil)))
 
 ;;; erc-scenarios-base-netid-bouncer-recon-base.el ends here
diff --git a/test/lisp/erc/erc-scenarios-base-netid-bouncer-recon-both.el 
b/test/lisp/erc/erc-scenarios-base-netid-bouncer-recon-both.el
index 09ddcef5fa3..f0939d6204a 100644
--- a/test/lisp/erc/erc-scenarios-base-netid-bouncer-recon-both.el
+++ b/test/lisp/erc/erc-scenarios-base-netid-bouncer-recon-both.el
@@ -27,6 +27,8 @@
 
 (ert-deftest erc-scenarios-base-netid-bouncer--recon-both ()
   :tags '(:expensive-test)
-  (erc-scenarios-common--base-network-id-bouncer--reconnect 'foo-id 'bar-id))
+  (let ((erc-server-reconnect-function #'erc-server-delayed-reconnect))
+    (erc-scenarios-common--base-network-id-bouncer--reconnect 'foo-id
+                                                              'bar-id)))
 
 ;;; erc-scenarios-base-netid-bouncer-recon-both.el ends here
diff --git a/test/lisp/erc/erc-scenarios-base-netid-bouncer-recon-id.el 
b/test/lisp/erc/erc-scenarios-base-netid-bouncer-recon-id.el
index 253ab4f72c9..d77a4287976 100644
--- a/test/lisp/erc/erc-scenarios-base-netid-bouncer-recon-id.el
+++ b/test/lisp/erc/erc-scenarios-base-netid-bouncer-recon-id.el
@@ -26,11 +26,13 @@
 
 (ert-deftest erc-scenarios-base-netid-bouncer--reconnect-id-foo ()
   :tags '(:expensive-test)
-  (erc-scenarios-common--base-network-id-bouncer--reconnect 'foo-id nil))
+  (let ((erc-server-reconnect-function #'erc-server-delayed-reconnect))
+    (erc-scenarios-common--base-network-id-bouncer--reconnect 'foo-id nil)))
 
 (ert-deftest erc-scenarios-base-netid-bouncer--reconnect-id-bar ()
   :tags '(:expensive-test)
-  (erc-scenarios-common--base-network-id-bouncer--reconnect nil 'bar-id))
+  (let ((erc-server-reconnect-function #'erc-server-delayed-reconnect))
+    (erc-scenarios-common--base-network-id-bouncer--reconnect nil 'bar-id)))
 
 
 ;;; erc-scenarios-base-netid-bouncer-recon-id.el ends here
diff --git a/test/lisp/erc/erc-scenarios-base-reconnect.el 
b/test/lisp/erc/erc-scenarios-base-reconnect.el
index 6f968b9fcbc..2dc34362fbf 100644
--- a/test/lisp/erc/erc-scenarios-base-reconnect.el
+++ b/test/lisp/erc/erc-scenarios-base-reconnect.el
@@ -37,6 +37,7 @@
        (dumb-server (erc-d-run "localhost" t 'timer 'timer 'timer-last))
        (port (process-contact dumb-server :service))
        (expect (erc-d-t-make-expecter))
+       (erc-server-reconnect-function #'erc-server-delayed-reconnect)
        (erc-server-auto-reconnect t)
        erc-autojoin-channels-alist
        erc-server-buffer)
@@ -144,6 +145,7 @@
        (dumb-server (erc-d-run "localhost" t 'timer 'timer 'timer-last))
        (port (process-contact dumb-server :service))
        (expect (erc-d-t-make-expecter))
+       (erc-server-reconnect-function #'erc-server-delayed-reconnect)
        (erc-server-auto-reconnect t)
        erc-autojoin-channels-alist
        erc-server-buffer)
diff --git a/test/lisp/erc/erc-scenarios-services-misc.el 
b/test/lisp/erc/erc-scenarios-services-misc.el
index 47d0bcff41a..ee35e8b93db 100644
--- a/test/lisp/erc/erc-scenarios-services-misc.el
+++ b/test/lisp/erc/erc-scenarios-services-misc.el
@@ -155,6 +155,7 @@
        (dumb-server (erc-d-run "localhost" t 'reconnect-retry
                                'reconnect-retry-again))
        (port (process-contact dumb-server :service))
+       (erc-server-reconnect-function #'erc-server-delayed-reconnect)
        (erc-server-auto-reconnect t)
        (erc-modules `(services-regain sasl ,@erc-modules))
        (erc-services-regain-alist
diff --git a/test/lisp/erc/erc-scenarios-stamp.el 
b/test/lisp/erc/erc-scenarios-stamp.el
index 8aea091333b..86074a9c35a 100644
--- a/test/lisp/erc/erc-scenarios-stamp.el
+++ b/test/lisp/erc/erc-scenarios-stamp.el
@@ -188,6 +188,7 @@
       ((erc-scenarios-common-dialog "base/reconnect")
        (erc-server-flood-penalty 0.1)
        (erc-stamp--tz t)
+       (erc-server-reconnect-function #'erc-server-delayed-reconnect)
        (erc-server-auto-reconnect t)
        ;; Start close to midnight: 2024-06-02T23:58:11.055Z
        (erc-stamp--current-time (if (< emacs-major-version 29)