Stefan Kangas writes: > "J.P." 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. Thinking about this more, I guess it's fine if a modern ERC running on an older Emacs uses the port alone to decide whether to connect over TLS. As such, I've abandoned the whole `url-irc-function' thing in favor of adding a proper argument, as suggested. The small fraction of users with their own `url-irc-function' (if any) may feel some churn though. >> 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. Makes sense. I have thus added the missing ingredients and wired them up. >> 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. Added. > I also added some notes inline below: Much appreciated! >> From 0d191d30b15ea2d5b6042f51c6cf421b82feb7e5 Mon Sep 17 00:00:00 2001 >> From: "F. Jason Park" >> 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. Installed. >> From 6fd2f75707f123abfbcfae2d4f2837efed5b7adc Mon Sep 17 00:00:00 2001 >> From: "F. Jason Park" >> 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. Added. > What about `irc6' and `irc6s'? Should they have aliases? I guess I was trying to avoid growing lisp/loaddefs.el on account of a couple URL schemes that haven't caught on in the wild (and don't seem poised to). Still, I might as well ask around with some IRC folk just to be sure. >> From a9b47f5a6079fb3030c9e1514b4cbbda86dafff8 Mon Sep 17 00:00:00 2001 >> From: "F. Jason Park" >> 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. Right, I definitely plan on mentioning this and most other ERC changes in some fashion. >> @@ -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. I've removed the option entirely because I've come to realize it's unlikely a new user would ever set a port parameter to an IANA name in the first place (via :port, `erc-port', or whatever). And existing users accustomed to doing so obviously already know what to expect (namely, quasi-obsolete port numbers, like 194). >> From 3658e89614cbe3b5b27f09271b7bc738a1c7ec38 Mon Sep 17 00:00:00 2001 >> From: "F. Jason Park" >> Date: Mon, 11 Jul 2022 05:14:57 -0700 >> Subject: [PATCH 6/6] Improve new connections in erc-handle-irc-url > [...] >> + >> +@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. Agreed! That's basically nonsense (and so removed). >> +;; 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. ERC has always supported the auth:pass@ stuff, but my comment was confusing, so I've deleted it. Thanks so much for looking at these changes!