master 96785a80377 02/15: Deprecate erc-server-alist and erc-server-select

[F. Jason Park]

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

    Deprecate erc-server-alist and erc-server-select
    
    * etc/ERC-NEWS: Announce deprecation of `erc-server-alist' and
    `erc-server-select'.
    * lisp/erc/erc-networks.el: Comment out call to `erc-get' at end of
    file.
    (erc-server-alist) Change shape to accommodate a fifth member: TLS
    ports.  Add default TLS ports for Libera.Chat and OFTC.  Deprecate
    option.
    (erc-ports-list): Overload for internal use to accept a number instead
    of a list, but don't advertise this fact.
    (erc-networks--server-select): Convert `erc-server-select' into a
    function that performs the same prompting but returns a full URL or a
    host name instead of calling `erc'.
    (erc-server-select): Move to erc.el.
    * lisp/erc/erc.el (erc--prompt-for-server-functions): New variable to
    allow callers of `erc-select-read-args' to affect how server-prompting
    is handled without adding additional params.
    (erc-select-read-args): Defer to `erc--prompt-for-server-function'
    when non-nil.
    (erc-server-select): New transplanted function, a deprecated, now
    TSL-aware version of the old quirky entry point from erc-networks.el.
    Reimplemented as a simple wrapper for `erc'.
    * test/lisp/erc/erc-networks-tests.el (erc-ports-list): New test.
    * test/lisp/erc/erc-tests.el (erc-server-select): New test.
    (Bug#64478)
---
 etc/ERC-NEWS                        | 16 ++++++++
 lisp/erc/erc-networks.el            | 79 +++++++++++++++++++++----------------
 lisp/erc/erc.el                     | 16 +++++++-
 test/lisp/erc/erc-networks-tests.el | 18 +++++++++
 test/lisp/erc/erc-tests.el          | 35 ++++++++++++++++
 5 files changed, 128 insertions(+), 36 deletions(-)

diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS
index 5665b760ea9..808d7dcb64f 100644
--- a/etc/ERC-NEWS
+++ b/etc/ERC-NEWS
@@ -75,6 +75,22 @@ the 'log' module may want to customize 
'erc-log-filter-function' to
 'erc-stamp-prefix-log-filter' to avoid ragged right-hand stamps
 appearing in their saved logs.
 
+** Awkward entry point 'erc-server-select' improved but deprecated.
+The alternate entry point 'erc-server-select' has mainly served to
+confuse users in more recent years because it requires certain
+options, like 'erc-nick', to be configured ahead of time, and it
+doesn't support TLS.  Its main selling point, historically, has been
+interactive completion based on the option 'erc-server-alist', which
+is a table of networks, servers, and ports.  But most of the option's
+400-odd entries are sadly defunct or otherwise outdated.  And, these
+days, most networks promote a well known load-balancing end point over
+individual servers anyway.  Regardless, the command has now been
+improved to prompt for the same slate of parameters sought by
+'erc-tls'.  Similarly, 'erc-server-alist' entries now support a fifth
+member in TLS ports (though this option too has been deprecated).  If
+you feel these deprecations rash or unwarranted, please file a bug
+report and petition the maintainers for a reprieve.
+
 ** Smarter reconnect handling for users on the move.
 ERC now offers a new, experimental reconnect strategy in the function
 'erc-server-delayed-check-reconnect', which tests for underlying
diff --git a/lisp/erc/erc-networks.el b/lisp/erc/erc-networks.el
index dd481032e7e..7cc64614573 100644
--- a/lisp/erc/erc-networks.el
+++ b/lisp/erc/erc-networks.el
@@ -29,8 +29,6 @@
 ;;
 ;; This is the "networks" module.
 ;;
-;; M-x erc-server-select provides an alternative way to connect to servers by
-;; choosing networks.
 ;; You can use (eq (erc-network) 'Network) if you'd like to set variables or do
 ;; certain actions according to which network you're connected to.
 ;; If a network you use is not listed in `erc-networks-alist', you can put
@@ -258,6 +256,7 @@
   ("IRChat: Random server" IRChat "irc.irchat.net" ((6660 6669)))
   ("IrcLordz: Random server" IrcLordz "irc.irclordz.com" 6667)
   ("IrcMalta: Random server" IrcMalta "irc.ircmalta.org" ((6660 6667)))
+  ;; This one is dead but used in testing.  Please retain.
   ("IRCnet: EU, FR, Random" IRCnet "irc.fr.ircnet.net" 6667)
   ("IRCnet: EU, IT, Random" IRCnet "irc.ircd.it" ((6665 6669)))
   ("IRCnet: AS, IL, Haifa" IRCnet "ircnet.netvision.net.il" ((6661 6668)))
@@ -318,13 +317,15 @@
   ("LagNet: Random server" LagNet "irc.lagnet.org.za" 6667)
   ("LagNet: AF, ZA, Cape Town" LagNet "reaper.lagnet.org.za" 6667)
   ("LagNet: AF, ZA, Johannesburg" LagNet "mystery.lagnet.org.za" 6667)
-  ("Libera.Chat: Random server" Libera.Chat "irc.libera.chat" 6667)
-  ("Libera.Chat: Random Europe server" Libera.Chat "irc.eu.libera.chat" 6667)
-  ("Libera.Chat: Random US & Canada server" Libera.Chat "irc.us.libera.chat" 
6667)
-  ("Libera.Chat: Random Australia & New Zealand server" Libera.Chat 
"irc.au.libera.chat" 6667)
-  ("Libera.Chat: Random East Asia server" Libera.Chat "irc.ea.libera.chat" 
6667)
-  ("Libera.Chat: IPv4 only server" Libera.Chat "irc.ipv4.libera.chat" 6667)
-  ("Libera.Chat: IPv6 only server" Libera.Chat "irc.ipv6.libera.chat" 6667)
+  ("Libera.Chat: Random server" Libera.Chat "irc.libera.chat"
+   ((6665 6667) (8000 8002)) (6697 7000 7070))
+  ;; If not deprecating this option, use ^ for the rest of these servers.
+  ("Libera.Chat: Random Europe server" Libera.Chat "irc.eu.libera.chat" 6667 
6697)
+  ("Libera.Chat: Random US & Canada server" Libera.Chat "irc.us.libera.chat" 
6667 6697)
+  ("Libera.Chat: Random Australia & New Zealand server" Libera.Chat 
"irc.au.libera.chat" 6667 6697)
+  ("Libera.Chat: Random East Asia server" Libera.Chat "irc.ea.libera.chat" 
6667 6697)
+  ("Libera.Chat: IPv4 only server" Libera.Chat "irc.ipv4.libera.chat" 6667 
6697)
+  ("Libera.Chat: IPv6 only server" Libera.Chat "irc.ipv6.libera.chat" 6667 
6697)
   ("Librenet: Random server" Librenet "irc.librenet.net" 6667)
   ("LinkNet: Random server" LinkNet "irc.link-net.org" ((6667 6669)))
   ("LinuxChix: Random server" LinuxChix "irc.linuxchix.org" 6667)
@@ -349,7 +350,7 @@
   ("Novernet: Random server" Novernet "irc.novernet.com" ((6665 6669) 7000 ))
   ("Nullrouted: Random server" Nullrouted "irc.nullrouted.org" ((6666 6669) 
7000 ))
   ("NullusNet: Random server" NullusNet "irc.nullus.net" 6667)
-  ("OFTC: Random server" OFTC "irc.oftc.net" ((6667 6670) 7000))
+  ("OFTC: Random server" OFTC "irc.oftc.net" ((6667 6670) 7000) (6697 9999))
   ("OpChat: Random server" OpChat "irc.opchat.org" ((6667 6669)))
   ("Othernet: Random server" Othernet "irc.othernet.org" 6667)
   ("Othernet: US, FL, Miami" Othernet "miami.fl.us.othernet.org" 6667)
@@ -472,12 +473,13 @@
   ("ZUHnet: Random server" ZUHnet "irc.zuh.net" 6667)
   ("Zurna: Random server" Zurna "irc.zurna.net" 6667))
   "Alist of irc servers.
-Each server is a list (NAME NET HOST PORTS) where
+Each server is a list (NAME NET HOST PORTS TLS-PORTS) where
 NAME is a name for that server,
 NET is a symbol indicating to which network from `erc-networks-alist'
   this server corresponds,
-HOST is the servers hostname and
-PORTS is either a number, a list of numbers, or a list of port ranges."
+HOST is the server's hostname, and (TLS-)PORTS is either a
+number, a list of numbers, or a list of port ranges."
+  :package-version '(ERC . "5.6") ; FIXME sync on release
   :type '(alist :key-type (string :tag "Name")
                :value-type
                (group symbol (string :tag "Hostname")
@@ -486,7 +488,15 @@ PORTS is either a number, a list of numbers, or a list of 
port ranges."
                               (repeat :tag "List of ports or ranges"
                                       (choice (integer :tag "Port number")
                                               (list :tag "Port range"
-                                                    integer integer)))))))
+                                                    integer integer))))
+                       (choice :tag "TLS ports"
+                               (integer :tag "TLS port number")
+                               (repeat :tag "List of TLS ports or ranges"
+                                       (choice (integer :tag "TLS port number")
+                                               (list :tag "TLS port range"
+                                                     integer integer)))))))
+(make-obsolete-variable 'erc-server-alist
+                        "specify `:server' with `erc-tls'." "30.1")
 
 (defcustom erc-networks-alist
   '((4-irc "4-irc.com")
@@ -1535,7 +1545,7 @@ As an example:
   (erc-ports-list \\='((1 5))) => (1 2 3 4 5)
   (erc-ports-list \\='(1 (3 5))) => (1 3 4 5)"
   (let (result)
-    (dolist (p ports)
+    (dolist (p (ensure-list ports))
       (cond ((numberp p)
             (push p result))
            ((listp p)
@@ -1544,31 +1554,32 @@ As an example:
                                 result)))))
     (nreverse result)))
 
-;;;###autoload
-(defun erc-server-select ()
-  "Interactively select a server to connect to using `erc-server-alist'."
-  (interactive)
+(defun erc-networks--server-select ()
+  "Prompt for a server in `erc-server-alist' and return its irc(s):// URL.
+Choose port at random if multiple candidates exist, but always
+prefer TLS without asking.  When a port can't be determined,
+return the host alone sans URL formatting (for compatibility)."
   (let* ((completion-ignore-case t)
         (net (intern
               (completing-read "Network: "
                                (delete-dups
                                 (mapcar (lambda (x)
-                                          (list (symbol-name (nth 1 x))))
+                                           (list (nth 1 x)))
                                         erc-server-alist)))))
-        (srv (assoc
-              (completing-read "Server: "
-                               (delq nil
-                                     (mapcar (lambda (x)
-                                               (when (equal (nth 1 x) net)
-                                                 x))
-                                             erc-server-alist)))
-              erc-server-alist))
+         (s-choose (lambda (entry)
+                     (and (equal (nth 1 entry) net)
+                          (if-let ((b (string-search ": " (car entry))))
+                              (cons (format "%s (%s)" (nth 2 entry)
+                                            (substring (car entry) (+ b 2)))
+                                    (cdr entry))
+                            entry))))
+         (s-entries (delq nil (mapcar s-choose erc-server-alist)))
+         (srv (assoc (completing-read "Server: " s-entries) s-entries))
         (host (nth 2 srv))
-        (ports (if (listp (nth 3 srv))
-                   (erc-ports-list (nth 3 srv))
-                 (list (nth 3 srv))))
-         (port (and ports (seq-random-elt ports))))
-    (erc :server host :port port)))
+         (pspec (nthcdr 3 srv))
+         (ports (erc-ports-list (or (cadr pspec) (car pspec))))
+         (scheme (if (cdr pspec) "ircs" "irc")))
+    (if ports (format "%s://%s:%d" scheme host (seq-random-elt ports)) host)))
 
 ;;; The following experimental
 ;; It does not work yet, help me with it if you
@@ -1605,7 +1616,7 @@ VALUE is the options value.")
                     items nil)))))
     val))
 
-(erc-get 'pals 'Libera.Chat)
+;; (erc-get 'pals 'Libera.Chat)
 
 (provide 'erc-networks)
 
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 1786c8924bd..7693947873e 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -2354,13 +2354,17 @@ parameters SERVER and NICK."
     (setq input (concat "irc://" input)))
   input)
 
+(defvar erc--prompt-for-server-function nil)
+
 ;;;###autoload
 (defun erc-select-read-args ()
   "Prompt the user for values of nick, server, port, and password.
 With prefix arg, also prompt for user and full name."
   (let* ((input (let ((d (erc-compute-server)))
-                  (read-string (format "Server or URL (default is %S): " d)
-                               nil 'erc-server-history-list d)))
+                  (if erc--prompt-for-server-function
+                      (funcall erc--prompt-for-server-function)
+                    (read-string (format "Server or URL (default is %S): " d)
+                                 nil 'erc-server-history-list d))))
          ;; For legacy reasons, also accept a URL without a scheme.
          (url (url-generic-parse-url (erc--ensure-url input)))
          (server (url-host url))
@@ -2419,6 +2423,14 @@ With prefix arg, also prompt for user and full name."
        (cl-progv ,syms ,vals
          ,@body))))
 
+;;;###autoload
+(defun erc-server-select ()
+  "Interactively connect to a server from `erc-server-alist'."
+  (declare (obsolete erc-tls "30.1"))
+  (interactive)
+  (let ((erc--prompt-for-server-function #'erc-networks--server-select))
+    (call-interactively #'erc)))
+
 ;;;###autoload
 (cl-defun erc (&key (server (erc-compute-server))
                     (port   (erc-compute-port))
diff --git a/test/lisp/erc/erc-networks-tests.el 
b/test/lisp/erc/erc-networks-tests.el
index f0fcbbc81c6..e95d99c128f 100644
--- a/test/lisp/erc/erc-networks-tests.el
+++ b/test/lisp/erc/erc-networks-tests.el
@@ -1750,4 +1750,22 @@
     (should (eq (erc-networks--determine)
                 erc-networks--name-missing-sentinel))))
 
+(ert-deftest erc-ports-list ()
+  (with-suppressed-warnings ((obsolete erc-server-alist))
+    (let* ((srv (assoc "Libera.Chat: Random server" erc-server-alist)))
+      (should (equal (erc-ports-list (nth 3 srv))
+                     '(6665 6666 6667 8000 8001 8002)))
+      (should (equal (erc-ports-list (nth 4 srv))
+                     '(6697 7000 7070))))
+
+    (let* ((srv (assoc "Libera.Chat: Random Europe server" erc-server-alist)))
+      (should (equal (erc-ports-list (nth 3 srv)) '(6667)))
+      (should (equal (erc-ports-list (nth 4 srv)) '(6697))))
+
+    (let* ((srv (assoc "OFTC: Random server" erc-server-alist)))
+      (should (equal (erc-ports-list (nth 3 srv))
+                     '(6667 6668 6669 6670 7000)))
+      (should (equal (erc-ports-list (nth 4 srv))
+                     '(6697 9999))))))
+
 ;;; erc-networks-tests.el ends here
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index f5c900df408..449b8e0df42 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1703,6 +1703,41 @@
                          (erc-server-connect-function
                           erc-open-network-stream))))))))
 
+(ert-deftest erc-server-select ()
+  (let (calls env)
+    (cl-letf (((symbol-function 'user-login-name)
+               (lambda (&optional _) "tester"))
+              ((symbol-function 'erc-open)
+               (lambda (&rest r)
+                 (push `((erc-join-buffer ,erc-join-buffer)
+                         (erc-server-connect-function
+                          ,erc-server-connect-function))
+                       env)
+                 (push r calls))))
+
+      (ert-info ("Selects Libera.Chat Europe, automatic TSL")
+        (ert-simulate-keys "Libera.Chat\rirc.eu.\t\r\r\r"
+          (with-suppressed-warnings ((obsolete erc-server-select))
+            (call-interactively #'erc-server-select)))
+        (should (equal (pop calls)
+                       '("irc.eu.libera.chat" 6697 "tester" "unknown" t nil
+                         nil nil nil nil "user" nil)))
+        (should (equal (pop env)
+                       '((erc-join-buffer window)
+                         (erc-server-connect-function erc-open-tls-stream)))))
+
+      (ert-info ("Selects entry that doesn't support TLS")
+        (ert-simulate-keys "IRCnet\rirc.fr.\t\rdummy\r\r"
+          (with-suppressed-warnings ((obsolete erc-server-select))
+            (call-interactively #'erc-server-select)))
+        (should (equal (pop calls)
+                       '("irc.fr.ircnet.net" 6667 "dummy" "unknown" t nil
+                         nil nil nil nil "user" nil)))
+        (should (equal (pop env)
+                       '((erc-join-buffer window)
+                         (erc-server-connect-function
+                          erc-open-network-stream))))))))
+
 (defun erc-tests--make-server-buf (name)
   (with-current-buffer (get-buffer-create name)
     (erc-mode)