bug#69627: 30.0.50; [PATCH] Allow 'browse-url-interactive-arg' to return the actual prefix arg

bug#69627: 30.0.50; [PATCH] Allow 'browse-url-interactive-arg' to return the actual prefix arg

[Jim Porter]

[-- Attachment #1: Type: text/plain, Size: 447 bytes --]

By default this shouldn't do anything different from before. However, 
with this patch, functions for 'browse-url-browser-function' can look at 
the argument (as modified by 'browse-url-new-window-flag') and do 
something special. For example, with this patch, I have a custom browser 
function that works like this:

   M-x browse-url      Open in Firefox
   C-u M-x browse-url  Open in EWW
   C-- M-x browse-url  Open in Firefox Private Browsing

[-- Attachment #2: 0001-Let-browse-url-interactive-arg-return-more-values-fo.patch --]
[-- Type: text/plain, Size: 1338 bytes --]

From cae6d59d30d2082aba1899a1d34d1ee109f07c8d Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 7 Mar 2024 21:55:45 -0800
Subject: [PATCH] Let 'browse-url-interactive-arg' return more values for
 NEW-WINDOW-FLAG

Previously it always returned t or nil for NEW-WINDOW-FLAG, but now it
can return the actual prefix arg when appropriate.  This lets functions
for 'browse-url-browser-function' consult it and do more things than
just open a new window or not (for example, you could use "C--" as the
prefix arg to do something special in a custom function).

* lisp/net/browse-url.el (browse-url-interactive-arg):
---
 lisp/net/browse-url.el | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lisp/net/browse-url.el b/lisp/net/browse-url.el
index ddc57724343..a04b87827e3 100644
--- a/lisp/net/browse-url.el
+++ b/lisp/net/browse-url.el
@@ -715,8 +715,7 @@ browse-url-interactive-arg
 				      (buffer-substring-no-properties
 				       (region-beginning) (region-end))))
 				(browse-url-url-at-point)))
-	(not (eq (null browse-url-new-window-flag)
-		 (null current-prefix-arg)))))
+	(xor browse-url-new-window-flag current-prefix-arg)))
 
 ;; called-interactive-p needs to be called at a function's top-level, hence
 ;; this macro.  We use that rather than interactive-p because
-- 
2.25.1

bug#69627: 30.0.50; [PATCH] Allow 'browse-url-interactive-arg' to return the actual prefix arg

[Eli Zaretskii]

> Date: Thu, 7 Mar 2024 22:04:39 -0800
> From: Jim Porter <jporterbugs@gmail.com>
> 
> By default this shouldn't do anything different from before. However, 
> with this patch, functions for 'browse-url-browser-function' can look at 
> the argument (as modified by 'browse-url-new-window-flag') and do 
> something special. For example, with this patch, I have a custom browser 
> function that works like this:
> 
>    M-x browse-url      Open in Firefox
>    C-u M-x browse-url  Open in EWW
>    C-- M-x browse-url  Open in Firefox Private Browsing

Shouldn't the effect of this change be documented in some doc string?

Thanks.

bug#69627: 30.0.50; [PATCH] Allow 'browse-url-interactive-arg' to return the actual prefix arg

[Jim Porter]

[-- Attachment #1: Type: text/plain, Size: 291 bytes --]

On 3/7/2024 11:43 PM, Eli Zaretskii wrote:
> Shouldn't the effect of this change be documented in some doc string?

How about this? I'm not sure if using "xor" in the docstrings is 
over-explaining things, but then I don't know if there's a better way to 
describe exactly what would happen.

[-- Attachment #2: 0001-Let-browse-url-interactive-arg-return-more-values-fo.patch --]
[-- Type: text/plain, Size: 2846 bytes --]

From c4842dacc734b75747e66255bc12e175457c871a Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 7 Mar 2024 21:55:45 -0800
Subject: [PATCH] Let 'browse-url-interactive-arg' return more values for
 NEW-WINDOW-FLAG

Previously it always returned t or nil for NEW-WINDOW-FLAG, but now it
can return the actual prefix arg when appropriate.  This lets functions
for 'browse-url-browser-function' consult it and do more things than
just open a new window or not (for example, you could use "C--" as the
prefix arg to do something special in a custom function).

* lisp/net/browse-url.el (browse-url-interactive-arg): Use 'xor' to
adjust the value of 'current-prefix-arg'.
(browse-url): Update docstring.
---
 lisp/net/browse-url.el | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lisp/net/browse-url.el b/lisp/net/browse-url.el
index ddc57724343..8d28a6ce7c6 100644
--- a/lisp/net/browse-url.el
+++ b/lisp/net/browse-url.el
@@ -704,8 +704,9 @@ browse-url-interactive-arg
 point.  If invoked with a mouse button, it moves point to the
 position clicked before acting.
 
-This function returns a list (URL NEW-WINDOW-FLAG)
-for use in `interactive'."
+This function returns a list (URL NEW-WINDOW-FLAG) for use in
+`interactive'.  NEW-WINDOW-FLAG is either the prefix arg xor
+`browse-url-new-window-flag'."
   (let ((event (elt (this-command-keys) 0)))
     (mouse-set-point event))
   (list (read-string prompt (or (and transient-mark-mode mark-active
@@ -715,8 +716,7 @@ browse-url-interactive-arg
 				      (buffer-substring-no-properties
 				       (region-beginning) (region-end))))
 				(browse-url-url-at-point)))
-	(not (eq (null browse-url-new-window-flag)
-		 (null current-prefix-arg)))))
+	(xor browse-url-new-window-flag current-prefix-arg)))
 
 ;; called-interactive-p needs to be called at a function's top-level, hence
 ;; this macro.  We use that rather than interactive-p because
@@ -879,8 +879,8 @@ browse-url
 `browse-url-handlers', and `browse-url-default-handlers'
 determine which browser function to use.
 
-This command prompts for a URL, defaulting to the URL at or
-before point.
+Interactively, this command prompts for a URL, defaulting to the
+URL at or before point.
 
 The additional ARGS are passed to the browser function.  See the
 doc strings of the actual functions, starting with
@@ -888,7 +888,8 @@ browse-url
 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."
+`browse-url-new-window-flag' as ARGS.  Interactively, pass the
+prefix arg xor `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))))
-- 
2.25.1

bug#69627: 30.0.50; [PATCH] Allow 'browse-url-interactive-arg' to return the actual prefix arg

[Eli Zaretskii]

> Date: Fri, 8 Mar 2024 10:29:05 -0800
> Cc: 69627@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> On 3/7/2024 11:43 PM, Eli Zaretskii wrote:
> > Shouldn't the effect of this change be documented in some doc string?
> 
> How about this? I'm not sure if using "xor" in the docstrings is 
> over-explaining things, but then I don't know if there's a better way to 
> describe exactly what would happen.

Just explain it in plain English, okay?

bug#69627: 30.0.50; [PATCH] Allow 'browse-url-interactive-arg' to return the actual prefix arg

[Jim Porter]

On 3/8/2024 11:46 AM, Eli Zaretskii wrote:
>> Date: Fri, 8 Mar 2024 10:29:05 -0800
>> Cc: 69627@debbugs.gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> On 3/7/2024 11:43 PM, Eli Zaretskii wrote:
>>> Shouldn't the effect of this change be documented in some doc string?
>>
>> How about this? I'm not sure if using "xor" in the docstrings is
>> over-explaining things, but then I don't know if there's a better way to
>> describe exactly what would happen.
> 
> Just explain it in plain English, okay?

Something like this?

"NEW-WINDOW-FLAG is the prefix arg; if `browse-url-new-window-flag' is 
non-nil, invert the prefix arg instead."

That's the conceptual idea behind 'browse-url-new-window-flag' anyway, 
and the defcustom is specified to be only a boolean, so the above should 
hopefully be enough.

bug#69627: 30.0.50; [PATCH] Allow 'browse-url-interactive-arg' to return the actual prefix arg

[Eli Zaretskii]

> Date: Fri, 8 Mar 2024 12:03:58 -0800
> Cc: 69627@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> On 3/8/2024 11:46 AM, Eli Zaretskii wrote:
> >>
> >> How about this? I'm not sure if using "xor" in the docstrings is
> >> over-explaining things, but then I don't know if there's a better way to
> >> describe exactly what would happen.
> > 
> > Just explain it in plain English, okay?
> 
> Something like this?
> 
> "NEW-WINDOW-FLAG is the prefix arg; if `browse-url-new-window-flag' is 
> non-nil, invert the prefix arg instead."

Yes, something like this.

bug#69627: 30.0.50; [PATCH] Allow 'browse-url-interactive-arg' to return the actual prefix arg

[Jim Porter]

[-- Attachment #1: Type: text/plain, Size: 179 bytes --]

On 3/8/2024 12:18 PM, Eli Zaretskii wrote:
> Yes, something like this.

Thanks. Updated patch attached. If no one has any other concerns, I'll 
merge this in the next day or two.

[-- Attachment #2: 0001-Let-browse-url-interactive-arg-return-more-values-fo.patch --]
[-- Type: text/plain, Size: 2929 bytes --]

From 7b8efb4af80504b621f0f6929469937f2cf4a2e9 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 7 Mar 2024 21:55:45 -0800
Subject: [PATCH] Let 'browse-url-interactive-arg' return more values for
 NEW-WINDOW-FLAG

Previously it always returned t or nil for NEW-WINDOW-FLAG, but now it
can return the actual prefix arg when appropriate.  This lets functions
for 'browse-url-browser-function' consult it and do more things than
just open a new window or not (for example, you could use "C--" as the
prefix arg to do something special in a custom function).

* lisp/net/browse-url.el (browse-url-interactive-arg): Use 'xor' to
adjust the value of 'current-prefix-arg'.
(browse-url): Update docstring.
---
 lisp/net/browse-url.el | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/lisp/net/browse-url.el b/lisp/net/browse-url.el
index ddc57724343..f22aa19f5e3 100644
--- a/lisp/net/browse-url.el
+++ b/lisp/net/browse-url.el
@@ -704,8 +704,10 @@ browse-url-interactive-arg
 point.  If invoked with a mouse button, it moves point to the
 position clicked before acting.
 
-This function returns a list (URL NEW-WINDOW-FLAG)
-for use in `interactive'."
+This function returns a list (URL NEW-WINDOW-FLAG) for use in
+`interactive'.  NEW-WINDOW-FLAG is the prefix arg; if
+`browse-url-new-window-flag' is non-nil, invert the prefix arg
+instead."
   (let ((event (elt (this-command-keys) 0)))
     (mouse-set-point event))
   (list (read-string prompt (or (and transient-mark-mode mark-active
@@ -715,8 +717,7 @@ browse-url-interactive-arg
 				      (buffer-substring-no-properties
 				       (region-beginning) (region-end))))
 				(browse-url-url-at-point)))
-	(not (eq (null browse-url-new-window-flag)
-		 (null current-prefix-arg)))))
+	(xor browse-url-new-window-flag current-prefix-arg)))
 
 ;; called-interactive-p needs to be called at a function's top-level, hence
 ;; this macro.  We use that rather than interactive-p because
@@ -879,8 +880,8 @@ browse-url
 `browse-url-handlers', and `browse-url-default-handlers'
 determine which browser function to use.
 
-This command prompts for a URL, defaulting to the URL at or
-before point.
+Interactively, this command prompts for a URL, defaulting to the
+URL at or before point.
 
 The additional ARGS are passed to the browser function.  See the
 doc strings of the actual functions, starting with
@@ -888,7 +889,9 @@ browse-url
 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."
+`browse-url-new-window-flag' as ARGS.  Interactively, pass the
+prefix arg as ARGS; if `browse-url-new-window-flag' is non-nil,
+invert the prefix arg instead."
   (interactive (browse-url-interactive-arg "URL: "))
   (unless (called-interactively-p 'interactive)
     (setq args (or args (list browse-url-new-window-flag))))
-- 
2.25.1

bug#69627: 30.0.50; [PATCH] Allow 'browse-url-interactive-arg' to return the actual prefix arg

[Jim Porter]

On 3/8/2024 1:02 PM, Jim Porter wrote:
> On 3/8/2024 12:18 PM, Eli Zaretskii wrote:
>> Yes, something like this.
> 
> Thanks. Updated patch attached. If no one has any other concerns, I'll 
> merge this in the next day or two.

Now merged to master as 46afc91c9f7, and closing this bug.