GNU bug report logs - #74730
[PATCH] 30.0.92; eww-browse-with-external-browser and eww-follow-link should use browse-url-with-browser-kind

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 74730 in the body.
You can then email your comments to 74730 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Daniel Mendler <mail <at> daniel-mendler.de>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] 30.0.92; eww-browse-with-external-browser and
 eww-follow-link should use browse-url-with-browser-kind
Date: Sat, 07 Dec 2024 23:02:09 +0100

[Message part 1 (text/plain, inline)]

Tags: patch

The command `eww-browse-with-external-browser' uses
`browse-url-secondary-browser-function'. This makes the command
ineffective in the common setup where
`browse-url-secondary-browser-function' is set to `eww-browse-url' and
`browse-url-browser-function' is set to an external browser.

Fortunately we can use `browse-url-with-browser-kind' instead, which
guarantees that an external browser is launched.

Furthermore `eww-follow-link' should use `browse-url-with-browser-kind'
if the EXTERNAL prefix argument is non-nil.

I've looked at all other uses of `browse-url-browser-function' and
`browse-url-secondary-browser-function' in the Emacs code base and I
have not found other problems. Most commands use a prefix argument to
switch to the secondary browser, relying on the primary browser by
default. The problem is limited to `eww-browse-with-external-browser'
and `eww-follow-link'.

In GNU Emacs 30.0.92 (build 2, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.18.2, Xaw scroll bars) of 2024-11-20

[0001-eww-Use-browse-url-browser-kind-for-external-browsin.patch (text/patch, attachment)]

Message #8 received at 74730 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Daniel Mendler <mail <at> daniel-mendler.de>
Cc: 74730 <at> debbugs.gnu.org
Subject: Re: bug#74730: [PATCH] 30.0.92;
 eww-browse-with-external-browser and eww-follow-link should use
 browse-url-with-browser-kind
Date: Sun, 08 Dec 2024 07:57:13 +0200

> Date: Sat, 07 Dec 2024 23:02:09 +0100
> From:  Daniel Mendler via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> The command `eww-browse-with-external-browser' uses
> `browse-url-secondary-browser-function'. This makes the command
> ineffective in the common setup where
> `browse-url-secondary-browser-function' is set to `eww-browse-url' and
> `browse-url-browser-function' is set to an external browser.

The doc string of browse-url-secondary-browser-function explicitly
says not to set it to eww.  So users who do the above are acting
against the design and the recommended usage, and I'm not sure we
should support that at all, let alone with a (seemingly)
backward-incompatible change such as the one you propose.

> Fortunately we can use `browse-url-with-browser-kind' instead, which
> guarantees that an external browser is launched.
> 
> Furthermore `eww-follow-link' should use `browse-url-with-browser-kind'
> if the EXTERNAL prefix argument is non-nil.
> 
> I've looked at all other uses of `browse-url-browser-function' and
> `browse-url-secondary-browser-function' in the Emacs code base and I
> have not found other problems. Most commands use a prefix argument to
> switch to the secondary browser, relying on the primary browser by
> default. The problem is limited to `eww-browse-with-external-browser'
> and `eww-follow-link'.

What will happen as result of this change to users who customize
browse-url-secondary-browser-function as its doc string says, and then
invoke the command eww-browse-with-external-browser?

Message #11 received at 74730 <at> debbugs.gnu.org (full text, mbox):

From: Daniel Mendler <mail <at> daniel-mendler.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 74730 <at> debbugs.gnu.org
Subject: Re: bug#74730: [PATCH] 30.0.92; eww-browse-with-external-browser
 and eww-follow-link should use browse-url-with-browser-kind
Date: Sun, 08 Dec 2024 07:13:54 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Sat, 07 Dec 2024 23:02:09 +0100
>> From:  Daniel Mendler via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>> 
>> The command `eww-browse-with-external-browser' uses
>> `browse-url-secondary-browser-function'. This makes the command
>> ineffective in the common setup where
>> `browse-url-secondary-browser-function' is set to `eww-browse-url' and
>> `browse-url-browser-function' is set to an external browser.
>
> The doc string of browse-url-secondary-browser-function explicitly
> says not to set it to eww.  So users who do the above are acting
> against the design and the recommended usage, and I'm not sure we
> should support that at all, let alone with a (seemingly)
> backward-incompatible change such as the one you propose.

How can I then use an external browser as the default and Eww as
alternative? I argue that this is a legitimate use case - an external
browser as primary and Eww as secondary browser for distraction-free
reading.

As I wrote, the only place which leads to problems is in
`eww-browse-with-external-browser' - I checked all other places in
Emacs. I argue that the behavior and implementation will be even more
explicit, since `browse-url-with-browser-kind' explicitly supports the
`external' kind. But you are right about the backward compatibility
problem, see below.

>> Fortunately we can use `browse-url-with-browser-kind' instead, which
>> guarantees that an external browser is launched.
>> 
>> Furthermore `eww-follow-link' should use `browse-url-with-browser-kind'
>> if the EXTERNAL prefix argument is non-nil.
>> 
>> I've looked at all other uses of `browse-url-browser-function' and
>> `browse-url-secondary-browser-function' in the Emacs code base and I
>> have not found other problems. Most commands use a prefix argument to
>> switch to the secondary browser, relying on the primary browser by
>> default. The problem is limited to `eww-browse-with-external-browser'
>> and `eww-follow-link'.
>
> What will happen as result of this change to users who customize
> browse-url-secondary-browser-function as its doc string says, and then
> invoke the command eww-browse-with-external-browser?

In this case the change will indeed be backward-incompatible, if
multiple external browsers are used and the secondary browser is
configured to a different one than the one which will be selected by
`browse-url-with-browser-kind'.

The patch can be extended however. I can change it such that the
`browse-url-secondary-browser-function' is checked first if it is indeed
external (via the `browser-kind' property). And only if it is not
external, `browse-url-with-browser-kind' will be used. One could be even
more strict and compare `browse-url-secondary-browser-function' to
`eww-browse-url' and only in this case fall back to
`browse-url-with-browser-kind'.

Daniel

Message #14 received at 74730 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Daniel Mendler <mail <at> daniel-mendler.de>
Cc: 74730 <at> debbugs.gnu.org
Subject: Re: bug#74730: [PATCH] 30.0.92; eww-browse-with-external-browser
 and eww-follow-link should use browse-url-with-browser-kind
Date: Sun, 08 Dec 2024 09:03:02 +0200

> From: Daniel Mendler <mail <at> daniel-mendler.de>
> Cc: 74730 <at> debbugs.gnu.org
> Date: Sun, 08 Dec 2024 07:13:54 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > The doc string of browse-url-secondary-browser-function explicitly
> > says not to set it to eww.  So users who do the above are acting
> > against the design and the recommended usage, and I'm not sure we
> > should support that at all, let alone with a (seemingly)
> > backward-incompatible change such as the one you propose.
> 
> How can I then use an external browser as the default and Eww as
> alternative? I argue that this is a legitimate use case - an external
> browser as primary and Eww as secondary browser for distraction-free
> reading.

It is a legitimate use case, but please describe it in more detail:
what do you mean by "using external browser as primary and eww as
secondary"?  That is, which Emacs commands should behave like that?

E.g., is "use browse-url-with-browser-kind" a valid solution for the
use case you describe?  If not, why not?

> As I wrote, the only place which leads to problems is in
> `eww-browse-with-external-browser' - I checked all other places in
> Emacs.

You haven't describe the results of your investigation in detail.  I
see 6 callers of browse-url-secondary-browser-function, so please
explain how this change is not a problem in each one of them.  Your
original message said that "most commands use a prefix argument to
switch to the secondary browser", but I don't understand how saying
that allows you to conclude that this change will not cause trouble?

> I argue that the behavior and implementation will be even more
> explicit, since `browse-url-with-browser-kind' explicitly supports the
> `external' kind. But you are right about the backward compatibility
> problem, see below.
> [...]
> > What will happen as result of this change to users who customize
> > browse-url-secondary-browser-function as its doc string says, and then
> > invoke the command eww-browse-with-external-browser?
> 
> In this case the change will indeed be backward-incompatible, if
> multiple external browsers are used and the secondary browser is
> configured to a different one than the one which will be selected by
> `browse-url-with-browser-kind'.
> 
> The patch can be extended however. I can change it such that the
> `browse-url-secondary-browser-function' is checked first if it is indeed
> external (via the `browser-kind' property). And only if it is not
> external, `browse-url-with-browser-kind' will be used. One could be even
> more strict and compare `browse-url-secondary-browser-function' to
> `eww-browse-url' and only in this case fall back to
> `browse-url-with-browser-kind'.

If we can make it so that existing customizations of
browse-url-secondary-browser-function will keep working as they did,
then the backward incompatibility issue will disappear, and such a
change becomes possible.  But in any case, the doc string of
browse-url-secondary-browser-function should be amended if we are
going to support its setting to eww.

Also, are we sure all the relevant functions always have the
'browse-url-browser-kind property? what about user-defined functions,
for example?

Message #17 received at 74730 <at> debbugs.gnu.org (full text, mbox):

From: Daniel Mendler <mail <at> daniel-mendler.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 74730 <at> debbugs.gnu.org
Subject: Re: bug#74730: [PATCH] 30.0.92; eww-browse-with-external-browser
 and eww-follow-link should use browse-url-with-browser-kind
Date: Sun, 08 Dec 2024 08:27:20 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Daniel Mendler <mail <at> daniel-mendler.de>
>> Cc: 74730 <at> debbugs.gnu.org
>> Date: Sun, 08 Dec 2024 07:13:54 +0100
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> > The doc string of browse-url-secondary-browser-function explicitly
>> > says not to set it to eww.  So users who do the above are acting
>> > against the design and the recommended usage, and I'm not sure we
>> > should support that at all, let alone with a (seemingly)
>> > backward-incompatible change such as the one you propose.
>> 
>> How can I then use an external browser as the default and Eww as
>> alternative? I argue that this is a legitimate use case - an external
>> browser as primary and Eww as secondary browser for distraction-free
>> reading.
>
> It is a legitimate use case, but please describe it in more detail:
> what do you mean by "using external browser as primary and eww as
> secondary"?  That is, which Emacs commands should behave like that?

In Emacs there are many commands which use `browse-url-browser-function'
or `browse-url-secondary-browser-function' depending on the prefix
argument. Many of the commands are generic and act on URLs or buttons at
point in different kinds of buffers. Also many external packages use
`browse-url'.

- `goto-address-at-point'
- `browse-url-button-open'
- `browse-url-button-open-url'
- `package-browse-url'
- `gnus-summary-browse-url'

When any of these commands on links is invoked, the
`browse-url-browser-function' is used by default. The primary browser is
configured by `browse-url-browser-function' while the secondary browser
is configured by `browse-url-secondary-browser-function'. Both use cases
are plausible: `browse-url-browser-function' can be external and
`browse-url-secondary-browser-function' can be internal and vice versa.

> E.g., is "use browse-url-with-browser-kind" a valid solution for the
> use case you describe?  If not, why not?

`browse-url-with-browser-kind' can be used when one explicitly wants to
call an external or internal browser. The command in Eww
`eww-browse-with-external-browser' has `*-external-*' as part of its
name, so in this case I think is a valid solution.

>> As I wrote, the only place which leads to problems is in
>> `eww-browse-with-external-browser' - I checked all other places in
>> Emacs.
>
> You haven't describe the results of your investigation in detail.  I
> see 6 callers of browse-url-secondary-browser-function, so please
> explain how this change is not a problem in each one of them.  Your
> original message said that "most commands use a prefix argument to
> switch to the secondary browser", but I don't understand how saying
> that allows you to conclude that this change will not cause trouble?

Each of the other call sites does not make assumptions about the
internal or external distinction. All that is important is that two
browser functions exist - a primary browser function and a secondary
function. By default the primary function is called, and if explicitly
requested by the user via a prefix argument, the secondary function is
used. It does not matter if the functions point to external or internal
browsers.

>> I argue that the behavior and implementation will be even more
>> explicit, since `browse-url-with-browser-kind' explicitly supports the
>> `external' kind. But you are right about the backward compatibility
>> problem, see below.
>> [...]
>> > What will happen as result of this change to users who customize
>> > browse-url-secondary-browser-function as its doc string says, and then
>> > invoke the command eww-browse-with-external-browser?
>> 
>> In this case the change will indeed be backward-incompatible, if
>> multiple external browsers are used and the secondary browser is
>> configured to a different one than the one which will be selected by
>> `browse-url-with-browser-kind'.
>> 
>> The patch can be extended however. I can change it such that the
>> `browse-url-secondary-browser-function' is checked first if it is indeed
>> external (via the `browser-kind' property). And only if it is not
>> external, `browse-url-with-browser-kind' will be used. One could be even
>> more strict and compare `browse-url-secondary-browser-function' to
>> `eww-browse-url' and only in this case fall back to
>> `browse-url-with-browser-kind'.
>
> If we can make it so that existing customizations of
> browse-url-secondary-browser-function will keep working as they did,
> then the backward incompatibility issue will disappear, and such a
> change becomes possible.  But in any case, the doc string of
> browse-url-secondary-browser-function should be amended if we are
> going to support its setting to eww.

Okay, I can do that.

> Also, are we sure all the relevant functions always have the
> 'browse-url-browser-kind property? what about user-defined functions,
> for example?

User-defined functions may not have the property. But we can be
conservative and preserve the existing behavior in the case where the
property is unavailable, treating the missing property like the value
`external'. Only if the property is found and `internal', the
`browse-url-with-browser-kind' will be used to make sure that an
external browser is used. As I mentioned, alternatively one can be even
more conservative and only use `browse-url-with-browser-kind' if
`browse-url-secondary-browser-function' is set to `eww-browse-url'. That
might be a little bit too restrictive, but it would be completely
backward compatible.

Daniel

Message #20 received at 74730 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Daniel Mendler <mail <at> daniel-mendler.de>
Cc: 74730 <at> debbugs.gnu.org
Subject: Re: bug#74730: [PATCH] 30.0.92; eww-browse-with-external-browser
 and eww-follow-link should use browse-url-with-browser-kind
Date: Sun, 08 Dec 2024 14:02:04 +0200

> From: Daniel Mendler <mail <at> daniel-mendler.de>
> Cc: 74730 <at> debbugs.gnu.org
> Date: Sun, 08 Dec 2024 08:27:20 +0100
> 
> > You haven't describe the results of your investigation in detail.  I
> > see 6 callers of browse-url-secondary-browser-function, so please
> > explain how this change is not a problem in each one of them.  Your
> > original message said that "most commands use a prefix argument to
> > switch to the secondary browser", but I don't understand how saying
> > that allows you to conclude that this change will not cause trouble?
> 
> Each of the other call sites does not make assumptions about the
> internal or external distinction.

Except that anything that calls them in eww must make some
assumptions, because it could otherwise cause infinite recursion.

> > If we can make it so that existing customizations of
> > browse-url-secondary-browser-function will keep working as they did,
> > then the backward incompatibility issue will disappear, and such a
> > change becomes possible.  But in any case, the doc string of
> > browse-url-secondary-browser-function should be amended if we are
> > going to support its setting to eww.
> 
> Okay, I can do that.
> 
> > Also, are we sure all the relevant functions always have the
> > 'browse-url-browser-kind property? what about user-defined functions,
> > for example?
> 
> User-defined functions may not have the property. But we can be
> conservative and preserve the existing behavior in the case where the
> property is unavailable, treating the missing property like the value
> `external'. Only if the property is found and `internal', the
> `browse-url-with-browser-kind' will be used to make sure that an
> external browser is used. As I mentioned, alternatively one can be even
> more conservative and only use `browse-url-with-browser-kind' if
> `browse-url-secondary-browser-function' is set to `eww-browse-url'. That
> might be a little bit too restrictive, but it would be completely
> backward compatible.

Looking forward to seeing an updated patch which preserves the current
behavior wrt browse-url-secondary-browser-function's customization.

Thanks.

Message #23 received at 74730 <at> debbugs.gnu.org (full text, mbox):

From: Daniel Mendler <mail <at> daniel-mendler.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 74730 <at> debbugs.gnu.org
Subject: Re: bug#74730: [PATCH] 30.0.92; eww-browse-with-external-browser
 and eww-follow-link should use browse-url-with-browser-kind
Date: Sun, 08 Dec 2024 14:00:56 +0100

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> > If we can make it so that existing customizations of
>> > browse-url-secondary-browser-function will keep working as they did,
>> > then the backward incompatibility issue will disappear, and such a
>> > change becomes possible.  But in any case, the doc string of
>> > browse-url-secondary-browser-function should be amended if we are
>> > going to support its setting to eww.
>> 
>> Okay, I can do that.
>> 
>> > Also, are we sure all the relevant functions always have the
>> > 'browse-url-browser-kind property? what about user-defined functions,
>> > for example?
>> 
>> User-defined functions may not have the property. But we can be
>> conservative and preserve the existing behavior in the case where the
>> property is unavailable, treating the missing property like the value
>> `external'. Only if the property is found and `internal', the
>> `browse-url-with-browser-kind' will be used to make sure that an
>> external browser is used. As I mentioned, alternatively one can be even
>> more conservative and only use `browse-url-with-browser-kind' if
>> `browse-url-secondary-browser-function' is set to `eww-browse-url'. That
>> might be a little bit too restrictive, but it would be completely
>> backward compatible.
>
> Looking forward to seeing an updated patch which preserves the current
> behavior wrt browse-url-secondary-browser-function's customization.
>
> Thanks.

Updated patch attached.

[0001-eww-Improve-eww-browse-with-external-browser.patch (text/x-diff, attachment)]

Message #26 received at 74730 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Daniel Mendler <mail <at> daniel-mendler.de>
Cc: 74730 <at> debbugs.gnu.org
Subject: Re: bug#74730: [PATCH] 30.0.92; eww-browse-with-external-browser
 and eww-follow-link should use browse-url-with-browser-kind
Date: Sun, 08 Dec 2024 15:32:31 +0200

> From: Daniel Mendler <mail <at> daniel-mendler.de>
> Cc: 74730 <at> debbugs.gnu.org
> Date: Sun, 08 Dec 2024 14:00:56 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> > If we can make it so that existing customizations of
> >> > browse-url-secondary-browser-function will keep working as they did,
> >> > then the backward incompatibility issue will disappear, and such a
> >> > change becomes possible.  But in any case, the doc string of
> >> > browse-url-secondary-browser-function should be amended if we are
> >> > going to support its setting to eww.
> >> 
> >> Okay, I can do that.
> >> 
> >> > Also, are we sure all the relevant functions always have the
> >> > 'browse-url-browser-kind property? what about user-defined functions,
> >> > for example?
> >> 
> >> User-defined functions may not have the property. But we can be
> >> conservative and preserve the existing behavior in the case where the
> >> property is unavailable, treating the missing property like the value
> >> `external'. Only if the property is found and `internal', the
> >> `browse-url-with-browser-kind' will be used to make sure that an
> >> external browser is used. As I mentioned, alternatively one can be even
> >> more conservative and only use `browse-url-with-browser-kind' if
> >> `browse-url-secondary-browser-function' is set to `eww-browse-url'. That
> >> might be a little bit too restrictive, but it would be completely
> >> backward compatible.
> >
> > Looking forward to seeing an updated patch which preserves the current
> > behavior wrt browse-url-secondary-browser-function's customization.
> >
> > Thanks.
> 
> Updated patch attached.

Thanks, a few comments below.

>  (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),
> -used as the secondary browser choice, typically when a prefix
> -argument is given to a URL-opening command in those modes that
> -support this (for instance, eww/shr).
> +
> +This is browser is used as the secondary browser choice, typically
> +when a prefix argument is given to a URL-opening command in those
> +modes that support this (for instance `goto-addr-at-point', eww or shr).

This doc string should explain the assumptions about this and the
other variable, browse-url-browser-function: that either one or the
other invokes the external browser, and the other one should then NOT
do the same.  Users should be aware and understand the rules of the
game here, which are now slightly more complex than they were before,
so removing the previous assumption without replacing it with anything
makes the doc string less useful.

>  (defun eww-browse-with-external-browser (&optional url)
>    "Browse the current URL with an external browser.
> -The browser to used is specified by the
> -`browse-url-secondary-browser-function' variable."
> +Calls `browse-url-secondary-browser-function' if it does not correspond
> +to EWW.  Otherwise `browse-url' is used."
                      ^^^^^^^^^^^^^^^^^^^^
Passive tense alert!

>    (interactive nil eww-mode)
> -  (funcall browse-url-secondary-browser-function
> +  (funcall (if (memq browse-url-secondary-browser-function
> +                     '(eww eww-browse-url))
> +               #'browse-url
> +             browse-url-secondary-browser-function)

I think we should allow here (and document) more just 2 literal
functions hard-coded in this command.  Perhaps some special property
on the function's symbol?  Then these two specific functions can be
supported via the property, and what's more important, users and
applications could use other functions with the same semantics.

>  (defun eww-follow-link (&optional external mouse-event)
>    "Browse the URL under point.
>  If EXTERNAL is single prefix, browse the URL using
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
What does this mean? does it mean that EXTERNAL is non-nil in
interactive usage if the command is invoked with C-u?  If so, let's
say that.  (Yes, I know that problem came from the original doc
string.)

> @@ -2180,7 +2182,7 @@ eww-follow-link
>        ;; and `browse-url-mailto-function'.
>        (browse-url url))
>       ((and (consp external) (<= (car external) 4))
> -      (funcall browse-url-secondary-browser-function url)
> +      (eww-browse-with-external-browser url)

I'm not sure I agree.  The call to eww-browse-with-external-browser
will no longer ensure an external browser is used, after these
changes.  Why not call browse-url-default-browser instead?

In any case, the doc string should say that if no external browser
could be found, this will fall back to eww.

Message #29 received at 74730 <at> debbugs.gnu.org (full text, mbox):

From: Daniel Mendler <mail <at> daniel-mendler.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 74730 <at> debbugs.gnu.org
Subject: Re: bug#74730: [PATCH] 30.0.92; eww-browse-with-external-browser
 and eww-follow-link should use browse-url-with-browser-kind
Date: Sun, 08 Dec 2024 18:39:28 +0100

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

Updated patch attached.

>>  (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),
>> -used as the secondary browser choice, typically when a prefix
>> -argument is given to a URL-opening command in those modes that
>> -support this (for instance, eww/shr).
>> +
>> +This is browser is used as the secondary browser choice, typically
>> +when a prefix argument is given to a URL-opening command in those
>> +modes that support this (for instance `goto-addr-at-point', eww or shr).
>
> This doc string should explain the assumptions about this and the
> other variable, browse-url-browser-function: that either one or the
> other invokes the external browser, and the other one should then NOT
> do the same.  Users should be aware and understand the rules of the
> game here, which are now slightly more complex than they were before,
> so removing the previous assumption without replacing it with anything
> makes the doc string less useful.

I expanded the docstring.

>>    (interactive nil eww-mode)
>> -  (funcall browse-url-secondary-browser-function
>> +  (funcall (if (memq browse-url-secondary-browser-function
>> +                     '(eww eww-browse-url))
>> +               #'browse-url
>> +             browse-url-secondary-browser-function)
>
> I think we should allow here (and document) more just 2 literal
> functions hard-coded in this command.  Perhaps some special property
> on the function's symbol?  Then these two specific functions can be
> supported via the property, and what's more important, users and
> applications could use other functions with the same semantics.

In this case I think it is better to use `browse-url-with-browser-kind'
as I had initially suggested. This will be better than introducing yet
another property only for this purpose.

>>  (defun eww-follow-link (&optional external mouse-event)
>>    "Browse the URL under point.
>>  If EXTERNAL is single prefix, browse the URL using
>    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> What does this mean? does it mean that EXTERNAL is non-nil in
> interactive usage if the command is invoked with C-u?  If so, let's
> say that.  (Yes, I know that problem came from the original doc
> string.)

I improved the docstring.

>> @@ -2180,7 +2182,7 @@ eww-follow-link
>>        ;; and `browse-url-mailto-function'.
>>        (browse-url url))
>>       ((and (consp external) (<= (car external) 4))
>> -      (funcall browse-url-secondary-browser-function url)
>> +      (eww-browse-with-external-browser url)
>
> I'm not sure I agree.  The call to eww-browse-with-external-browser
> will no longer ensure an external browser is used, after these
> changes.  Why not call browse-url-default-browser instead?

Now, it is ensured that the external browser is called.

Daniel

[0001-eww-Use-browse-url-with-browser-kind-in-eww-browse-w.patch (text/x-diff, attachment)]

Message #34 received at 74730-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Daniel Mendler <mail <at> daniel-mendler.de>
Cc: 74730-done <at> debbugs.gnu.org
Subject: Re: bug#74730: [PATCH] 30.0.92; eww-browse-with-external-browser
 and eww-follow-link should use browse-url-with-browser-kind
Date: Thu, 12 Dec 2024 12:42:12 +0200

> From: Daniel Mendler <mail <at> daniel-mendler.de>
> Cc: 74730 <at> debbugs.gnu.org
> Date: Sun, 08 Dec 2024 18:39:28 +0100
> 
> Updated patch attached.

Thanks, installed on the master branch, and closing the bug.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.