Package: emacs;
Reported by: "J.P." <jp <at> neverwas.me>
Date: Tue, 12 Jul 2022 08:16:01 UTC
Severity: wishlist
Found in version 29.0.50
Fixed in version 29.1
Done: "J.P." <jp <at> neverwas.me>
Bug is archived. No further changes may be made.
Message #28 received at 56514 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Kangas <stefankangas <at> gmail.com> To: "J.P." <jp <at> neverwas.me>, 56514 <at> debbugs.gnu.org Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, emacs-erc <at> gnu.org Subject: Re: bug#56514: 29.0.50; Improve ERC's URI scheme integration for irc:// links Date: Tue, 8 Nov 2022 07:16:02 -0800
"J.P." <jp <at> neverwas.me> writes:
> Questions (for anyone):
>
> 1. I added a couple autoloads in lisp/url/url-irc.el to avoid having
> to create a url-ircs.el (or even a url-irc6{,s}.el). Is there a
> better alternate means of getting `url-scheme-get-property' to
> discover handlers that doesn't rely on autoloads?
I'm hoping someone else will weigh in about this.
> 2. In the function `url-irc', I bind `url-current-object' around the
> call to `url-irc-function' to avoid adding another parameter to the
> latter's interface (which mainly benefits ERC). Any obvious problem
> with borrowing `url-current-object' for this purpose?
No real opinion here. It feels slightly cleaner to add it as a proper
argument, if we expect that other IRC clients than ERC would be
interested in its value.
> 3. In browse-url, I basically ignore what looks like the favored
> practice for adding handlers, namely, registering an internal
> function with `browse-url-default-handlers' that calls a public
> function assigned to a user option. An example of this pattern is:
>
> internal: browse-url--mailto
> option: browse-url-mailto-function
> public: browse-url-mail
>
> The reason for sidestepping the intervening indirection and adding
> a public function directly to `browse-url-default-handlers' is that
> I figure users wishing to override this can already do so via
> `browse-url-handlers'. Is that misguided somehow?
You do have a point, but I think it's better to have the user option for
consistency, and for ease of customization. Customizing an alist with
customize is always going to be harder than customizing a single-value
user option.
> 4. Are any of these non-ERC changes newsworthy enough for etc/NEWS?
I think teaching browse-url to recognize irc URLs is NEWS-worthy.
I also added some notes inline below:
> From 0d191d30b15ea2d5b6042f51c6cf421b82feb7e5 Mon Sep 17 00:00:00 2001
> From: "F. Jason Park" <jp <at> neverwas.me>
> Date: Wed, 13 Jul 2022 01:54:19 -0700
> Subject: [PATCH 1/6] Teach thing-at-point to recognize bracketed IPv6 URLs
I suggest pushing this patch so that we're sure to have it in Emacs 29.
I don't think it's NEWS-worthy, as it's more of a bug fix.
> From 6fd2f75707f123abfbcfae2d4f2837efed5b7adc Mon Sep 17 00:00:00 2001
> From: "F. Jason Park" <jp <at> neverwas.me>
> Date: Mon, 11 Jul 2022 05:14:57 -0700
> Subject: [PATCH 2/6] Accommodate ircs:// URLs in url-irc and browse-url
[...]
> +;;;; ircs://
> +
> +;; The function `url-scheme-get-property' tries and fails to load the
> +;; nonexistent url-ircs.el but falls back to using the following:
> +
> +;;;###autoload
> +(defconst url-ircs-default-port 6697 "Default port for IRCS connections.")
> +
> +;;;###autoload
> +(defalias 'url-ircs 'url-irc)
This change (support for ircs) should probably be in NEWS.
What about `irc6' and `irc6s'? Should they have aliases?
> From a9b47f5a6079fb3030c9e1514b4cbbda86dafff8 Mon Sep 17 00:00:00 2001
> From: "F. Jason Park" <jp <at> neverwas.me>
> Date: Mon, 11 Jul 2022 05:14:57 -0700
> Subject: [PATCH 4/6] Default to TLS port when calling erc-tls from lisp
>
> * lisp/erc/erc.el (erc-legacy-port-names, erc-normalize-port): Add
> standard IANA port-name mappings for 6667 and 6697, as well as an
> option to opt in for saner but nonstandard behavior.
> (erc-open): Add note to doc string explaining that params `connect'
> and `channel' are mutually exclusive.
> (erc-tls): Call `erc-compute-port' with override.
> (erc-compute-port): Call `erc-normalize-port' with result'.
>
> * test/lisp/erc/erc-tests.el (erc-tls): Add simplistic test focusing
> on default parameters.
This belongs in NEWS.
> @@ -1550,8 +1564,16 @@ erc-normalize-port
> (cond
> ((> port-nr 0)
> port-nr)
> - ((string-equal port "irc")
> - 194)
> + ((string-equal port "ircu") 6667)
> + ((string-equal port "ircs-u") 6697)
> + ((member port '("irc" "ircs"))
> + (when (eq erc-legacy-port-names 'legacy)
> + (lwarn 'ERC 'warning
> + (concat "`erc-legacy-port-names' will default to nil "
> + "in a future version of ERC.")))
Warning about the default seems unfortunate. Then every user will be
warned until they customize this.
I think we should either disable the warning, or flip the default to
nil.
> From 3658e89614cbe3b5b27f09271b7bc738a1c7ec38 Mon Sep 17 00:00:00 2001
> From: "F. Jason Park" <jp <at> neverwas.me>
> Date: Mon, 11 Jul 2022 05:14:57 -0700
> Subject: [PATCH 6/6] Improve new connections in erc-handle-irc-url
[...]
> @@ -990,6 +992,43 @@ Sample Configuration
> ;; (setq erc-kill-server-buffer-on-quit t)
> @end lisp
>
> +@node Integrations
> +@section Integrations
> +@cindex integrations
> +
> +@subheading URL
> +For anything to work, you'll want to set @code{url-irc-function} to
> +@code{url-irc-erc}. As a rule of thumb, libraries that rely directly
> +on @code{url-retrieve} should be good to go out the box from Emacs
> +29.1 onward. On older versions of Emacs, you may need to
> +@code{(require 'erc)} beforehand. @pxref{Retrieving URLs,,, url, URL}.
> +
> +For other apps and libraries, such as those relying on the
> +higher-level @code{browse-url}, you'll oftentimes be asked to specify
> +a pattern, sometimes paired with a function that accepts a string URL
> +as a first argument. For example, with EWW, you may need to tack
> +something like @code{"\\|\\`irc6?s?:"} onto the end of
> +@code{eww-use-browse-url}. But with @code{gnus-button-alist}, you'll
> +need a function as well:
> +
> +@lisp
> + '("\\birc6?s?://[][a-z0-9.,@@_:+%?&/#-]+"
> + 0 t erc-browse-url-handler 0)
> +@end lisp
> +
> +@defun erc-browse-url-handler url &rest args
> +An autoloaded convenience function for use in options like those
> +mentioned above. @var{url} must be a string. In Emacs 29 and above,
> +the function @code{browse-url-irc} can be used instead.
> +@end defun
> +
> +@noindent
> +Keep in mind that when fiddling with these options, it may be easier
> +(and more polite) to connect to a local server or a test network, like
> +@samp{ircs://testnet.ergo.chat/#test}, since these generally don't
> +require authentication.
Why would that be more polite? It seems to me that, sure, if you're
developing an IRC client I can see why you'd want to use a test network.
But it seems like overkill just for user customization.
> @@ -7184,25 +7184,98 @@ erc-get-parsed-vector-type
> ;; Teach url.el how to open irc:// URLs with ERC.
> ;; To activate, customize `url-irc-function' to `url-irc-erc'.
>
> -;; FIXME change user to nick, and use API to find server buffer
> +(defcustom erc-url-connect-function nil
> + "When non-nil, a function used to connect to an IRC URL.
> +Called with any number of keyword arguments recognized by `erc'
> +and `erc-tls'. The variable `url-current-object', if non-nil,
> +can be used to help determine whether to connect using TLS."
> + :group 'erc
> + :package-version '(ERC . "5.4.1") ; FIXME increment on release
> + :type '(choice (const nil) function))
> +
> +(defun erc--url-default-connect-function (&rest plist)
> + (let* ((scheme (and url-current-object (url-type url-current-object)))
> + (ircsp (if scheme
> + (string-suffix-p "s" scheme)
> + (or (eql 6697 (plist-get plist :port))
> + (yes-or-no-p "Connect using TLS? "))))
> + (erc-server (plist-get plist :server))
> + (erc-port (or (plist-get plist :port)
> + (and ircsp (erc-normalize-port 'ircs-u))
> + erc-port))
> + (erc-nick (or (plist-get plist :nick) erc-nick))
> + (erc-password (plist-get plist :password))
> + (args (erc-select-read-args)))
> + (unless ircsp
> + (setq ircsp (eql 6697 erc-port)))
> + (apply (if ircsp #'erc-tls #'erc) args)))
> +
> +;; The current spec, unlike the 2003 Butcher draft, doesn't explicitly
> +;; allow for an auth[:password]@ component (or trailing ,flags or
> +;; &options).
> +;;
> +;; https://www.iana.org/assignments/uri-schemes
> +;; https://datatracker.ietf.org/doc/html/draft-butcher-irc-url#section-6
> +
This is a breaking change, no? I think it should be in NEWS, even if it
is only to make ERC more standards compliant.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.