* bug#75052: 31; browse-url-transform-alist is not used by secondary browser
@ 2024-12-23 18:41 Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-12-24 1:00 ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 2+ messages in thread
From: Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-23 18:41 UTC (permalink / raw)
To: 75052; +Cc: Stefan Kangas
Hello Stefan,
you recently added `browse-url-transform-alist' to `browse-url'. At a
few places in Emacs browser functions are called directly. For direct
browser function calls the transformation alist is not applied.
Examples:
- `browse-url-secondary-browser-function' is called by
`browse-url-button-open' or by `package-browse-url' and others.
- `browse-url-with-browser-kind' calls browser functions directly.
A possible solution could be to add a function
`browse-url-with-function' which takes a function argument:
(defun browse-url-with-function (func url &rest args)
"Open URL with browser FUNC.
If FUNC is a function use this function.
If FUNC is nil use the default `browse-url'.
For other non-nil values use `browse-url-secondary-browser-function'."
(if (not func)
(apply #'browse-url url args)
(apply
(if (functionp func) func browse-url-secondary-browser-function)
(browse-url--transform url) args)))
All call sites which call a browse function directly could use
`browse-url-with-function' instead. Calling browser functions directly
should be discouraged. For example `package-browse-url' would simplify
to this:
(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)))))
(unless url
(user-error "No website for %s" (package-desc-name desc)))
(browse-url-with-function secondary url)))
Similarly `browse-url-with-browser-kind':
(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))))
(browse-url-with-function function url arg)))
Does this sound like an acceptable plan? Thank you.
Daniel
^ permalink raw reply [flat|nested] 2+ messages in thread
* bug#75052: 31; browse-url-transform-alist is not used by secondary browser
2024-12-23 18:41 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
@ 2024-12-24 1:00 ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 0 replies; 2+ messages in thread
From: Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-12-24 1:00 UTC (permalink / raw)
To: 75052; +Cc: Stefan Kangas
[-- Attachment #1: Type: text/plain, Size: 512 bytes --]
I've attached a patch which implements a `browse-url-with-function'.
However I am not sure if we want to add more `browse-url-*' function
variants, since this will make the `browse-url' API increasingly
unclear.
An alternative could be to add keyword arguments to `browse-url' for
consolidation:
For example:
(browse-url url :kind 'external) ;; replaces `browse-url-with-browser-kind'
(browse-url url :via some-function) ;; alternative to `browse-url-with-function'
(browse-url url :via secondary)
Daniel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-New-function-browse-url-with-function.patch --]
[-- Type: text/x-diff, Size: 10147 bytes --]
From 0b7edfc61aace7c692c04f1d0f7a6b6eb987b657 Mon Sep 17 00:00:00 2001
From: Daniel Mendler <mail@daniel-mendler.de>
Date: Tue, 24 Dec 2024 01:31:35 +0100
Subject: [PATCH] New function browse-url-with-function
`browse-url-with-function' takes a browser function as argument,
the URL and rest arguments, which are passed to the browser
function. `browse-url-with-function' transforms the URL first
with `browse-url-transform-alist' before calling the browser
function.
Calling browser functions directly is discouraged. Instead
`browse-url' or `browse-url-with-function' should be used, such
that URL transformations are applied.
* lisp/net/browse-url.el (browse-url--transform): New function,
extracted from `browse-url'.
(browse-url): Use it.
(browse-url-with-function): New function.
(browse-url-with-browser-kind, browse-url-button-open)
(browse-url-button-open-url):
* lisp/net/shr.el (shr-browse-url):
* lisp/net/eww.el (eww-browse-with-external-browser):
* lisp/gnus/gnus-sum.el (gnus-summary-browse-url):
* lisp/emacs-lisp/package.el (package-browse-url): Use it.
---
lisp/emacs-lisp/package.el | 4 +--
lisp/gnus/gnus-sum.el | 10 +++-----
lisp/net/browse-url.el | 50 ++++++++++++++++++++++++--------------
lisp/net/eww.el | 2 +-
lisp/net/goto-addr.el | 6 ++---
lisp/net/shr.el | 18 ++++++--------
6 files changed, 49 insertions(+), 41 deletions(-)
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 5f785071ea3..ebd56c75677 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -4689,9 +4689,7 @@ package-browse-url
(let ((url (cdr (assoc :url (package-desc-extras desc)))))
(unless url
(user-error "No website for %s" (package-desc-name desc)))
- (if secondary
- (funcall browse-url-secondary-browser-function url)
- (browse-url url))))
+ (browse-url-with-function secondary url)))
(declare-function ietf-drums-parse-address "ietf-drums"
(string &optional decode))
diff --git a/lisp/gnus/gnus-sum.el b/lisp/gnus/gnus-sum.el
index cebeb6d4c37..931952a2885 100644
--- a/lisp/gnus/gnus-sum.el
+++ b/lisp/gnus/gnus-sum.el
@@ -9408,11 +9408,11 @@ gnus-shorten-url
(concat "#" target)))))
(concat host (string-truncate-left rest (- max (length host)))))))
-(defun gnus-summary-browse-url (&optional external)
+(defun gnus-summary-browse-url (&optional secondary)
"Scan the current article body for links, and offer to browse them.
-Links are opened using `browse-url' unless a prefix argument is
-given: then `browse-url-secondary-browser-function' is used instead.
+Links are opened using the primary browser unless a prefix argument is
+given. Then the secondary browser is used instead.
If only one link is found, browse that directly, otherwise use
completion to select a link. The first link marked in the
@@ -9429,9 +9429,7 @@ gnus-summary-browse-url
(gnus-shorten-url (car urls) 40))
urls nil t nil nil (car urls))))))
(if target
- (if external
- (funcall browse-url-secondary-browser-function target)
- (browse-url target))
+ (browse-url-with-function secondary target)
(message "No URLs found."))))
(defun gnus-summary-isearch-article (&optional regexp-p)
diff --git a/lisp/net/browse-url.el b/lisp/net/browse-url.el
index 9dd990108df..acc61162e15 100644
--- a/lisp/net/browse-url.el
+++ b/lisp/net/browse-url.el
@@ -888,6 +888,18 @@ browse-url-of-region
;; A generic command to call the current browse-url-browser-function
+(defun browse-url--transform (url)
+ "Transform URL according to `browse-url-transform-alist'."
+ (when browse-url-transform-alist
+ (dolist (trans browse-url-transform-alist)
+ (when (string-match (car trans) url)
+ (setq url (replace-match (cdr trans) nil t url)))))
+ (when (and url-handler-mode
+ (not (file-name-absolute-p url))
+ (not (string-match "\\`[a-z]+:" url)))
+ (setq url (expand-file-name url)))
+ url)
+
(declare-function pgtk-backend-display-class "pgtkfns.c" (&optional terminal))
;;;###autoload
@@ -916,14 +928,7 @@ browse-url
(interactive (browse-url-interactive-arg "URL: "))
(unless (called-interactively-p 'interactive)
(setq args (or args (list browse-url-new-window-flag))))
- (when browse-url-transform-alist
- (dolist (trans browse-url-transform-alist)
- (when (string-match (car trans) url)
- (setq url (replace-match (cdr trans) nil t url)))))
- (when (and url-handler-mode
- (not (file-name-absolute-p url))
- (not (string-match "\\`[a-z]+:" url)))
- (setq url (expand-file-name url)))
+ (setq url (browse-url--transform url))
(let ((process-environment (copy-sequence process-environment))
(function (or (browse-url-select-handler url)
browse-url-browser-function))
@@ -957,6 +962,19 @@ browse-url
(apply function url args)
(error "No suitable browser for URL %s" url))))
+;;;###autoload
+(defun browse-url-with-function (func url &rest args)
+ "Open URL with browser FUNC.
+If FUNC is a function use this function.
+If FUNC is nil use the `browse-url', which either calls a handler or the
+primary `browse-url-browser-function'.
+For other non-nil values use `browse-url-secondary-browser-function'."
+ (if (not func)
+ (apply #'browse-url url args)
+ (apply
+ (if (functionp func) func browse-url-secondary-browser-function)
+ (browse-url--transform url) args)))
+
;;;###autoload
(defun browse-url-at-point (&optional arg)
"Open URL at point using a configurable method.
@@ -1008,7 +1026,7 @@ browse-url-with-browser-kind
browse-url-secondary-browser-function
#'browse-url-default-browser
#'eww))))
- (funcall function url arg)))
+ (browse-url-with-function function url arg)))
;;;###autoload
(defun browse-url-at-mouse (event)
@@ -1815,27 +1833,23 @@ browse-url-add-buttons
browse-url-data ,(match-string 0)))))))
;;;###autoload
-(defun browse-url-button-open (&optional external mouse-event)
+(defun browse-url-button-open (&optional secondary mouse-event)
"Follow the link under point using `browse-url'.
-If EXTERNAL (the prefix if used interactively), open with the
-external browser instead of the default one."
+If SECONDARY (the prefix if used interactively), open with the
+secondary browser instead of the default one."
(interactive (list current-prefix-arg last-nonmenu-event))
(mouse-set-point mouse-event)
(let ((url (get-text-property (point) 'browse-url-data)))
(unless url
(error "No URL under point"))
- (if external
- (funcall browse-url-secondary-browser-function url)
- (browse-url url))))
+ (browse-url-with-function secondary url)))
;;;###autoload
(defun browse-url-button-open-url (url)
"Open URL using `browse-url'.
If `current-prefix-arg' is non-nil, use
`browse-url-secondary-browser-function' instead."
- (if current-prefix-arg
- (funcall browse-url-secondary-browser-function url)
- (browse-url url)))
+ (browse-url-with-function current-prefix-arg url))
(defun browse-url-button-copy ()
"Copy the URL under point."
diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index 842db3f27f4..a40232d977b 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -2166,7 +2166,7 @@ eww-browse-with-external-browser
(setq url (or url (plist-get eww-data :url)))
(if (eq 'external (browse-url--browser-kind
browse-url-secondary-browser-function url))
- (funcall browse-url-secondary-browser-function url)
+ (browse-url-with-function browse-url-secondary-browser-function url)
(browse-url-with-browser-kind 'external url)))
(defun eww-remove-tracking (url)
diff --git a/lisp/net/goto-addr.el b/lisp/net/goto-addr.el
index 3e5bc1ec090..59d6a01f585 100644
--- a/lisp/net/goto-addr.el
+++ b/lisp/net/goto-addr.el
@@ -227,9 +227,9 @@ goto-address-at-point
Compose message to address at point. See documentation for
`goto-address-find-address-at-point'.
-If no e-mail address is found at point, open the URL at or before
-point using `browse-url'. With a prefix argument, open the URL
-using `browse-url-secondary-browser-function' instead."
+If no e-mail address is found at point, open the URL at or before point
+using `browse-url-button-open-url'. With a prefix argument the
+secondary browser is used."
(interactive (list last-input-event))
(save-excursion
(if event (posn-set-point (event-end event)))
diff --git a/lisp/net/shr.el b/lisp/net/shr.el
index 6d8b235a2b8..da77bbf75a8 100644
--- a/lisp/net/shr.el
+++ b/lisp/net/shr.el
@@ -1097,24 +1097,22 @@ shr-mouse-browse-url-new-window
(mouse-set-point ev)
(shr-browse-url nil nil t))
-(defun shr-browse-url (&optional external mouse-event new-window)
+(defun shr-browse-url (&optional secondary mouse-event new-window)
"Browse the URL at point using `browse-url'.
-If EXTERNAL is non-nil (interactively, the prefix argument), browse
-the URL using `browse-url-secondary-browser-function'.
+If SECONDARY is non-nil (interactively, the prefix argument), browse
+the URL with the secondary browser.
If this function is invoked by a mouse click, it will browse the URL
at the position of the click. Optional argument MOUSE-EVENT describes
the mouse click event."
(interactive (list current-prefix-arg last-nonmenu-event))
(mouse-set-point mouse-event)
(let ((url (get-text-property (point) 'shr-url)))
- (cond
- ((not url)
+ (if (not url)
(message "No link under point"))
- (external
- (funcall browse-url-secondary-browser-function url)
- (shr--blink-link))
- (t
- (browse-url url (xor new-window browse-url-new-window-flag))))))
+ (browse-url-with-function secondary url
+ (xor new-window browse-url-new-window-flag))
+ (when secondary
+ (shr--blink-link))))
(defun shr-save-contents (directory)
"Save the contents from URL in a file."
--
2.45.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-12-24 1:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-23 18:41 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
2024-12-24 1:00 ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
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).