From 8383193362db77d814747d9f2074fe514359e76c Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Fri, 29 Nov 2024 15:56:47 -0800
Subject: [PATCH] [5.6.1] Add 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'.
* 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-known-check-aware-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
still incorporates the behavior under test that's exhibited by
`erc-server-delayed-check-reconnect'.
* 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.  (Bug#62044)
---
 doc/misc/erc.texi                             | 37 ++++++-------------
 etc/ERC-NEWS                                  |  6 +++
 lisp/erc/erc-backend.el                       | 21 +++++++++--
 .../lisp/erc/erc-scenarios-base-auto-recon.el |  8 ++--
 .../erc/erc-scenarios-base-buffer-display.el  |  1 +
 ...rc-scenarios-base-compat-rename-bouncer.el |  1 +
 ...scenarios-base-netid-bouncer-recon-base.el |  3 +-
 ...scenarios-base-netid-bouncer-recon-both.el |  4 +-
 ...c-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, 56 insertions(+), 35 deletions(-)

diff --git a/doc/misc/erc.texi b/doc/misc/erc.texi
index 0f6b6b8c5be..d7791c04357 100644
--- a/doc/misc/erc.texi
+++ b/doc/misc/erc.texi
@@ -1340,8 +1340,7 @@ Sample Configuration
   ;; 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 @@ Sample Configuration
 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}
+@samp{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 persists 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 e72fa036f17..0d39f3949f9 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -429,15 +429,16 @@ erc-server-reconnect-timeout
 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 @@ erc-server-delayed-check-reconnect
              (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,20 @@ erc-server-delayed-check-reconnect
               (set-process-sentinel proc sentinel))
           (file-error (funcall reschedule nil)))))))
 
+(defvar erc-server-known-check-aware-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'.
+When it's a member of `erc-server-known-check-aware-connectors', prefer
+`erc-server-delayed-check-reconnect'.  Otherwise, use
+`erc-server-delayed-reconnect'."
+  (if (memq (buffer-local-value 'erc-session-connector buffer)
+            erc-server-known-check-aware-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 @@ erc-scenarios-base-auto-recon-unavailable
        (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-scenarios-base-auto-recon-unavailable
         (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-scenarios-base-auto-recon-no-proto
        (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 @@ erc-scenarios-base-buffer-display--reconnect-common
        (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 @@ erc-scenarios-common--base-compat-no-rename-bouncer
        (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 @@ erc-scenarios-base-reconnect-timer
        (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 @@ erc-scenarios-base-cancel-reconnect
        (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 @@ erc-scenarios-services-misc--reconnect-retry-nick
        (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-stamp--date-mode/reconnect
       ((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)
-- 
2.47.0