bug#75052: 31; browse-url-transform-alist is not used by secondary browser

[Daniel Mendler via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

Stefan Kangas <stefankangas@gmail.com> writes:

>> Another perhaps simpler and significantly more minimal approach would be
>> to let-bind `browse-url-browser-function' to
>> `browse-url-secondary-browser-function' at all call-sites instead of
>> calling `browse-url-secondary-browser-function' directly. Inside the
>> let, `browse-url' would be called as usual.
>>
>> However the problem with that approach is that it wouldn't be entirely
>> backward compatible, since the `browse-url-handlers' take precedence
>> over the `browse-url-browser-function'. In order to circumvent this
>> problem we could introduce a new variable
>> `browse-url-override-browser-function' which would take precedence in
>> `browse-url'. `package-browse-url' would become the following:
>>
>> #+begin_src emacs-lisp
>> (defun package-browse-url (desc &optional secondary)
>>   (interactive (list (package--query-desc) current-prefix-arg)
>>                package-menu-mode)
>>   (unless desc
>>     (user-error "No package here"))
>>   (let ((url (cdr (assoc :url (package-desc-extras desc))))
>>         (browse-url-override-browser-function
>>          (and secondary browse-url-secondary-browser-function)))
>>     (unless url
>>       (user-error "No website for %s" (package-desc-name desc)))
>>     (browse-url url)))
>> #+end_src
>>
>> However this small incompatibility may even be acceptable. In case URL
>> handlers are defined, you may want them to take precedence in any case.
>>
>> I believe the problem is only pronounced in
>> `browse-url-with-browser-kind' which under all circumstances wants to
>> use the browser function it has chosen. In order to achieve that it
>> could let-bind `browse-url-handlers' and `browse-url-default-handlers'
>> to nil. The function would become this:
>>
>> #+begin_src emacs-lisp
>> (defun browse-url-with-browser-kind (kind url &optional arg)
>>   (interactive ...)
>>   (let ((function (browse-url-select-handler url kind)))
>>     (unless function
>>       (setq function
>>             (seq-find
>>              (lambda (fun)
>>                (eq kind (browse-url--browser-kind fun url)))
>>              (list browse-url-browser-function
>>                    browse-url-secondary-browser-function
>>                    #'browse-url-default-browser
>>                    #'eww))))
>>     ;; Ensure that function is used
>>     (let ((browse-url-browser-function function)
>>           (browse-url-default-handlers nil)
>>           (browse-url-handlers))
>>       (browse-url url arg))))
>> #+end_src
>
> I quite like this idea, and I think it buys us some improvements even,
> as you detail above, while the compatibility issues are fixed with the
> new variable.
>
> This is the only idea so far that guarantees that the environment
> variables, etc., that we set now and in future in `browse-url`, are used
> also for the secondary browser.  IIUC, it would also give us exactly one
> function that Lisp programs should normally call: `browse-url`.

Yes, it is best if all calls just go through `browse-url'. Also we avoid
adding more functions such that the API isn't made more complex.

Just to be clear - I think we don't actually need the override variable
if we accept a minor change in behavior with respect to the handlers. If
a specific handler is defined we may always want it to take precedence,
except in the case of `browse-url-with-browser-kind', but there we can
simply rebind the handler variables to nil.

> Could you perhaps send a patch?

Yes, I'll send a patch and then you can take another look.

Daniel