Re: bug-reference.el: Allow custom handlers for opening URLs

[Tassilo Horn <tsdh@gnu.org>]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Yes, that's probably even better/more generic.  But again, I wouldn't
>> want to hard-code that, so I guess we'd want some
>> `browse-url-handlers' in browse-url.el doing basically the same as
>> the `bug-reference-url-handlers' I proposed, right?
>
> Yes.  Hopefully that can be used for other purposes such as
> redirecting some URLs to EWW?

Sure, just add an entry like:

  ("http://\\(www\\.\\)?gnu\\.org\\b" . eww)

>> Looking at it, there're already some special handlers for mailto: and
>> man: page URLs.  Should I convert those to default entries of the new
>> `browse-url-handlers' alist?  (IMHO, yes.)
>
> I think that would make sense, yes.

Done so.

>> And should the presence of `debbugs-gnu-bugs' create an entry in the
>> alist, i.e., should having debbugs installed be enough to open bug
>> links with debbugs or should we still be using the browser?
>
> You mean, should the debbugs package come with autoloads that add
> themselves to this new variable?  That's up to the debbugs package
> maintainers, I'd say ;-)

Right, that's their job.

I've implemented it on the feature/browse-url-handlers branch (patch
also attached below).  Please, could you have a look?  (I've tested man,
mailto, the special gnu.org-eww handler, and web URLs matching no entry
and it worked right.)

And when hacking that up, I've seen that this feature is already
implemented!

The value of `browse-url-browser-function' may already be a (REGEXP
. FUNCTION) alist itself and then it works the same way as my new
`browse-url-handlers'.  But I think that this is less flexible design as
it doesn't allow other packages to easily hook in those mechanics (in
the sense of Yuri's mobile application metaphor which fits well, I
think).

I would like to remove that entry from
`browse-url--browser-defcustom-type' and issue a warning in `browse-url'
pointing to the new `browse-url-handlers' when function is a dotted pair
(while still supporting that usage for the next 20 years, of course).

But removing the alist from `browse-url--browser-defcustom-type' would
break the customize interface if the user's value is indeed an alist...
So is there a better way to deprecate a single choice of a defcustom
type?  Just writing that in the docstring?

Here's the patch:

--8<---------------cut here---------------start------------->8---
From 8046f46252b5e23a9fa157660efb5c6fc0ea82e2 Mon Sep 17 00:00:00 2001
From: Tassilo Horn <tsdh@gnu.org>
Date: Mon, 4 May 2020 11:24:08 +0200
Subject: [PATCH] Allow for custom URL handlers in browse-url.

* lisp/net/browse-url.el (browse-url-handlers): New defcustom.
(browse-url): Use it.
---
 lisp/net/browse-url.el | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/lisp/net/browse-url.el b/lisp/net/browse-url.el
index 7aad44b287..ab0be7b913 100644
--- a/lisp/net/browse-url.el
+++ b/lisp/net/browse-url.el
@@ -175,6 +175,23 @@ browse-url-browser-function
   :type browse-url--browser-defcustom-type
   :version "24.1")
 
+;;;#autoload
+(defcustom browse-url-handlers
+  `(("\\`mailto:" . ,browse-url-mailto-function)
+    ("\\`man:" . ,browse-url-man-function))
+  "An alist with elements of the form (REGEXP HANDLER).
+Each REGEXP is matched against each URL to be opened in turn and
+the first match's HANDLER is invoked with the URL.
+
+A HANDLER must either be a function with the same arguments as
+`browse-url' or a variable whos value is such a function.
+
+If no REGEXP matches, the bug reference URL is opened using the
+value of `browse-url-browser-function'."
+  :type '(alist :key-type (regexp :tag "Regexp")
+                :value-type (function :tag "Handler"))
+  :version "28.1")
+
 (defcustom browse-url-secondary-browser-function 'browse-url-default-browser
   "Function used to launch an alternative browser.
 This is usually an external browser (that is, not eww or w3m),
@@ -786,12 +803,14 @@ browse-url
              (not (string-match "\\`[a-z]+:" url)))
     (setq url (expand-file-name url)))
   (let ((process-environment (copy-sequence process-environment))
-	(function (or (and (string-match "\\`mailto:" url)
-			   browse-url-mailto-function)
-                      (and (string-match "\\`man:" url)
-                           browse-url-man-function)
-		      browse-url-browser-function))
-	;; Ensure that `default-directory' exists and is readable (b#6077).
+	(function
+         (catch 'custom-url-handler
+           (dolist (regex-handler browse-url-handlers)
+             (when (string-match-p (car regex-handler) url)
+               (throw 'custom-url-handler (cdr regex-handler))))
+           ;; No special handler found.
+           browse-url-browser-function))
+	;; Ensure that `default-directory' exists and is readable (bug#6077).
 	(default-directory (or (unhandled-file-name-directory default-directory)
 			       (expand-file-name "~/"))))
     ;; When connected to various displays, be careful to use the display of
-- 
2.26.2
--8<---------------cut here---------------end--------------->8---

Bye,
Tassilo