unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* bug-reference.el: Allow custom handlers for opening URLs
@ 2020-05-03  8:50 Tassilo Horn
  2020-05-03 14:44 ` Stefan Monnier
  2020-05-05 19:55 ` Arash Esbati
  0 siblings, 2 replies; 15+ messages in thread
From: Tassilo Horn @ 2020-05-03  8:50 UTC (permalink / raw)
  To: emacs-devel

Hi all,

currently all bug references are opened using `browse-url'.  I'd like to
have a way to override that behavior, e.g., so that I can use the
debbugs package for reports for GNU packages using the debbugs tracker.

Below is a patch which makes that possible.  Is it ok to commit that?
Something to improve?

Oh, and how do I correctly write #'my-function in a docstring so that
the ' doesn't get displayed differently?

--8<---------------cut here---------------start------------->8---
From 8450db635e52a56addccd435bc03449d5caa0a2b Mon Sep 17 00:00:00 2001
From: Tassilo Horn <tsdh@gnu.org>
Date: Sun, 3 May 2020 10:39:47 +0200
Subject: [PATCH] Allow customizing how bug reference URLs are opened.

* lisp/progmodes/bug-reference.el (bug-reference-url-handlers): New
defcustom.
(bug-reference-push-button): Use it.
(bug-reference-open-with-debbugs): New function.
---
 lisp/progmodes/bug-reference.el | 36 +++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/bug-reference.el b/lisp/progmodes/bug-reference.el
index 02af263ec3..cc862eb160 100644
--- a/lisp/progmodes/bug-reference.el
+++ b/lisp/progmodes/bug-reference.el
@@ -122,6 +122,31 @@ bug-reference-fontify
                                        (match-string-no-properties 2))
                              (funcall bug-reference-url-format))))))))))
 
+(defcustom bug-reference-url-handlers nil
+  "An alist with elements of the form (REGEXP HANDLER).
+Each REGEXP is matched against a bug reference URL in turn and
+the first match's HANDLER function is invoked with the URL.
+
+If no REGEXP matches, the bug reference URL is opened using
+`browse-url'.
+
+For example, to open GNU bug reports using the debbugs ELPA
+package, you could use an entry like this.
+
+  (\"https://debbugs.gnu.org/cgi/bugreport\\\\.cgi\"
+   . #'bug-reference-open-with-debbugs)"
+  :type '(alist :key-type (regexp :tag "Regexp")
+                :value-type (function :tag "Handler"))
+  :version "28.1"
+  :group 'bug-reference)
+
+(defun bug-reference-open-with-debbugs (url)
+  (unless (fboundp #'debbugs-gnu-bugs)
+    (error "The debbugs package is not installed"))
+  (if (string-match "bug=\\([0-9]+\\)" url)
+      (debbugs-gnu-bugs (string-to-number (match-string 1 url)))
+    (error "The URL %s contains no bug number" url)))
+
 ;; Taken from button.el.
 (defun bug-reference-push-button (&optional pos _use-mouse-action)
   "Open URL corresponding to the bug reference at POS."
@@ -135,9 +160,16 @@ bug-reference-push-button
     ;; POS is just normal position.
     (dolist (o (overlays-at pos))
       ;; It should only be possible to have one URL overlay.
-      (let ((url (overlay-get o 'bug-reference-url)))
+      (let ((url (overlay-get o 'bug-reference-url))
+            handler)
 	(when url
-	  (browse-url url))))))
+          (setq handler
+                (catch 'handler-set
+                  (dolist (regex-handler bug-reference-url-handlers)
+                    (when (string-match-p (car regex-handler) url)
+                      (throw 'handler-set (cdr regex-handler))))
+                  #'browse-url))
+	  (funcall handler url))))))
 
 ;;;###autoload
 (define-minor-mode bug-reference-mode
-- 
2.26.2

--8<---------------cut here---------------end--------------->8---

Bye,
Tassilo



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: bug-reference.el: Allow custom handlers for opening URLs
  2020-05-03  8:50 bug-reference.el: Allow custom handlers for opening URLs Tassilo Horn
@ 2020-05-03 14:44 ` Stefan Monnier
  2020-05-03 15:24   ` Tassilo Horn
  2020-05-05 19:55 ` Arash Esbati
  1 sibling, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2020-05-03 14:44 UTC (permalink / raw)
  To: emacs-devel

> currently all bug references are opened using `browse-url'.  I'd like to
> have a way to override that behavior, e.g., so that I can use the
> debbugs package for reports for GNU packages using the debbugs tracker.
>
> Below is a patch which makes that possible.  Is it ok to commit that?
> Something to improve?

How 'bout hooking into browse-url instead, so that all uses of
browse-url for debbugs URLs would be redirected to the debbugs package?


        Stefan




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: bug-reference.el: Allow custom handlers for opening URLs
  2020-05-03 14:44 ` Stefan Monnier
@ 2020-05-03 15:24   ` Tassilo Horn
  2020-05-03 20:39     ` Stefan Monnier
  2020-05-04  6:52     ` bug-reference.el: Allow custom handlers for opening URLs Yuri Khan
  0 siblings, 2 replies; 15+ messages in thread
From: Tassilo Horn @ 2020-05-03 15:24 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

>> currently all bug references are opened using `browse-url'.  I'd like
>> to have a way to override that behavior, e.g., so that I can use the
>> debbugs package for reports for GNU packages using the debbugs
>> tracker.
>>
>> Below is a patch which makes that possible.  Is it ok to commit that?
>> Something to improve?
>
> How 'bout hooking into browse-url instead, so that all uses of
> browse-url for debbugs URLs would be redirected to the debbugs
> package?

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?

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.)

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?

Bye,
Tassilo



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: bug-reference.el: Allow custom handlers for opening URLs
  2020-05-03 15:24   ` Tassilo Horn
@ 2020-05-03 20:39     ` Stefan Monnier
  2020-05-04  9:41       ` Tassilo Horn
  2020-05-04  6:52     ` bug-reference.el: Allow custom handlers for opening URLs Yuri Khan
  1 sibling, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2020-05-03 20:39 UTC (permalink / raw)
  To: emacs-devel

> 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?

> 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.

> 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 ;-)


        Stefan




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: bug-reference.el: Allow custom handlers for opening URLs
  2020-05-03 15:24   ` Tassilo Horn
  2020-05-03 20:39     ` Stefan Monnier
@ 2020-05-04  6:52     ` Yuri Khan
  1 sibling, 0 replies; 15+ messages in thread
From: Yuri Khan @ 2020-05-04  6:52 UTC (permalink / raw)
  To: Stefan Monnier, Emacs developers

On Sun, 3 May 2020 at 22:25, Tassilo Horn <tsdh@gnu.org> wrote:

> 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?
>
> 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.)

This is very similar to what applications on mobile systems do (both
Android and iOS). The colloquial name is “deeplinking”. Looking from
one angle, it lets you exchange links to various screens inside an
application as if they were pages on the web. From another angle, it
lets application authors to provide a generic implementation on the
web (so that desktop users are served), and substitute a more capable
implementation via an app.

Basically, an application declares to the system that it is capable of
handling URLs with this or that scheme, and/or URLs referring to this
or that domain. After that, if any application (including the system
web browser) signals an intent to open a URL that matches these
conditions, the system offers the user a choice which handler to use,
with an option to remember this choice for later occasions.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: bug-reference.el: Allow custom handlers for opening URLs
  2020-05-03 20:39     ` Stefan Monnier
@ 2020-05-04  9:41       ` Tassilo Horn
  2020-05-04 15:32         ` Stefan Monnier
  0 siblings, 1 reply; 15+ messages in thread
From: Tassilo Horn @ 2020-05-04  9:41 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Yuri Khan, emacs-devel

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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: bug-reference.el: Allow custom handlers for opening URLs
  2020-05-04  9:41       ` Tassilo Horn
@ 2020-05-04 15:32         ` Stefan Monnier
  2020-05-04 17:09           ` Tassilo Horn
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2020-05-04 15:32 UTC (permalink / raw)
  To: emacs-devel; +Cc: Yuri Khan

> 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'.

Hmm... right.  Ideally we should have 2 places to specify those
redirections: one autoload-set by packages to change the default
behavior, and one custom-set by the user to override that
default behavior.

But if we use `browse-url-browser-function` as the user-override, then
as soon as the user sets it to `browse-url-firefox` it would override
all the defaults :-(

So we still need to deprecate this old hack on `browse-url-browser-function`.

> 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?

Good question, hopefully someone else knows.


        Stefan




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: bug-reference.el: Allow custom handlers for opening URLs
  2020-05-04 15:32         ` Stefan Monnier
@ 2020-05-04 17:09           ` Tassilo Horn
  2020-05-04 18:54             ` Stefan Monnier
  0 siblings, 1 reply; 15+ messages in thread
From: Tassilo Horn @ 2020-05-04 17:09 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

>> 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'.
>
> Hmm... right.  Ideally we should have 2 places to specify those
> redirections: one autoload-set by packages to change the default
> behavior, and one custom-set by the user to override that default
> behavior.

So you mean we need to have a `browse-url-handlers' defcustom and a
`browse-url-handlers-default' defvar?  Maybe you are right.  On the
other hand, we haven't something like that for similar candidates like
`auto-mode-alist', too.  There, we use the "if you don't like it, remove
it yourself" approach.

Another way would be to cross fingers and hope that package authors are
responsible enough to provide a defcustom which controls if a handler is
added to `browse-url-handlers'.  That would be much more convenient as
both adding an override and removing an entry.

> But if we use `browse-url-browser-function` as the user-override, then
> as soon as the user sets it to `browse-url-firefox` it would override
> all the defaults :-(
>
> So we still need to deprecate this old hack on
> `browse-url-browser-function`.

Right.

>> 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?
>
> Good question, hopefully someone else knows.

Hm, the info docs say that every custom type may have a :doc string:

--8<---------------cut here---------------start------------->8---
‘:doc DOC’
     Use DOC as the documentation string for this value (or part of the
     value) that corresponds to this type.  In order for this to work,
     you must specify a value for ‘:format’, and use ‘%d’ or ‘%h’ in
     that value.

     The usual reason to specify a documentation string for a type is to
     provide more information about the meanings of alternatives inside
     a ‘:choice’ type or the parts of some other composite type.
--8<---------------cut here---------------end--------------->8---

That second paragraph sounds exactly like what we would need, so I added

  :doc "Deprecated.  Use `browse-url-handlers' instead."

to the alist choice.  However, I cannot see that in the customize
interface.  Am I doing something wrong?

And another question.  The definition below breaks the build.

--8<---------------cut here---------------start------------->8---
;;;###autoload ;; FIXME: This autoload breaks the build...
(defcustom browse-url-handlers
  `(("\\`mailto:" . ,browse-url-mailto-function)
    ("\\`man:" . ,browse-url-man-function)
    ("\\`file://" . browse-url-emacs))
  "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")
--8<---------------cut here---------------end--------------->8---

The error is:

--8<---------------cut here---------------start------------->8---
Loading files...
Loading emacs-lisp/macroexp...
Loading cus-face...
Loading faces...
Loading button...
Loading loaddefs.el (source)...
Symbol's value as variable is void: browse-url-mailto-function
make[2]: *** [Makefile:584: emacs.pdmp] Error 255
make[2]: Leaving directory '/home/horn/Repos/el/emacs/src'
make[1]: *** [Makefile:424: src] Error 2
make[1]: Leaving directory '/home/horn/Repos/el/emacs'
make: *** [Makefile:1119: bootstrap] Error 2
--8<---------------cut here---------------end--------------->8---

When removing the autoload cookie, it builds fine.  Why is that?
`browse-url-mailto-function' and `browse-url-man-function' are defined
way before `browse-url-handlers'.

Bye,
Tassilo



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: bug-reference.el: Allow custom handlers for opening URLs
  2020-05-04 17:09           ` Tassilo Horn
@ 2020-05-04 18:54             ` Stefan Monnier
  2020-05-05  7:06               ` Tassilo Horn
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2020-05-04 18:54 UTC (permalink / raw)
  To: emacs-devel

> So you mean we need to have a `browse-url-handlers' defcustom and a
> `browse-url-handlers-default' defvar?  Maybe you are right.  On the
> other hand, we haven't something like that for similar candidates like
> `auto-mode-alist', too.  There, we use the "if you don't like it, remove
> it yourself" approach.

Good point.  In that case the solution is to forego the use of
`defcustom`.  I'm fine with that.

After all, the autoloads are supposed to run before the .emacs, so the
user can `add-to-list` just fine to override whatever was added by autoloads.

> Symbol's value as variable is void: browse-url-mailto-function
> make[2]: *** [Makefile:584: emacs.pdmp] Error 255
> make[2]: Leaving directory '/home/horn/Repos/el/emacs/src'
> make[1]: *** [Makefile:424: src] Error 2
> make[1]: Leaving directory '/home/horn/Repos/el/emacs'
> make: *** [Makefile:1119: bootstrap] Error 2
> --8<---------------cut here---------------end--------------->8---
>
> When removing the autoload cookie, it builds fine.  Why is that?
> `browse-url-mailto-function' and `browse-url-man-function' are defined
> way before `browse-url-handlers'.

But they're not autoloaded, so in the loaddefs.el file, you have
a reference to it without the preceding definition.

Maybe the better option is to define (and use) a new function
`browse-url-mailto` (or some other name) which just funcalls
`browse-url-mailto-function`.


        Stefan




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: bug-reference.el: Allow custom handlers for opening URLs
  2020-05-04 18:54             ` Stefan Monnier
@ 2020-05-05  7:06               ` Tassilo Horn
  2020-05-05 13:49                 ` Stefan Monnier
  0 siblings, 1 reply; 15+ messages in thread
From: Tassilo Horn @ 2020-05-05  7:06 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

>> So you mean we need to have a `browse-url-handlers' defcustom and a
>> `browse-url-handlers-default' defvar?  Maybe you are right.  On the
>> other hand, we haven't something like that for similar candidates
>> like `auto-mode-alist', too.  There, we use the "if you don't like
>> it, remove it yourself" approach.
>
> Good point.  In that case the solution is to forego the use of
> `defcustom`.  I'm fine with that.
>
> After all, the autoloads are supposed to run before the .emacs, so the
> user can `add-to-list` just fine to override whatever was added by
> autoloads.

I don't quite understand.  Let's assume debbugs would add a handler and
I wouldn't like to use it.  Isn't it correct that it would add it as
soon as it is loaded which means either where I have a (require
'debbugs) in my .emacs or call one of its autoloaded functions?

If that was true, we could document that packages should append their
entries which would give the users a chance to override those entries
because theirs would come earlier in the alist.  OTOH, when customizing
after the package has already been loaded and the entry is there, we
would have an undefined handler forever after the package was
uninstalled...

Or do you say it has an autoloaded (add-to-list ...) form?

Well, after some thinking I lean to the defvar + defcustom approach...

>> Symbol's value as variable is void: browse-url-mailto-function
>> make[2]: *** [Makefile:584: emacs.pdmp] Error 255
>> make[2]: Leaving directory '/home/horn/Repos/el/emacs/src'
>> make[1]: *** [Makefile:424: src] Error 2
>> make[1]: Leaving directory '/home/horn/Repos/el/emacs'
>> make: *** [Makefile:1119: bootstrap] Error 2
>> --8<---------------cut here---------------end--------------->8---
>>
>> When removing the autoload cookie, it builds fine.  Why is that?
>> `browse-url-mailto-function' and `browse-url-man-function' are defined
>> way before `browse-url-handlers'.
>
> But they're not autoloaded, so in the loaddefs.el file, you have
> a reference to it without the preceding definition.

I'm pretty sure I've tried adding autoload cookies to them as well and
got the same error.

> Maybe the better option is to define (and use) a new function
> `browse-url-mailto` (or some other name) which just funcalls
> `browse-url-mailto-function`.

Sounds good.  Will do.

Bye,
Tassilo



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: bug-reference.el: Allow custom handlers for opening URLs
  2020-05-05  7:06               ` Tassilo Horn
@ 2020-05-05 13:49                 ` Stefan Monnier
  2020-05-05 15:51                   ` Tassilo Horn
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2020-05-05 13:49 UTC (permalink / raw)
  To: emacs-devel

>> After all, the autoloads are supposed to run before the .emacs, so the
>> user can `add-to-list` just fine to override whatever was added by
>> autoloads.
> I don't quite understand.  Let's assume debbugs would add a handler and
> I wouldn't like to use it.  Isn't it correct that it would add it as
> soon as it is loaded

No, it would add it via an ;;;###autoload cookie, so it's added when the
package is activated (which used to happen via a call to
`package-initialize` either inside the .emacs or, as a last resort,
afterwards, but since Emacs-27 it now happens before the .emacs is
loaded via a call to `package-activate-all`).

>> But they're not autoloaded, so in the loaddefs.el file, you have
>> a reference to it without the preceding definition.
> I'm pretty sure I've tried adding autoload cookies to them as well and
> got the same error.

I'm pretty sure this can be made to work, but yes it's fiddly and it's
not the way I'd recommend in any case.


        Stefan




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: bug-reference.el: Allow custom handlers for opening URLs
  2020-05-05 13:49                 ` Stefan Monnier
@ 2020-05-05 15:51                   ` Tassilo Horn
  2020-05-05 17:44                     ` Stefan Monnier
  0 siblings, 1 reply; 15+ messages in thread
From: Tassilo Horn @ 2020-05-05 15:51 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

>>> After all, the autoloads are supposed to run before the .emacs, so
>>> the user can `add-to-list` just fine to override whatever was added
>>> by autoloads.
>> I don't quite understand.  Let's assume debbugs would add a handler
>> and I wouldn't like to use it.  Isn't it correct that it would add it
>> as soon as it is loaded
>
> No, it would add it via an ;;;###autoload cookie, so it's added when
> the package is activated (which used to happen via a call to
> `package-initialize` either inside the .emacs or, as a last resort,
> afterwards, but since Emacs-27 it now happens before the .emacs is
> loaded via a call to `package-activate-all`).

Ok, I see.

I want to come to a decision now.  What should we choose?  Either

  (a) a defvar for packages to plug in their url handler plus a
      defcustom for the user's preferences and to override the defvar's
      values, or

  (b) just a single defvar being used by both packages and the user.

I would prefer option (a) because I think this feature should be easily
configurable by users, i.e., without having to write Lisp code.  And
maybe also because not every package author might know that they should
add their handler with an autoload cookie.

I'm also open for an option (c). :-)

Bye,
Tassilo



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: bug-reference.el: Allow custom handlers for opening URLs
  2020-05-05 15:51                   ` Tassilo Horn
@ 2020-05-05 17:44                     ` Stefan Monnier
  2020-05-05 19:48                       ` browse-url.el: Custom handlers for certain URLs (was: bug-reference.el: Allow custom handlers for opening URLs) Tassilo Horn
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2020-05-05 17:44 UTC (permalink / raw)
  To: emacs-devel

> I want to come to a decision now.  What should we choose?  Either
>
>   (a) a defvar for packages to plug in their url handler plus a
>       defcustom for the user's preferences and to override the defvar's
>       values, or
>
>   (b) just a single defvar being used by both packages and the user.
> I'm also open for an option (c). :-)

They all sound good.  Tho I wonder what happened with option (d),


        Stefan




^ permalink raw reply	[flat|nested] 15+ messages in thread

* browse-url.el: Custom handlers for certain URLs (was: bug-reference.el: Allow custom handlers for opening URLs)
  2020-05-05 17:44                     ` Stefan Monnier
@ 2020-05-05 19:48                       ` Tassilo Horn
  0 siblings, 0 replies; 15+ messages in thread
From: Tassilo Horn @ 2020-05-05 19:48 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

>> I want to come to a decision now.  What should we choose?  Either
>>
>>   (a) a defvar for packages to plug in their url handler plus a
>>       defcustom for the user's preferences and to override the defvar's
>>       values, or
>>
>>   (b) just a single defvar being used by both packages and the user.
>> I'm also open for an option (c). :-)
>
> They all sound good.

Ok, great, so I've implemented option (a) now, i.e. there's a defvar
`browse-url-default-handlers' for packages and a defcustom
`browse-url-handlers' for the user.

Below is the full diff against master.  It works as intended (and I also
got that deprecation :doc thingy in the alist choice of
browse-url-browser-function working).  If nobody finds anything worth to
complain, I'm going to commit that anytime soon.

I guess that also requires a NEWS entry, right?

--8<---------------cut here---------------start------------->8---
modified   lisp/net/browse-url.el
@@ -114,9 +114,10 @@
 ;; To always save modified buffers before displaying the file in a browser:
 ;;	(setq browse-url-save-file t)
 
-;; To invoke different browsers for different URLs:
-;;      (setq browse-url-browser-function '(("^mailto:" . browse-url-mail)
-;;      				    ("." . browse-url-firefox)))
+;; To invoke different browsers/tools for different URLs, customize
+;; `browse-url-handlers'.  In earlier versions of Emacs, the same
+;; could be done by setting `browse-url-browser-function' to an alist
+;; but this usage is deprecated now.
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;; Code:
@@ -157,7 +158,9 @@ browse-url--browser-defcustom-type
 		   :value browse-url-default-browser)
     (function :tag "Your own function")
     (alist :tag "Regexp/function association list"
-	   :key-type regexp :value-type function)))
+	   :key-type regexp :value-type function
+           :format "%{%t%}\n%d%v\n"
+           :doc "Deprecated.  Use `browse-url-handlers' instead.")))
 
 ;;;###autoload
 (defcustom browse-url-browser-function 'browse-url-default-browser
@@ -165,13 +168,8 @@ browse-url-browser-function
 This is used by the `browse-url-at-point', `browse-url-at-mouse', and
 `browse-url-of-file' commands.
 
-If the value is not a function it should be a list of pairs
-\(REGEXP . FUNCTION).  In this case the function called will be the one
-associated with the first REGEXP which matches the current URL.  The
-function is passed the URL and any other args of `browse-url'.  The last
-regexp should probably be \".\" to specify a default browser.
-
-Also see `browse-url-secondary-browser-function'."
+Also see `browse-url-secondary-browser-function' and
+`browse-url-handlers'."
   :type browse-url--browser-defcustom-type
   :version "24.1")
 
@@ -595,6 +593,41 @@ browse-url-elinks-wrapper
   "Wrapper command prepended to the Elinks command-line."
   :type '(repeat (string :tag "Wrapper")))
 
+(defun browse-url--mailto (url &rest args)
+  "Calls `browse-url-mailto-function' with URL and ARGS."
+  (funcall browse-url-mailto-function url args))
+
+(defun browse-url--man (url &rest args)
+  "Calls `browse-url-man-function' with URL and ARGS."
+  (funcall browse-url-man-function url args))
+
+;;;###autoload
+(defvar browse-url-default-handlers
+  '(("\\`mailto:" . browse-url--mailto)
+    ("\\`man:" . browse-url--man)
+    ("\\`file://" . browse-url-emacs))
+  "Like `browse-url-handlers' but populated by Emacs and packages.
+
+Emacs and external packages capable of browsing certain URLs
+should place their entries in this alist rather than
+`browse-url-handlers' which is reserved for the user.")
+
+(defcustom browse-url-handlers nil
+  "An alist with elements of the form (REGEXP HANDLER).
+Each REGEXP is matched against the URL to be opened in turn and
+the first match's HANDLER is invoked with the URL.
+
+A HANDLER must be a function with the same arguments as
+`browse-url'.
+
+If no REGEXP matches, the same procedure is performed with the
+value of `browse-url-default-handlers'.  If there is also no
+match, the 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")
+
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; URL encoding
 
@@ -768,16 +801,18 @@ browse-url
   "Ask a WWW browser to load URL.
 Prompt for a URL, defaulting to the URL at or before point.
 Invokes a suitable browser function which does the actual job.
-The variable `browse-url-browser-function' says which browser function to
-use.  If the URL is a mailto: URL, consult `browse-url-mailto-function'
-first, if that exists.
-
-The additional ARGS are passed to the browser function.  See the doc
-strings of the actual functions, starting with `browse-url-browser-function',
-for information about the significance of ARGS (most of the functions
-ignore it).
-If ARGS are omitted, the default is to pass `browse-url-new-window-flag'
-as ARGS."
+
+The variables `browse-url-browser-function',
+`browse-url-handlers', and `browse-url-default-handlers'
+determine which browser function to use.
+
+The additional ARGS are passed to the browser function.  See the
+doc strings of the actual functions, starting with
+`browse-url-browser-function', for information about the
+significance of ARGS (most of the functions ignore it).
+
+If ARGS are omitted, the default is to pass
+`browse-url-new-window-flag' as ARGS."
   (interactive (browse-url-interactive-arg "URL: "))
   (unless (called-interactively-p 'interactive)
     (setq args (or args (list browse-url-new-window-flag))))
@@ -786,12 +821,15 @@ 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 (append browse-url-handlers
+                                          browse-url-default-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
@@ -801,15 +839,19 @@ browse-url
         (setenv "DISPLAY" (frame-parameter nil 'display)))
     (if (and (consp function)
 	     (not (functionp function)))
-	;; The `function' can be an alist; look down it for first match
-	;; and apply the function (which might be a lambda).
-	(catch 'done
-	  (dolist (bf function)
-	    (when (string-match (car bf) url)
-	      (apply (cdr bf) url args)
-	      (throw 'done t)))
-	  (error "No browse-url-browser-function matching URL %s"
-		 url))
+	;; The `function' can be an alist; look down it for first
+	;; match and apply the function (which might be a lambda).
+	;; However, this usage is deprecated as of Emacs 28.1.
+        (progn
+          (warn "Having `browse-url-browser-function' set to an
+alist is deprecated.  Use `browse-url-handlers' instead.")
+          (catch 'done
+	    (dolist (bf function)
+	      (when (string-match (car bf) url)
+	        (apply (cdr bf) url args)
+	        (throw 'done t)))
+	    (error "No browse-url-browser-function matching URL %s"
+	           url)))
       ;; Unbound symbols go down this leg, since void-function from
       ;; apply is clearer than wrong-type-argument from dolist.
       (apply function url args))))
--8<---------------cut here---------------end--------------->8---

> Tho I wonder what happened with option (d),

It's superceded by option (e). :-)

Bye,
Tassilo



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: bug-reference.el: Allow custom handlers for opening URLs
  2020-05-03  8:50 bug-reference.el: Allow custom handlers for opening URLs Tassilo Horn
  2020-05-03 14:44 ` Stefan Monnier
@ 2020-05-05 19:55 ` Arash Esbati
  1 sibling, 0 replies; 15+ messages in thread
From: Arash Esbati @ 2020-05-05 19:55 UTC (permalink / raw)
  To: emacs-devel

Hi Tassilo,

Tassilo Horn <tsdh@gnu.org> writes:

> Oh, and how do I correctly write #'my-function in a docstring so that
> the ' doesn't get displayed differently?

I'm not sure if this question was also answered but are you looking for

  #\\='my-function ?

Best, Arash



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-05-05 19:55 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-03  8:50 bug-reference.el: Allow custom handlers for opening URLs Tassilo Horn
2020-05-03 14:44 ` Stefan Monnier
2020-05-03 15:24   ` Tassilo Horn
2020-05-03 20:39     ` Stefan Monnier
2020-05-04  9:41       ` Tassilo Horn
2020-05-04 15:32         ` Stefan Monnier
2020-05-04 17:09           ` Tassilo Horn
2020-05-04 18:54             ` Stefan Monnier
2020-05-05  7:06               ` Tassilo Horn
2020-05-05 13:49                 ` Stefan Monnier
2020-05-05 15:51                   ` Tassilo Horn
2020-05-05 17:44                     ` Stefan Monnier
2020-05-05 19:48                       ` browse-url.el: Custom handlers for certain URLs (was: bug-reference.el: Allow custom handlers for opening URLs) Tassilo Horn
2020-05-04  6:52     ` bug-reference.el: Allow custom handlers for opening URLs Yuri Khan
2020-05-05 19:55 ` Arash Esbati

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).