GNU bug report logs - #39353
[PATCH] Use current-prefix-arg instead of prefix-arg

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 39353 in the body.
You can then email your comments to 39353 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: Kyle Hubert <khubert <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Cc: Kyle Hubert <khubert <at> gmail.com>
Subject: [PATCH] Use current-prefix-arg instead of prefix-arg
Date: Wed, 29 Jan 2020 22:35:04 -0500

Forwarding arguments through to children function invocations should
be handled by current-prefix-arg. This was first uncovered when ediff
used full window width scrolling in ediff-scroll-horizontally, when
the call to scroll-left or scroll-right should have been half-window
size. Through further grepping, there was another usage in
simple.el. Note, execute-extended-command was visually inspected that
it was in interactive mode.

---
 lisp/simple.el        | 2 +-
 lisp/vc/ediff-util.el | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index 2ec3da680f..53afa7b138 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1888,7 +1888,7 @@ invoking, give a prefix argument to `execute-extended-command'."
     ;; `function' and not `execute-extended-command'.  The difference is
     ;; visible in cases such as M-x <cmd> RET and then C-x z (bug#11506).
     (setq real-this-command function)
-    (let ((prefix-arg prefixarg))
+    (let ((current-prefix-arg prefixarg))
       (command-execute function 'record))
     ;; If enabled, show which key runs this command.
     ;; But first wait, and skip the message if there is input.
diff --git a/lisp/vc/ediff-util.el b/lisp/vc/ediff-util.el
index a8af9ba37a..5f8a4a86b1 100644
--- a/lisp/vc/ediff-util.el
+++ b/lisp/vc/ediff-util.el
@@ -1540,10 +1540,10 @@ the width of the A/B/C windows."
    ;; hscrolling.
    (if (= last-command-event ?<)
        (lambda (arg)
-	 (let ((prefix-arg arg))
+	 (let ((current-prefix-arg arg))
 	   (call-interactively #'scroll-left)))
      (lambda (arg)
-       (let ((prefix-arg arg))
+       (let ((current-prefix-arg arg))
 	 (call-interactively #'scroll-right))))
    ;; calculate argument to scroll-left/right
    ;; if there is an explicit argument
-- 
2.25.0

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

From: Kyle Hubert <khubert <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: [PATCH] Use current-prefix-arg instead of prefix-arg
Date: Wed, 29 Jan 2020 22:53:58 -0500

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

I have a hard time testing the change to simple.el, as I don't understand
execute-extended-command. Can anyone help here? I'm worried since it isn't
using (interactive "P") that this is incorrect. I admit I'm deeper in the
guts of emacs than typical.

Thanks,
-Kyle


On Wed, Jan 29, 2020 at 10:35 PM Kyle Hubert <khubert <at> gmail.com> wrote:

> Forwarding arguments through to children function invocations should
> be handled by current-prefix-arg. This was first uncovered when ediff
> used full window width scrolling in ediff-scroll-horizontally, when
> the call to scroll-left or scroll-right should have been half-window
> size. Through further grepping, there was another usage in
> simple.el. Note, execute-extended-command was visually inspected that
> it was in interactive mode.
>
> ---
>  lisp/simple.el        | 2 +-
>  lisp/vc/ediff-util.el | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lisp/simple.el b/lisp/simple.el
> index 2ec3da680f..53afa7b138 100644
> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -1888,7 +1888,7 @@ invoking, give a prefix argument to
> `execute-extended-command'."
>      ;; `function' and not `execute-extended-command'.  The difference is
>      ;; visible in cases such as M-x <cmd> RET and then C-x z (bug#11506).
>      (setq real-this-command function)
> -    (let ((prefix-arg prefixarg))
> +    (let ((current-prefix-arg prefixarg))
>        (command-execute function 'record))
>      ;; If enabled, show which key runs this command.
>      ;; But first wait, and skip the message if there is input.
> diff --git a/lisp/vc/ediff-util.el b/lisp/vc/ediff-util.el
> index a8af9ba37a..5f8a4a86b1 100644
> --- a/lisp/vc/ediff-util.el
> +++ b/lisp/vc/ediff-util.el
> @@ -1540,10 +1540,10 @@ the width of the A/B/C windows."
>     ;; hscrolling.
>     (if (= last-command-event ?<)
>         (lambda (arg)
> -        (let ((prefix-arg arg))
> +        (let ((current-prefix-arg arg))
>            (call-interactively #'scroll-left)))
>       (lambda (arg)
> -       (let ((prefix-arg arg))
> +       (let ((current-prefix-arg arg))
>          (call-interactively #'scroll-right))))
>     ;; calculate argument to scroll-left/right
>     ;; if there is an explicit argument
> --
> 2.25.0
>
>

[Message part 2 (text/html, inline)]

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Kyle Hubert <khubert <at> gmail.com>
Cc: 39353 <at> debbugs.gnu.org
Subject: Re: bug#39353: [PATCH] Use current-prefix-arg instead of prefix-arg
Date: Thu, 30 Jan 2020 08:12:27 -0500

Kyle Hubert <khubert <at> gmail.com> writes:

> I have a hard time testing the change to simple.el, as I don't understand
> execute-extended-command. Can anyone help here? I'm worried since it isn't
> using (interactive "P") that this is incorrect. I admit I'm deeper in the
> guts of emacs than typical.

The simple.el let-binding is around command-execute, not
execute-extended-command.  command-execute does specifically read
prefix-arg, so I think that part of your patch should be dropped (I
haven't looked in detail at the ediff part, but it sounds right).

>> --- a/lisp/simple.el
>> +++ b/lisp/simple.el
>> @@ -1888,7 +1888,7 @@ invoking, give a prefix argument to
>> `execute-extended-command'."
>>      ;; `function' and not `execute-extended-command'.  The difference is
>>      ;; visible in cases such as M-x <cmd> RET and then C-x z (bug#11506).
>>      (setq real-this-command function)
>> -    (let ((prefix-arg prefixarg))
>> +    (let ((current-prefix-arg prefixarg))
>>        (command-execute function 'record))

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

From: Kyle Hubert <khubert <at> gmail.com>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 39353 <at> debbugs.gnu.org
Subject: Re: bug#39353: [PATCH] Use current-prefix-arg instead of prefix-arg
Date: Thu, 30 Jan 2020 11:35:01 -0500

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

Thank you for the review. I had a feeling that
execute-extended-command's calling of prefix-arg was the correct
usage for command-execute. If I drop that from the PATCH, do I
repost to this bug ID email? I'm unfamiliar with the project.

-Kyle

On Thu, Jan 30, 2020 at 8:12 AM Noam Postavsky <npostavs <at> gmail.com> wrote:

> Kyle Hubert <khubert <at> gmail.com> writes:
>
> > I have a hard time testing the change to simple.el, as I don't understand
> > execute-extended-command. Can anyone help here? I'm worried since it
> isn't
> > using (interactive "P") that this is incorrect. I admit I'm deeper in the
> > guts of emacs than typical.
>
> The simple.el let-binding is around command-execute, not
> execute-extended-command.  command-execute does specifically read
> prefix-arg, so I think that part of your patch should be dropped (I
> haven't looked in detail at the ediff part, but it sounds right).
>
> >> --- a/lisp/simple.el
> >> +++ b/lisp/simple.el
> >> @@ -1888,7 +1888,7 @@ invoking, give a prefix argument to
> >> `execute-extended-command'."
> >>      ;; `function' and not `execute-extended-command'.  The difference
> is
> >>      ;; visible in cases such as M-x <cmd> RET and then C-x z
> (bug#11506).
> >>      (setq real-this-command function)
> >> -    (let ((prefix-arg prefixarg))
> >> +    (let ((current-prefix-arg prefixarg))
> >>        (command-execute function 'record))
>

[Message part 2 (text/html, inline)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kyle Hubert <khubert <at> gmail.com>
Cc: 39353 <at> debbugs.gnu.org, npostavs <at> gmail.com
Subject: Re: bug#39353: [PATCH] Use current-prefix-arg instead of prefix-arg
Date: Thu, 30 Jan 2020 20:39:33 +0200

> From: Kyle Hubert <khubert <at> gmail.com>
> Date: Thu, 30 Jan 2020 11:35:01 -0500
> Cc: 39353 <at> debbugs.gnu.org
> 
> If I drop that from the PATCH, do I repost to this bug ID email?

Yes, please.

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

From: Kyle Hubert <khubert <at> gmail.com>
To: 39353 <at> debbugs.gnu.org
Cc: Kyle Hubert <khubert <at> gmail.com>
Subject: [PATCH] Ediff fix for ediff-scroll-horizontally
Date: Fri, 31 Jan 2020 09:29:43 -0500

When ediff-scroll-horizontally interactively calls scroll-left and
scroll-right, current-prefix-arg should be used. This allows for the
case that when no args or '-' are passed to ediff-scroll-horizontally,
it will determine the default amount (half the window width or
half-again, respectively).
---
 lisp/vc/ediff-util.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/vc/ediff-util.el b/lisp/vc/ediff-util.el
index a8af9ba37a..5f8a4a86b1 100644
--- a/lisp/vc/ediff-util.el
+++ b/lisp/vc/ediff-util.el
@@ -1540,10 +1540,10 @@ the width of the A/B/C windows."
    ;; hscrolling.
    (if (= last-command-event ?<)
        (lambda (arg)
-	 (let ((prefix-arg arg))
+	 (let ((current-prefix-arg arg))
 	   (call-interactively #'scroll-left)))
      (lambda (arg)
-       (let ((prefix-arg arg))
+       (let ((current-prefix-arg arg))
 	 (call-interactively #'scroll-right))))
    ;; calculate argument to scroll-left/right
    ;; if there is an explicit argument
-- 
2.25.0

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kyle Hubert <khubert <at> gmail.com>
Cc: 39353-done <at> debbugs.gnu.org
Subject: Re: bug#39353: [PATCH] Ediff fix for ediff-scroll-horizontally
Date: Sat, 08 Feb 2020 12:01:59 +0200

> From: Kyle Hubert <khubert <at> gmail.com>
> Date: Fri, 31 Jan 2020 09:29:43 -0500
> Cc: Kyle Hubert <khubert <at> gmail.com>
> 
> When ediff-scroll-horizontally interactively calls scroll-left and
> scroll-right, current-prefix-arg should be used. This allows for the
> case that when no args or '-' are passed to ediff-scroll-horizontally,
> it will determine the default amount (half the window width or
> half-again, respectively).
> ---
>  lisp/vc/ediff-util.el | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Thanks, pushed to the master branch.

Previous Next

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