bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

Hello,

vc-git-log-edit-mode is already able to properly understand git-log(1)
output which logs multiple branches.  Here's a hack to demonstrate that:

    (let ((default-directory "~/src/emacs/")
          (vc-git-log-switches
           (cons "origin/scratch/posix-spawn" vc-git-log-switches)))
      (call-process-shell-command "git branch -f tmp 4b2ca6bfc0")
      (vc-print-branch-log "tmp"))

(4b2ca6bfc0 is an old tip of master close to scratch/posix-spawn.)

An example of a case in which it is useful to produce logs like this is
when you're trying to figure out how several feature branches relate to
each other and to the trunk.

I don't think the idea of producing a log showing more than one branch
at once is fundamentally git-specific.  So I would like to extend
vc-print-branch-log to allow logging more than one branch, or
alternatively, add a vc-print-branch-logs command.  I believe that there
are two design issues, on which I'm requesting review/input:

(1) Extending the VC history functions API to permit expressing a
request for the backend to print a log of multiple branches.  We could
allow the START-REVISION argument to `print-log' to take a list of
branches, or add an additional optional argument
ADDITIONAL-START-REVISIONS.  The latter seems more robust to me.

(2) How could vc-print-branch-log prompt the user for multiple branch
names?  Magit has the user separate branch names with commas, offering
completion for each branch name.  But I suppose commas never appearing
in branch names is git-specific.  So the command could prompt the user
repeatedly for branch names, until the user hits RET to input a
zero-length branch name, indicating they've input them all.

-- 
Sean Whitton

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Date: Wed, 14 Sep 2022 10:19:48 -0700
> 
> vc-git-log-edit-mode is already able to properly understand git-log(1)
> output which logs multiple branches.  Here's a hack to demonstrate that:
> 
>     (let ((default-directory "~/src/emacs/")
>           (vc-git-log-switches
>            (cons "origin/scratch/posix-spawn" vc-git-log-switches)))
>       (call-process-shell-command "git branch -f tmp 4b2ca6bfc0")
>       (vc-print-branch-log "tmp"))
> 
> (4b2ca6bfc0 is an old tip of master close to scratch/posix-spawn.)
> 
> An example of a case in which it is useful to produce logs like this is
> when you're trying to figure out how several feature branches relate to
> each other and to the trunk.
> 
> I don't think the idea of producing a log showing more than one branch
> at once is fundamentally git-specific.  So I would like to extend
> vc-print-branch-log to allow logging more than one branch, or
> alternatively, add a vc-print-branch-logs command.  I believe that there
> are two design issues, on which I'm requesting review/input:
> 
> (1) Extending the VC history functions API to permit expressing a
> request for the backend to print a log of multiple branches.  We could
> allow the START-REVISION argument to `print-log' to take a list of
> branches, or add an additional optional argument
> ADDITIONAL-START-REVISIONS.  The latter seems more robust to me.
> 
> (2) How could vc-print-branch-log prompt the user for multiple branch
> names?  Magit has the user separate branch names with commas, offering
> completion for each branch name.  But I suppose commas never appearing
> in branch names is git-specific.  So the command could prompt the user
> repeatedly for branch names, until the user hits RET to input a
> zero-length branch name, indicating they've input them all.

I'm not sure I understand: how is this different from what "C-x v L"
already produces?

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

Hello,

On Wed 14 Sep 2022 at 08:41PM +03, Eli Zaretskii wrote:

> I'm not sure I understand: how is this different from what "C-x v L"
> already produces?

Here's an example showing logging two branches "melete" and "erebus" in
a repo of mine:

    * 8d679018 (origin/erebus) sync from melete ~/doc on erebus
    * 44398919 use light-on-dark on erebus
    * 272fc7a6 manual DPMS on erebus since it means no sound
    * 22fc8f11 erebus wallpaper
    * 512eb187 erebus terminal font size
    | * 038c0a31 (HEAD -> melete, origin/melete) sync from erebus ~/doc on melete
    | * 8197e31e modus-vivendi on melete for erebus
    |/
    * c3f9d152 (origin/master, origin/HEAD, master) attempt to ensure [...]
    * 643b20f6 spw/maybe-scale-basic-faces: simplify checking fonts were found

From this view one is able to see how erebus is 5 commits ahead of
master and melete is two (different) commits ahead of master.

-- 
Sean Whitton

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Date: Wed, 14 Sep 2022 15:42:54 -0700
> 
> On Wed 14 Sep 2022 at 08:41PM +03, Eli Zaretskii wrote:
> 
> > I'm not sure I understand: how is this different from what "C-x v L"
> > already produces?
> 
> Here's an example showing logging two branches "melete" and "erebus" in
> a repo of mine:
> 
>     * 8d679018 (origin/erebus) sync from melete ~/doc on erebus
>     * 44398919 use light-on-dark on erebus
>     * 272fc7a6 manual DPMS on erebus since it means no sound
>     * 22fc8f11 erebus wallpaper
>     * 512eb187 erebus terminal font size
>     | * 038c0a31 (HEAD -> melete, origin/melete) sync from erebus ~/doc on melete
>     | * 8197e31e modus-vivendi on melete for erebus
>     |/
>     * c3f9d152 (origin/master, origin/HEAD, master) attempt to ensure [...]
>     * 643b20f6 spw/maybe-scale-basic-faces: simplify checking fonts were found
> 
> >From this view one is able to see how erebus is 5 commits ahead of
> master and melete is two (different) commits ahead of master.

So the difference is that what you want to see is all the branches up
to their HEAD, and not just branches merged into the current branch,
is that right?

If so, I guess some optional feature of "C-x v L" could do that.
However, if the repository has many branches (and ours, for example,
does), wouldn't that make for a crowded and hard-t-read log buffer?
Perhaps asking the user for a name of one branch will be enough?  More
than one branch could be problematic on display, I think.

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

Hello,

On Thu 15 Sep 2022 at 08:33AM +03, Eli Zaretskii wrote:

> So the difference is that what you want to see is all the branches up
> to their HEAD, and not just branches merged into the current branch,
> is that right?
>
> If so, I guess some optional feature of "C-x v L" could do that.
> However, if the repository has many branches (and ours, for example,
> does), wouldn't that make for a crowded and hard-t-read log buffer?
> Perhaps asking the user for a name of one branch will be enough?  More
> than one branch could be problematic on display, I think.

Not all branches, as indeed that would quickly become unreadable.  The
user has to say exactly which branches they would like to be included.
And usually you wouldn't type more than two or three.

-- 
Sean Whitton

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Date: Thu, 15 Sep 2022 09:18:13 -0700
> 
> Hello,
> 
> On Thu 15 Sep 2022 at 08:33AM +03, Eli Zaretskii wrote:
> 
> > So the difference is that what you want to see is all the branches up
> > to their HEAD, and not just branches merged into the current branch,
> > is that right?
> >
> > If so, I guess some optional feature of "C-x v L" could do that.
> > However, if the repository has many branches (and ours, for example,
> > does), wouldn't that make for a crowded and hard-t-read log buffer?
> > Perhaps asking the user for a name of one branch will be enough?  More
> > than one branch could be problematic on display, I think.
> 
> Not all branches, as indeed that would quickly become unreadable.  The
> user has to say exactly which branches they would like to be included.
> And usually you wouldn't type more than two or three.

I think 3 is already too much for the largely unidimensional display
we (and Git) provide.

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

Hello,

On Thu 15 Sep 2022 at 07:48PM +03, Eli Zaretskii wrote:

>> From: Sean Whitton <spwhitton@spwhitton.name>
>> Date: Thu, 15 Sep 2022 09:18:13 -0700
>>
>> Hello,
>>
>> On Thu 15 Sep 2022 at 08:33AM +03, Eli Zaretskii wrote:
>>
>> > So the difference is that what you want to see is all the branches up
>> > to their HEAD, and not just branches merged into the current branch,
>> > is that right?
>> >
>> > If so, I guess some optional feature of "C-x v L" could do that.
>> > However, if the repository has many branches (and ours, for example,
>> > does), wouldn't that make for a crowded and hard-t-read log buffer?
>> > Perhaps asking the user for a name of one branch will be enough?  More
>> > than one branch could be problematic on display, I think.
>> 
>> Not all branches, as indeed that would quickly become unreadable.  The
>> user has to say exactly which branches they would like to be included.
>> And usually you wouldn't type more than two or three.
>
> I think 3 is already too much for the largely unidimensional display
> we (and Git) provide.

Fair enough, but I find it useful and Magit having this feature suggests
others do too.

-- 
Sean Whitton

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Juri Linkov]

> vc-git-log-edit-mode is already able to properly understand git-log(1)

I suppose vc-git-log-view-mode, not vc-git-log-edit-mode?

> output which logs multiple branches.  Here's a hack to demonstrate that:
>
>     (let ((default-directory "~/src/emacs/")
>           (vc-git-log-switches
>            (cons "origin/scratch/posix-spawn" vc-git-log-switches)))
>       (call-process-shell-command "git branch -f tmp 4b2ca6bfc0")
>       (vc-print-branch-log "tmp"))
>
> (4b2ca6bfc0 is an old tip of master close to scratch/posix-spawn.)

Is this equivalent to `M-& git log origin/scratch/posix-spawn 4b2ca6bfc0`,
then enabling vc-git-log-view-mode in the output buffer?

> An example of a case in which it is useful to produce logs like this is
> when you're trying to figure out how several feature branches relate to
> each other and to the trunk.

I never encountered anything like this.  I wonder if this is a common need?

> I don't think the idea of producing a log showing more than one branch
> at once is fundamentally git-specific.  So I would like to extend
> vc-print-branch-log to allow logging more than one branch, or
> alternatively, add a vc-print-branch-logs command.

Maybe vc-print-branch-logs, to keep the original vc-print-branch-log
uncomplicated.

> I believe that there are two design issues, on which I'm requesting
> review/input:
>
> (1) Extending the VC history functions API to permit expressing a
> request for the backend to print a log of multiple branches.  We could
> allow the START-REVISION argument to `print-log' to take a list of
> branches, or add an additional optional argument
> ADDITIONAL-START-REVISIONS.  The latter seems more robust to me.
>
> (2) How could vc-print-branch-log prompt the user for multiple branch
> names?  Magit has the user separate branch names with commas, offering
> completion for each branch name.  But I suppose commas never appearing
> in branch names is git-specific.  So the command could prompt the user
> repeatedly for branch names, until the user hits RET to input a
> zero-length branch name, indicating they've input them all.

Maybe by using completing-read-multiple with branch names separated
by crm-separator, not necessarily comma?

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

Hello,

On Wed 14 Sep 2022 at 10:17PM +03, Juri Linkov wrote:

>> vc-git-log-edit-mode is already able to properly understand git-log(1)
>
> I suppose vc-git-log-view-mode, not vc-git-log-edit-mode?

Yes, sorry.

>> output which logs multiple branches.  Here's a hack to demonstrate that:
>>
>>     (let ((default-directory "~/src/emacs/")
>>           (vc-git-log-switches
>>            (cons "origin/scratch/posix-spawn" vc-git-log-switches)))
>>       (call-process-shell-command "git branch -f tmp 4b2ca6bfc0")
>>       (vc-print-branch-log "tmp"))
>>
>> (4b2ca6bfc0 is an old tip of master close to scratch/posix-spawn.)
>
> Is this equivalent to `M-& git log origin/scratch/posix-spawn 4b2ca6bfc0`,
> then enabling vc-git-log-view-mode in the output buffer?

It's not, because vc-git-log-view-mode requires you pass four or five
options to git-log to get output it can properly display.  That
knowledge is locked up in vc-git.el right now.

> Maybe vc-print-branch-logs, to keep the original vc-print-branch-log
> uncomplicated.

Perhaps a prefix argument?

>> (2) How could vc-print-branch-log prompt the user for multiple branch
>> names?  Magit has the user separate branch names with commas, offering
>> completion for each branch name.  But I suppose commas never appearing
>> in branch names is git-specific.  So the command could prompt the user
>> repeatedly for branch names, until the user hits RET to input a
>> zero-length branch name, indicating they've input them all.
>
> Maybe by using completing-read-multiple with branch names separated
> by crm-separator, not necessarily comma?

I guess it would be a strange VC system that allows spaces in branch
names, huh?

-- 
Sean Whitton

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Juri Linkov]

>> Is this equivalent to `M-& git log origin/scratch/posix-spawn 4b2ca6bfc0`,
>> then enabling vc-git-log-view-mode in the output buffer?
>
> It's not, because vc-git-log-view-mode requires you pass four or five
> options to git-log to get output it can properly display.  That
> knowledge is locked up in vc-git.el right now.

This part is clear, but I also wanted to know what git command
corresponds to this feature.  Is it just `git log` with two
branch names on the command line?  What additional options
are required?  Maybe `--graph --oneline`?  Also I see that
git documentation mentions `--branches[=<pattern>]`.

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

Hello,

On Thu 15 Sep 2022 at 09:59AM +03, Juri Linkov wrote:

>>> Is this equivalent to `M-& git log origin/scratch/posix-spawn 4b2ca6bfc0`,
>>> then enabling vc-git-log-view-mode in the output buffer?
>>
>> It's not, because vc-git-log-view-mode requires you pass four or five
>> options to git-log to get output it can properly display.  That
>> knowledge is locked up in vc-git.el right now.
>
> This part is clear, but I also wanted to know what git command
> corresponds to this feature.  Is it just `git log` with two
> branch names on the command line?  What additional options
> are required?  Maybe `--graph --oneline`?  Also I see that
> git documentation mentions `--branches[=<pattern>]`.

It's something along these lines:

git log -100 --graph --decorate --date=short \
    --pretty="tformat:%d%h..: %an %ad %s" --abbrev-commit \
    BRANCH1 BRANCH2

-- 
Sean Whitton

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Juri Linkov]

>>>> Is this equivalent to `M-& git log origin/scratch/posix-spawn 4b2ca6bfc0`,
>>>> then enabling vc-git-log-view-mode in the output buffer?
>>>
>>> It's not, because vc-git-log-view-mode requires you pass four or five
>>> options to git-log to get output it can properly display.  That
>>> knowledge is locked up in vc-git.el right now.
>>
>> This part is clear, but I also wanted to know what git command
>> corresponds to this feature.  Is it just `git log` with two
>> branch names on the command line?  What additional options
>> are required?  Maybe `--graph --oneline`?  Also I see that
>> git documentation mentions `--branches[=<pattern>]`.
>
> It's something along these lines:
>
> git log -100 --graph --decorate --date=short \
>     --pretty="tformat:%d%h..: %an %ad %s" --abbrev-commit \
>     BRANCH1 BRANCH2

This is what I proposed a long ago, maybe it makes sense here.
After invoking a vc command with a prefix argument it could pop up
the minibuffer with the command line that you can edit to add or delete
more arguments for the git command.  So e.g. after 'C-u C-x v b l BRANCH1 RET'
it will insert in the minibuffer the default command `git log ... BRANCH1`,
where you can add BRANCH2 and modify any other option.  Then it runs
this command and displays the output in the vc-git-log-view-mode buffer.

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

Hello,

On Thu 15 Sep 2022 at 08:27PM +03, Juri Linkov wrote:

>>>>> Is this equivalent to `M-& git log origin/scratch/posix-spawn 4b2ca6bfc0`,
>>>>> then enabling vc-git-log-view-mode in the output buffer?
>>>>
>>>> It's not, because vc-git-log-view-mode requires you pass four or five
>>>> options to git-log to get output it can properly display.  That
>>>> knowledge is locked up in vc-git.el right now.
>>>
>>> This part is clear, but I also wanted to know what git command
>>> corresponds to this feature.  Is it just `git log` with two
>>> branch names on the command line?  What additional options
>>> are required?  Maybe `--graph --oneline`?  Also I see that
>>> git documentation mentions `--branches[=<pattern>]`.
>>
>> It's something along these lines:
>>
>> git log -100 --graph --decorate --date=short \
>>     --pretty="tformat:%d%h..: %an %ad %s" --abbrev-commit \
>>     BRANCH1 BRANCH2
>
> This is what I proposed a long ago, maybe it makes sense here.
> After invoking a vc command with a prefix argument it could pop up
> the minibuffer with the command line that you can edit to add or delete
> more arguments for the git command.  So e.g. after 'C-u C-x v b l BRANCH1 RET'
> it will insert in the minibuffer the default command `git log ... BRANCH1`,
> where you can add BRANCH2 and modify any other option.  Then it runs
> this command and displays the output in the vc-git-log-view-mode buffer.

That would be enough for me.  vc-push already works that way.  Shall I
look into preparing a patch?

-- 
Sean Whitton

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Juri Linkov]

>>>>>> Is this equivalent to `M-& git log origin/scratch/posix-spawn 4b2ca6bfc0`,
>>>>>> then enabling vc-git-log-view-mode in the output buffer?
>>>>>
>>>>> It's not, because vc-git-log-view-mode requires you pass four or five
>>>>> options to git-log to get output it can properly display.  That
>>>>> knowledge is locked up in vc-git.el right now.
>>>>
>>>> This part is clear, but I also wanted to know what git command
>>>> corresponds to this feature.  Is it just `git log` with two
>>>> branch names on the command line?  What additional options
>>>> are required?  Maybe `--graph --oneline`?  Also I see that
>>>> git documentation mentions `--branches[=<pattern>]`.
>>>
>>> It's something along these lines:
>>>
>>> git log -100 --graph --decorate --date=short \
>>>     --pretty="tformat:%d%h..: %an %ad %s" --abbrev-commit \
>>>     BRANCH1 BRANCH2
>>
>> This is what I proposed a long ago, maybe it makes sense here.
>> After invoking a vc command with a prefix argument it could pop up
>> the minibuffer with the command line that you can edit to add or delete
>> more arguments for the git command.  So e.g. after 'C-u C-x v b l BRANCH1 RET'
>> it will insert in the minibuffer the default command `git log ... BRANCH1`,
>> where you can add BRANCH2 and modify any other option.  Then it runs
>> this command and displays the output in the vc-git-log-view-mode buffer.
>
> That would be enough for me.  vc-push already works that way.  Shall I
> look into preparing a patch?

Yes, please try to prepare a patch, preferably without changing the
established backend API, maybe by using a global variable that later
could be reused to confirm and edit other commands.

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

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

Hello,

On Fri 16 Sep 2022 at 09:59AM +03, Juri Linkov wrote:

> Yes, please try to prepare a patch, preferably without changing the
> established backend API, maybe by using a global variable that later
> could be reused to confirm and edit other commands.

Here's my attempt.  Let me know what you think.

-- 
Sean Whitton

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-support-for-user-edits-to-VC-command-arguments.patch --]
[-- Type: text/x-patch, Size: 7522 bytes --]

From db5cbe0a693d4f231141576b07ba68e3287ca645 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Sun, 18 Sep 2022 14:47:23 -0700
Subject: [PATCH] Add support for user edits to VC command arguments

* lisp/vc/vc-dispatcher.el (vc-want-edit-command-p): New variable.
(vc-do-command): If vc-want-edit-command-p is non-nil, prompt the user
to edit the arguments to the VC command before execution.
* lisp/vc/vc-git.el (vc-git--pushpull): Drop prompting code.  Bind
vc-want-edit-command-p so that vc-do-command handles the prompting.
* lisp/vc/vc.el (vc-print-branch-log): A non-nil prefix argument now
means vc-want-edit-command-p is bound to a non-nil value (bug#57807).
---
 lisp/vc/vc-dispatcher.el | 35 +++++++++++++++++++++++++++++------
 lisp/vc/vc-git.el        | 25 +++++++------------------
 lisp/vc/vc.el            | 14 ++++++++++----
 3 files changed, 46 insertions(+), 28 deletions(-)

diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el
index 88bf6627ae..d83db53dc8 100644
--- a/lisp/vc/vc-dispatcher.el
+++ b/lisp/vc/vc-dispatcher.el
@@ -156,6 +156,9 @@ vc-parent-buffer
 (defvar vc-parent-buffer-name nil)
 (put 'vc-parent-buffer-name 'permanent-local t)
 
+(defvar vc-want-edit-command-p nil
+  "If non-nil, let user edit the VC shell command before running it.")
+
 ;; Common command execution logic
 
 (defun vc-process-filter (p s)
@@ -296,15 +299,16 @@ vc-do-command
 files or be nil (to execute commands that don't expect a file
 name or set of files).  If an optional list of FLAGS is present,
 that is inserted into the command line before the filename.
+
+If `vc-want-edit-command-p' is non-nil, prompt the user to edit
+FLAGS before execution.
+
 Return the return value of the slave command in the synchronous
 case, and the process object in the asynchronous case."
-  (when vc-tor
-    (push command flags)
-    (setq command "torsocks"))
-  ;; FIXME: file-relative-name can return a bogus result because
-  ;; it doesn't look at the actual file-system to see if symlinks
-  ;; come into play.
   (let* ((files
+          ;; FIXME: file-relative-name can return a bogus result
+          ;; because it doesn't look at the actual file-system to see
+          ;; if symlinks come into play.
 	  (mapcar (lambda (f) (file-relative-name (expand-file-name f)))
 		  (if (listp file-or-list) file-or-list (list file-or-list))))
 	 ;; Keep entire commands in *Messages* but avoid resizing the
@@ -312,6 +316,22 @@ vc-do-command
 	 ;; a such way that the important parts are at the beginning,
 	 ;; due to potential truncation of long messages.
 	 (message-truncate-lines t)
+         (files-separator-p (string= "--" (car (last flags))))
+         (flags (if vc-want-edit-command-p
+                    (append
+                     (split-string-and-unquote
+                      (read-shell-command
+                       (format "Edit VC command arguments%s: %s "
+                               (if files
+                                   " (files list to be appended)"
+                                 "")
+                               command)
+                       (combine-and-quote-strings
+                        (remq nil (if files-separator-p
+                                      (butlast flags)
+                                    flags)))))
+                     (and files-separator-p (list "--")))
+                  flags))
 	 (full-command
 	  (concat (if (string= (substring command -1) "\n")
 		      (substring command 0 -1)
@@ -321,6 +341,9 @@ vc-do-command
 	 (vc-inhibit-message
 	  (or (eq vc-command-messages 'log)
 	      (eq (selected-window) (active-minibuffer-window)))))
+    (when vc-tor
+      (push command flags)
+      (setq command "torsocks"))
     (save-current-buffer
       (unless (or (eq buffer t)
 		  (and (stringp buffer)
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index a5d12f03bc..91951d3277 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1089,35 +1089,24 @@ vc-git-error-regexp-alist
 (declare-function vc-compilation-mode "vc-dispatcher" (backend))
 (defvar compilation-directory)
 (defvar compilation-arguments)
+(defvar vc-want-edit-command-p)
 
 (defun vc-git--pushpull (command prompt extra-args)
   "Run COMMAND (a string; either push or pull) on the current Git branch.
 If PROMPT is non-nil, prompt for the Git command to run."
   (let* ((root (vc-git-root default-directory))
 	 (buffer (format "*vc-git : %s*" (expand-file-name root)))
-	 (git-program vc-git-program)
-	 args)
-    ;; If necessary, prompt for the exact command.
-    ;; TODO if pushing, prompt if no default push location - cf bzr.
-    (when prompt
-      (setq args (split-string
-		  (read-shell-command
-                   (format "Git %s command: " command)
-                   (format "%s %s" git-program command)
-                   'vc-git-history)
-		  " " t))
-      (setq git-program (car  args)
-	    command     (cadr args)
-	    args        (cddr args)))
-    (setq args (nconc args extra-args))
+         ;; TODO if pushing, prompt if no default push location - cf bzr.
+         (vc-want-edit-command-p prompt))
     (require 'vc-dispatcher)
-    (apply #'vc-do-async-command buffer root git-program command args)
+    (apply #'vc-do-async-command
+           buffer root vc-git-program command extra-args)
     (with-current-buffer buffer
       (vc-run-delayed
         (vc-compilation-mode 'git)
         (setq-local compile-command
-                    (concat git-program " " command " "
-                            (mapconcat #'identity args " ")))
+                    (concat vc-git-program " " command " "
+                            (mapconcat #'identity extra-args " ")))
         (setq-local compilation-directory root)
         ;; Either set `compilation-buffer-name-function' locally to nil
         ;; or use `compilation-arguments' to set `name-function'.
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index c67dad69fc..027dfc72ea 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1046,6 +1046,7 @@ log-view-vc-backend
 (defvar log-edit-vc-backend)
 (defvar diff-vc-backend)
 (defvar diff-vc-revisions)
+(defvar vc-want-edit-command-p)
 
 (defun vc-deduce-backend ()
   (cond ((derived-mode-p 'vc-dir-mode)   vc-dir-backend)
@@ -2744,17 +2745,22 @@ vc-print-root-log
     (setq vc-parent-buffer-name nil)))
 
 ;;;###autoload
-(defun vc-print-branch-log (branch)
-  "Show the change log for BRANCH root in a window."
+(defun vc-print-branch-log (branch &optional arg)
+  "Show the change log for BRANCH root in a window.
+Optional prefix ARG non-nil requests an opportunity for the user
+to edit the VC shell command that will be run to generate the
+log."
   (interactive
    (let* ((backend (vc-responsible-backend default-directory))
           (rootdir (vc-call-backend backend 'root default-directory)))
      (list
-      (vc-read-revision "Branch to log: " (list rootdir) backend))))
+      (vc-read-revision "Branch to log: " (list rootdir) backend)
+      current-prefix-arg)))
   (when (equal branch "")
     (error "No branch specified"))
   (let* ((backend (vc-responsible-backend default-directory))
-         (rootdir (vc-call-backend backend 'root default-directory)))
+         (rootdir (vc-call-backend backend 'root default-directory))
+         (vc-want-edit-command-p arg))
     (vc-print-log-internal backend
                            (list rootdir) branch t
                            (when (> vc-log-show-limit 0) vc-log-show-limit))))
-- 
2.30.2

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Juri Linkov]

>> Yes, please try to prepare a patch, preferably without changing the
>> established backend API, maybe by using a global variable that later
>> could be reused to confirm and edit other commands.
>
> Here's my attempt.  Let me know what you think.

Thanks, this would be one of the most useful vc features.
I tried it out, and it works nicely.  Only minor details:

Previously 'git-pull' asked for a full command including command name "git".
I know there is no need to edit the command name "git", but splitting
the command between the prompt and the minibuffer looks a little odd.

Also the previous version of 'vc-git--pushpull' sets 'compile-command'
to the edited command.  It seems this is implemented to be able to
repeat the pull command with 'g' that is bound to 'recompile'.
I don't have a good idea how to fix this.  Maybe by setting
another global variable, e.g. 'vc-last-command' after editing
the command in 'vc-do-command'.

And also 'vc-do-async-command' reports the initial unedited command
with its "Running" message string.

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

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

Hello,

On Mon 19 Sep 2022 at 09:42AM +03, Juri Linkov wrote:

> Thanks, this would be one of the most useful vc features.
> I tried it out, and it works nicely.  Only minor details:
>
> Previously 'git-pull' asked for a full command including command name "git".
> I know there is no need to edit the command name "git", but splitting
> the command between the prompt and the minibuffer looks a little odd.
>
> Also the previous version of 'vc-git--pushpull' sets 'compile-command'
> to the edited command.  It seems this is implemented to be able to
> repeat the pull command with 'g' that is bound to 'recompile'.
> I don't have a good idea how to fix this.  Maybe by setting
> another global variable, e.g. 'vc-last-command' after editing
> the command in 'vc-do-command'.
>
> And also 'vc-do-async-command' reports the initial unedited command
> with its "Running" message string.

I've done something about all these in the attached.  What do you think?

-- 
Sean Whitton

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-Add-support-for-user-edits-to-VC-command-argument.patch --]
[-- Type: text/x-patch, Size: 9525 bytes --]

From 181241f981dce0610807f42e50a89713fafb9af0 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Sun, 18 Sep 2022 14:47:23 -0700
Subject: [PATCH v2] Add support for user edits to VC command arguments

* lisp/vc/vc-dispatcher.el (vc-pre-command-functions): New hook.
(vc-want-edit-command-p): New variable.
(vc-do-command): If vc-want-edit-command-p is non-nil, prompt the user
to edit the VC command and arguments command before execution.  Run
the new hook.
(vc-do-async-command): Use the new hook to insert into BUFFER the
command that's next to be run.
* lisp/vc/vc-git.el (vc-git--pushpull): Drop prompting code.  Bind
vc-want-edit-command-p so that vc-do-command handles the prompting.
Use the new hook to update compile-command with the edited command.
* lisp/vc/vc.el (vc-print-branch-log): A non-nil prefix argument now
means vc-want-edit-command-p is bound to a non-nil value (bug#57807).
---
 lisp/vc/vc-dispatcher.el | 55 ++++++++++++++++++++++++++++++++--------
 lisp/vc/vc-git.el        | 31 ++++++++++------------
 lisp/vc/vc.el            | 18 ++++++++++---
 3 files changed, 71 insertions(+), 33 deletions(-)

diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el
index 88bf6627ae..459c2ae103 100644
--- a/lisp/vc/vc-dispatcher.el
+++ b/lisp/vc/vc-dispatcher.el
@@ -156,6 +156,9 @@ vc-parent-buffer
 (defvar vc-parent-buffer-name nil)
 (put 'vc-parent-buffer-name 'permanent-local t)
 
+(defvar vc-want-edit-command-p nil
+  "If non-nil, let user edit the VC shell command before running it.")
+
 ;; Common command execution logic
 
 (defun vc-process-filter (p s)
@@ -262,6 +265,12 @@ vc-run-delayed
   (declare (indent 0) (debug (def-body)))
   `(vc-exec-after (lambda () ,@body)))
 
+(defvar vc-pre-command-functions nil
+  "Hook run at the beginning of `vc-do-command'.
+Each function is called inside the buffer in which the command
+will be run and is passed 3 arguments: the COMMAND, the FILES and
+the FLAGS.")
+
 (defvar vc-post-command-functions nil
   "Hook run at the end of `vc-do-command'.
 Each function is called inside the buffer in which the command was run
@@ -296,8 +305,27 @@ vc-do-command
 files or be nil (to execute commands that don't expect a file
 name or set of files).  If an optional list of FLAGS is present,
 that is inserted into the command line before the filename.
+
+If `vc-want-edit-command-p' is non-nil, prompt the user to edit
+COMMAND and FLAGS before execution.
+
 Return the return value of the slave command in the synchronous
 case, and the process object in the asynchronous case."
+  (when vc-want-edit-command-p
+    (let* ((files-separator-p (string= "--" (car (last flags))))
+           (edited (split-string-and-unquote
+                    (read-shell-command
+                     (format "Edit VC command & arguments%s: "
+                             (if file-or-list
+                                 " (files list to be appended)"
+                               ""))
+                     (combine-and-quote-strings
+                      (cons command (remq nil (if files-separator-p
+                                                  (butlast flags)
+                                                flags))))))))
+      (setq command (car edited)
+            flags (nconc (cdr edited)
+                         (and files-separator-p '("--"))))))
   (when vc-tor
     (push command flags)
     (setq command "torsocks"))
@@ -327,6 +355,8 @@ vc-do-command
 		       (string= (buffer-name) buffer))
 		  (eq buffer (current-buffer)))
 	(vc-setup-buffer buffer))
+      (run-hook-with-args 'vc-pre-command-functions
+		          command file-or-list flags)
       ;; If there's some previous async process still running, just kill it.
       (let ((squeezed (remq nil flags))
 	    (inhibit-read-only t)
@@ -386,22 +416,25 @@ vc-do-async-command
 of a buffer, which is created.
 ROOT should be the directory in which the command should be run.
 Display the buffer in some window, but don't select it."
-  (let* ((dir default-directory)
-	 (inhibit-read-only t)
-	 window new-window-start)
+  (letrec ((dir default-directory)
+	   (inhibit-read-only t)
+           (fun (lambda (command _ args)
+                  (remove-hook 'vc-pre-command-functions fun)
+                  (goto-char (point-max))
+                  (unless (eq (point) (point-min))
+	            (insert "\f\n"))
+                  (setq new-window-start (point))
+                  (insert "Running \"" command)
+                  (dolist (arg args)
+	            (insert " " arg))
+                  (insert "\"...\n")))
+	   (window nil) (new-window-start nil))
     (setq buffer (get-buffer-create buffer))
     (if (get-buffer-process buffer)
 	(error "Another VC action on %s is running" root))
     (with-current-buffer buffer
       (setq default-directory root)
-      (goto-char (point-max))
-      (unless (eq (point) (point-min))
-	(insert "\f\n"))
-      (setq new-window-start (point))
-      (insert "Running \"" command)
-      (dolist (arg args)
-	(insert " " arg))
-      (insert "\"...\n")
+      (add-hook 'vc-pre-command-functions fun)
       ;; Run in the original working directory.
       (let ((default-directory dir))
 	(apply #'vc-do-command t 'async command nil args)))
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index a5d12f03bc..2228cf8665 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1089,35 +1089,30 @@ vc-git-error-regexp-alist
 (declare-function vc-compilation-mode "vc-dispatcher" (backend))
 (defvar compilation-directory)
 (defvar compilation-arguments)
+(defvar vc-want-edit-command-p)
 
 (defun vc-git--pushpull (command prompt extra-args)
   "Run COMMAND (a string; either push or pull) on the current Git branch.
 If PROMPT is non-nil, prompt for the Git command to run."
   (let* ((root (vc-git-root default-directory))
 	 (buffer (format "*vc-git : %s*" (expand-file-name root)))
-	 (git-program vc-git-program)
-	 args)
-    ;; If necessary, prompt for the exact command.
-    ;; TODO if pushing, prompt if no default push location - cf bzr.
-    (when prompt
-      (setq args (split-string
-		  (read-shell-command
-                   (format "Git %s command: " command)
-                   (format "%s %s" git-program command)
-                   'vc-git-history)
-		  " " t))
-      (setq git-program (car  args)
-	    command     (cadr args)
-	    args        (cddr args)))
-    (setq args (nconc args extra-args))
+         ;; TODO if pushing, prompt if no default push location - cf bzr.
+         (vc-want-edit-command-p prompt))
     (require 'vc-dispatcher)
-    (apply #'vc-do-async-command buffer root git-program command args)
+    (when vc-want-edit-command-p
+      (with-current-buffer (get-buffer-create buffer)
+        (add-hook 'vc-pre-command-functions
+                  (pcase-lambda (_ _ `(,new-command . ,new-args))
+                    (setq command new-command extra-args new-args))
+                  nil t)))
+    (apply #'vc-do-async-command
+           buffer root vc-git-program command extra-args)
     (with-current-buffer buffer
       (vc-run-delayed
         (vc-compilation-mode 'git)
         (setq-local compile-command
-                    (concat git-program " " command " "
-                            (mapconcat #'identity args " ")))
+                    (concat vc-git-program " " command " "
+                            (mapconcat #'identity extra-args " ")))
         (setq-local compilation-directory root)
         ;; Either set `compilation-buffer-name-function' locally to nil
         ;; or use `compilation-arguments' to set `name-function'.
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index c67dad69fc..76e577d682 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1046,6 +1046,7 @@ log-view-vc-backend
 (defvar log-edit-vc-backend)
 (defvar diff-vc-backend)
 (defvar diff-vc-revisions)
+(defvar vc-want-edit-command-p)
 
 (defun vc-deduce-backend ()
   (cond ((derived-mode-p 'vc-dir-mode)   vc-dir-backend)
@@ -2744,17 +2745,26 @@ vc-print-root-log
     (setq vc-parent-buffer-name nil)))
 
 ;;;###autoload
-(defun vc-print-branch-log (branch)
-  "Show the change log for BRANCH root in a window."
+(defun vc-print-branch-log (branch &optional arg)
+  "Show the change log for BRANCH root in a window.
+Optional prefix ARG non-nil requests an opportunity for the user
+to edit the VC shell command that will be run to generate the
+log."
+  ;; The original motivation for ARG was to make it possible to
+  ;; produce a log of more than one Git branch without modifying the
+  ;; print-log VC API.  The user can append the other branches to the
+  ;; command line arguments to 'git log'.  See bug#57807.
   (interactive
    (let* ((backend (vc-responsible-backend default-directory))
           (rootdir (vc-call-backend backend 'root default-directory)))
      (list
-      (vc-read-revision "Branch to log: " (list rootdir) backend))))
+      (vc-read-revision "Branch to log: " (list rootdir) backend)
+      current-prefix-arg)))
   (when (equal branch "")
     (error "No branch specified"))
   (let* ((backend (vc-responsible-backend default-directory))
-         (rootdir (vc-call-backend backend 'root default-directory)))
+         (rootdir (vc-call-backend backend 'root default-directory))
+         (vc-want-edit-command-p arg))
     (vc-print-log-internal backend
                            (list rootdir) branch t
                            (when (> vc-log-show-limit 0) vc-log-show-limit))))
-- 
2.30.2

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Juri Linkov]

> I've done something about all these in the attached.  What do you think?

Adding vc-pre-command-functions as a counterpart of the existing
vc-post-command-functions is a nice solution.  I tested everything,
and there are no more problems.  Thanks for implementing this feature.

After you will push it, it would be good also to try to generalize it
to other vc commands.  The problem is that some commands already use C-u
for other purposes, such as 'C-u C-x v L' asks for a limit.  What would be
a possible way to set vc-want-edit-command-p without using a prefix arg?
Maybe to add a new command that will only set vc-want-edit-command-p
for the next vc command, and bind it to e.g. 'C-x v c'.  Then
'C-x v c C-x v L' will allow editing the root log command.

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

On Wed 21 Sep 2022 at 09:52PM +03, Juri Linkov wrote:

> Adding vc-pre-command-functions as a counterpart of the existing
> vc-post-command-functions is a nice solution.  I tested everything,
> and there are no more problems.  Thanks for implementing this feature.

Thank you for testing it.

> After you will push it, it would be good also to try to generalize it
> to other vc commands.  The problem is that some commands already use
> C-u for other purposes, such as 'C-u C-x v L' asks for a limit.  What
> would be a possible way to set vc-want-edit-command-p without using a
> prefix arg?  Maybe to add a new command that will only set
> vc-want-edit-command-p for the next vc command, and bind it to
> e.g. 'C-x v c'.  Then 'C-x v c C-x v L' will allow editing the root
> log command.

Right.  It would be good to find a way to make all this consistent.
It's unfortunate that C-x v l and C-x v P were different to begin with.

Regarding your idea, how about 'C-x V V'?  It's easier to type, won't
clash with another command we might want to put on 'c', and feels a bit
like C-x 4 4, C-x 5 5 etc.  Though, to be honest, I'd prefer it if we
could come up with something involving multiple 'C-u'.

-- 
Sean Whitton

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Juri Linkov]

>> After you will push it, it would be good also to try to generalize it
>> to other vc commands.  The problem is that some commands already use
>> C-u for other purposes, such as 'C-u C-x v L' asks for a limit.  What
>> would be a possible way to set vc-want-edit-command-p without using a
>> prefix arg?  Maybe to add a new command that will only set
>> vc-want-edit-command-p for the next vc command, and bind it to
>> e.g. 'C-x v c'.  Then 'C-x v c C-x v L' will allow editing the root
>> log command.
>
> Right.  It would be good to find a way to make all this consistent.
> It's unfortunate that C-x v l and C-x v P were different to begin with.
>
> Regarding your idea, how about 'C-x V V'?  It's easier to type, won't
> clash with another command we might want to put on 'c', and feels a bit
> like C-x 4 4, C-x 5 5 etc.

It's easier to type the same letter again, but holding the Shift key
for V V after typing Control-X is more hard to type in a key sequence
'C-x V V C-x v L'.  It's a pity that 'C-x v v' is not available.

> Though, to be honest, I'd prefer it if we could come up with something
> involving multiple 'C-u'.

Another variant is to bind 'C-x v c' to a command that uses
'read-key-sequence' to read a key sequence from the vc keymap,
then call a command with call-interactively while let-binding
vc-want-edit-command-p, like project--switch-project-command does.
Then e.g. 'C-x v c L' will have mnemonic "vc command for root log".

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

Hello,

On Thu 22 Sep 2022 at 09:39AM +03, Juri Linkov wrote:

> Another variant is to bind 'C-x v c' to a command that uses
> 'read-key-sequence' to read a key sequence from the vc keymap,
> then call a command with call-interactively while let-binding
> vc-want-edit-command-p, like project--switch-project-command does.
> Then e.g. 'C-x v c L' will have mnemonic "vc command for root log".

That seems alright.  Maybe we could use some symbol, e.g. '$' or '#',
instead of a letter 'c', to distinguish its special status.

-- 
Sean Whitton

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Juri Linkov]

>> Another variant is to bind 'C-x v c' to a command that uses
>> 'read-key-sequence' to read a key sequence from the vc keymap,
>> then call a command with call-interactively while let-binding
>> vc-want-edit-command-p, like project--switch-project-command does.
>> Then e.g. 'C-x v c L' will have mnemonic "vc command for root log".
>
> That seems alright.  Maybe we could use some symbol, e.g. '$' or '#',
> instead of a letter 'c', to distinguish its special status.

I see that 'c' might clash with a possible future command.
For example, git has a lot of commands whose names start
with the letter 'c': checkout, cherry-pick, clone, commit, ...
So the probability of the need to use 'c' is high.

OTOH, what key would keep the mnemonic of a prefix arg?
Maybe 'C-x v C-u'?

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

Hello,

On Thu 22 Sep 2022 at 09:44PM +03, Juri Linkov wrote:

>>> Another variant is to bind 'C-x v c' to a command that uses
>>> 'read-key-sequence' to read a key sequence from the vc keymap,
>>> then call a command with call-interactively while let-binding
>>> vc-want-edit-command-p, like project--switch-project-command does.
>>> Then e.g. 'C-x v c L' will have mnemonic "vc command for root log".
>>
>> That seems alright.  Maybe we could use some symbol, e.g. '$' or '#',
>> instead of a letter 'c', to distinguish its special status.
>
> I see that 'c' might clash with a possible future command.
> For example, git has a lot of commands whose names start
> with the letter 'c': checkout, cherry-pick, clone, commit, ...
> So the probability of the need to use 'c' is high.
>
> OTOH, what key would keep the mnemonic of a prefix arg?
> Maybe 'C-x v C-u'?

Might be confusing if you need to do C-u C-x v C-u C-x v b l ...

'$' and '!' have shell command mnemonics.

-- 
Sean Whitton

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Juri Linkov]

>>> That seems alright.  Maybe we could use some symbol, e.g. '$' or '#',
>>> instead of a letter 'c', to distinguish its special status.
>>
>> I see that 'c' might clash with a possible future command.
>> For example, git has a lot of commands whose names start
>> with the letter 'c': checkout, cherry-pick, clone, commit, ...
>> So the probability of the need to use 'c' is high.
>>
>> OTOH, what key would keep the mnemonic of a prefix arg?
>> Maybe 'C-x v C-u'?
>
> Might be confusing if you need to do C-u C-x v C-u C-x v b l ...
>
> '$' and '!' have shell command mnemonics.

I didn't recognize '$' and '#' as shell-related symbols here
because in Emacs they have another semantics: e.g.
'C-x #' is 'server-edit', 'M-$' is ispell-checking, etc.

But 'M-!' has a clear connection with editing a shell command.  So
'C-x v !' looks like the best candidate among all considered so far.

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

Hello,

On Fri 23 Sep 2022 at 09:42AM +03, Juri Linkov wrote:

> I didn't recognize '$' and '#' as shell-related symbols here
> because in Emacs they have another semantics: e.g.
> 'C-x #' is 'server-edit', 'M-$' is ispell-checking, etc.
>
> But 'M-!' has a clear connection with editing a shell command.  So
> 'C-x v !' looks like the best candidate among all considered so far.

I'll look into implementing the prefix command, then, though I will need
to complete the refactoring in response to Stefan's comments first.

-- 
Sean Whitton

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

Hello,

On Fri 23 Sep 2022 at 09:42AM +03, Juri Linkov wrote:

>>>> That seems alright.  Maybe we could use some symbol, e.g. '$' or '#',
>>>> instead of a letter 'c', to distinguish its special status.
>>>
>>> I see that 'c' might clash with a possible future command.
>>> For example, git has a lot of commands whose names start
>>> with the letter 'c': checkout, cherry-pick, clone, commit, ...
>>> So the probability of the need to use 'c' is high.
>>>
>>> OTOH, what key would keep the mnemonic of a prefix arg?
>>> Maybe 'C-x v C-u'?
>>
>> Might be confusing if you need to do C-u C-x v C-u C-x v b l ...
>>
>> '$' and '!' have shell command mnemonics.
>
> I didn't recognize '$' and '#' as shell-related symbols here
> because in Emacs they have another semantics: e.g.
> 'C-x #' is 'server-edit', 'M-$' is ispell-checking, etc.
>
> But 'M-!' has a clear connection with editing a shell command.  So
> 'C-x v !' looks like the best candidate among all considered so far.

I'm running into the problem that you pointed out in another thread.
How do we apply vc-filter-command-function to the final VC command
that's to be run without also applying it to intermediate VC commands,
e.g. in vc-read-revision?

-- 
Sean Whitton

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Juri Linkov]

>> But 'M-!' has a clear connection with editing a shell command.  So
>> 'C-x v !' looks like the best candidate among all considered so far.
>
> I'm running into the problem that you pointed out in another thread.
> How do we apply vc-filter-command-function to the final VC command
> that's to be run without also applying it to intermediate VC commands,
> e.g. in vc-read-revision?

Maybe there is no problem to apply the editing filter to all commands,
I don't know.  First need to try to see how it really works.

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

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

Hello,

On Sat 24 Sep 2022 at 10:57PM +03, Juri Linkov wrote:

>>> But 'M-!' has a clear connection with editing a shell command.  So
>>> 'C-x v !' looks like the best candidate among all considered so far.
>>
>> I'm running into the problem that you pointed out in another thread.
>> How do we apply vc-filter-command-function to the final VC command
>> that's to be run without also applying it to intermediate VC commands,
>> e.g. in vc-read-revision?
>
> Maybe there is no problem to apply the editing filter to all commands,
> I don't know.  First need to try to see how it really works.

Here's the WIP patch, hopefully it works well enough for testing.

I think it's considerably worse than C-u C-x v b l tbh.

-- 
Sean Whitton

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-WIP-Add-vc-edit-next-command.patch --]
[-- Type: text/x-patch, Size: 2200 bytes --]

From 8521996056c980d096f1ec935c34345f265d3665 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Sat, 24 Sep 2022 16:17:56 -0700
Subject: [PATCH] WIP Add vc-edit-next-command.

---
 lisp/vc/vc-hooks.el |  3 ++-
 lisp/vc/vc.el       | 25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index 7f0d9e4d86..6ad26cfe67 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -882,7 +882,8 @@ vc-prefix-map
   "="   #'vc-diff
   "D"   #'vc-root-diff
   "~"   #'vc-revision-other-window
-  "x"   #'vc-delete-file)
+  "x"   #'vc-delete-file
+  "!"   #'vc-edit-next-command)
 (fset 'vc-prefix-map vc-prefix-map)
 (define-key ctl-x-map "v" 'vc-prefix-map)
 
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 4950a1a32d..595288d30f 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -3244,6 +3244,31 @@ vc-update-change-log
   (vc-call-backend (vc-responsible-backend default-directory)
                    'update-changelog args))
 
+(defvar vc-filter-command-function)
+
+;;;###autoload
+(defun vc-edit-next-command ()
+  "Request editing the next VC shell command before execution.
+This is a prefix command.  It affects only a VC command executed
+immediately after this one."
+  (interactive)
+  (letrec ((command this-command)
+           (keys (key-description (this-command-keys)))
+           (old vc-filter-command-function)
+           (echofun (lambda () keys))
+           (postfun
+            (lambda ()
+              (unless (eq this-command command)
+                (remove-hook 'post-command-hook postfun)
+                (remove-hook 'prefix-command-echo-keystrokes-functions
+                             echofun)
+                (setq vc-filter-command-function old)))))
+    (add-hook 'post-command-hook postfun)
+    (add-hook 'prefix-command-echo-keystrokes-functions echofun)
+    (setq vc-filter-command-function
+          (lambda (&rest args)
+            (apply #'vc-user-edit-command (apply old args))))))
+
 (defun vc-default-responsible-p (_backend _file)
   "Indicate whether BACKEND is responsible for FILE.
 The default is to return nil always."
-- 
2.30.2

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Juri Linkov]

>>> I'm running into the problem that you pointed out in another thread.
>>> How do we apply vc-filter-command-function to the final VC command
>>> that's to be run without also applying it to intermediate VC commands,
>>> e.g. in vc-read-revision?
>>
>> Maybe there is no problem to apply the editing filter to all commands,
>> I don't know.  First need to try to see how it really works.
>
> Here's the WIP patch, hopefully it works well enough for testing.

Thanks, I'm going to try it out.

> I think it's considerably worse than C-u C-x v b l tbh.

Undoubtedly, it's worse.  But it solves the problem that
all vc commands can't use C-u.

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Sean Whitton]

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

Hello,

On Sun 25 Sep 2022 at 10:29AM +03, Juri Linkov wrote:

>>>> I'm running into the problem that you pointed out in another thread.
>>>> How do we apply vc-filter-command-function to the final VC command
>>>> that's to be run without also applying it to intermediate VC commands,
>>>> e.g. in vc-read-revision?
>>>
>>> Maybe there is no problem to apply the editing filter to all commands,
>>> I don't know.  First need to try to see how it really works.
>>
>> Here's the WIP patch, hopefully it works well enough for testing.
>
> Thanks, I'm going to try it out.

It looks like I need to call (minibuffer-depth) like
display-buffer-override-next-command does.  Updated patch attached.

>> I think it's considerably worse than C-u C-x v b l tbh.
>
> Undoubtedly, it's worse.  But it solves the problem that
> all vc commands can't use C-u.

Just to be clear, the thing that I think is much worse is being prompted
for multiple intermediate commands, not the longer key sequence.

I realised that we could add an override inside vc-read-revision, and
potentially other places, which makes it considerably less annoying.
Done in the attached.

-- 
Sean Whitton

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-WIP-Add-vc-edit-next-command.patch --]
[-- Type: text/x-patch, Size: 3049 bytes --]

From 7115ae59cf5f263dbc1b1a208fce974d790d8cae Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Sat, 24 Sep 2022 16:17:56 -0700
Subject: [PATCH v2] WIP Add vc-edit-next-command.

---
 lisp/vc/vc-hooks.el |  3 ++-
 lisp/vc/vc.el       | 34 ++++++++++++++++++++++++++++++++--
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index 7f0d9e4d86..6ad26cfe67 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -882,7 +882,8 @@ vc-prefix-map
   "="   #'vc-diff
   "D"   #'vc-root-diff
   "~"   #'vc-revision-other-window
-  "x"   #'vc-delete-file)
+  "x"   #'vc-delete-file
+  "!"   #'vc-edit-next-command)
 (fset 'vc-prefix-map vc-prefix-map)
 (define-key ctl-x-map "v" 'vc-prefix-map)
 
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 4950a1a32d..76554d2340 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1918,8 +1918,11 @@ vc-read-revision
       (setq files (cadr vc-fileset))
       (setq backend (car vc-fileset))))
    ((null backend) (setq backend (vc-backend (car files)))))
-  (let ((completion-table
-         (vc-call-backend backend 'revision-completion-table files)))
+  ;; Override any `vc-filter-command-function' value, as user probably
+  ;; doesn't want to edit the command to get the completions.
+  (let* ((vc-filter-command-function #'list)
+         (completion-table
+          (vc-call-backend backend 'revision-completion-table files)))
     (if completion-table
         (completing-read prompt completion-table
                          nil nil initial-input 'vc-revision-history default)
@@ -3244,6 +3247,33 @@ vc-update-change-log
   (vc-call-backend (vc-responsible-backend default-directory)
                    'update-changelog args))
 
+(defvar vc-filter-command-function)
+
+;;;###autoload
+(defun vc-edit-next-command ()
+  "Request editing the next VC shell command before execution.
+This is a prefix command.  It affects only a VC command executed
+immediately after this one."
+  (interactive)
+  (letrec ((minibuffer-depth (minibuffer-depth))
+           (command this-command)
+           (keys (key-description (this-command-keys)))
+           (old vc-filter-command-function)
+           (echofun (lambda () keys))
+           (postfun
+            (lambda ()
+              (unless (or (eq this-command command)
+                          (> (minibuffer-depth) minibuffer-depth))
+                (remove-hook 'post-command-hook postfun)
+                (remove-hook 'prefix-command-echo-keystrokes-functions
+                             echofun)
+                (setq vc-filter-command-function old)))))
+    (add-hook 'post-command-hook postfun)
+    (add-hook 'prefix-command-echo-keystrokes-functions echofun)
+    (setq vc-filter-command-function
+          (lambda (&rest args)
+            (apply #'vc-user-edit-command (apply old args))))))
+
 (defun vc-default-responsible-p (_backend _file)
   "Indicate whether BACKEND is responsible for FILE.
 The default is to return nil always."
-- 
2.30.2

bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches

[Juri Linkov]

>> Thanks, I'm going to try it out.
>
> It looks like I need to call (minibuffer-depth) like
> display-buffer-override-next-command does.  Updated patch attached.

I tested your latest patch with many vc commands, and everything
works nicely, even M-x for vc commands without a keybinding
and with a prefix arg is supported like in

  C-x v ! C-u M-x vc-log-search RET

(C-u is not needed for vc-log-search anymore,
it's just for testing a prefix arg here).

> I realised that we could add an override inside vc-read-revision, and
> potentially other places, which makes it considerably less annoying.
> Done in the attached.

Maybe it's fine to disable editing for vc-read-revision,
but I'm not sure about other commands, such as e.g.
vc-git-log-incoming that runs two git commands:

  vc-git-command fetch
  vc-git-command log

I see no problem to type an extra RET even for a secondary git command.
But it helps in case when someone really wants to edit it.

bug#57807: vc-edit-next-command (was bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches)

[Sean Whitton]

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

Hello,

On Tue 27 Sep 2022 at 09:59PM +03, Juri Linkov wrote:

>>> Thanks, I'm going to try it out.
>>
>> It looks like I need to call (minibuffer-depth) like
>> display-buffer-override-next-command does.  Updated patch attached.
>
> I tested your latest patch with many vc commands, and everything
> works nicely, even M-x for vc commands without a keybinding
> and with a prefix arg is supported like in
>
>   C-x v ! C-u M-x vc-log-search RET
>
> (C-u is not needed for vc-log-search anymore,
> it's just for testing a prefix arg here).

Thank you for the through testing.

Here is a more formal patch, including manual updates.

>> I realised that we could add an override inside vc-read-revision, and
>> potentially other places, which makes it considerably less annoying.
>> Done in the attached.
>
> Maybe it's fine to disable editing for vc-read-revision,
> but I'm not sure about other commands, such as e.g.
> vc-git-log-incoming that runs two git commands:
>
>   vc-git-command fetch
>   vc-git-command log
>
> I see no problem to type an extra RET even for a secondary git command.
> But it helps in case when someone really wants to edit it.

Yeah, we should be conservative.  I've done it for only vc-read-revision
for now, and perhaps I'll raise some others later.

-- 
Sean Whitton

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v3-0001-Add-vc-edit-next-command.patch --]
[-- Type: text/x-patch, Size: 7771 bytes --]

From 381c04afb4f741fbbf2f4dfcda3973c3738d0442 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Sat, 24 Sep 2022 16:17:56 -0700
Subject: [PATCH v3] Add vc-edit-next-command

* lisp/vc/vc.el (vc-read-revision): Override
vc-filter-command-function back to its default value.
(vc-print-branch-log): Remove recently-added prefix argument.
(vc-edit-next-command): New command.
* lisp/vc/vc-hooks.el (vc-prefix-map): Bind it to 'C-x v !'.
* etc/NEWS:
* doc/emacs/vc1-xtra.texi (Editing Shell Commands): Document it.
---
 doc/emacs/vc1-xtra.texi | 26 ++++++++++++++++----
 etc/NEWS                |  7 ++++++
 lisp/vc/vc-hooks.el     |  3 ++-
 lisp/vc/vc.el           | 53 ++++++++++++++++++++++++++++-------------
 4 files changed, 67 insertions(+), 22 deletions(-)

diff --git a/doc/emacs/vc1-xtra.texi b/doc/emacs/vc1-xtra.texi
index 3ccad50715..1266873c4b 100644
--- a/doc/emacs/vc1-xtra.texi
+++ b/doc/emacs/vc1-xtra.texi
@@ -11,10 +11,11 @@ Miscellaneous VC
   This section explains the less-frequently-used features of VC.
 
 @menu
-* Change Logs and VC::  Generating a change log file from log entries.
-* VC Delete/Rename::    Deleting and renaming version-controlled files.
-* Revision Tags::       Symbolic names for revisions.
-* Version Headers::     Inserting version control headers into working files.
+* Change Logs and VC::     Generating a change log file from log entries.
+* VC Delete/Rename::       Deleting and renaming version-controlled files.
+* Revision Tags::          Symbolic names for revisions.
+* Version Headers::        Inserting version control headers into working files.
+* Editing Shell Commands:: Editing the VC shell commands that Emacs will run.
 @end menu
 
 @node Change Logs and VC
@@ -263,6 +264,23 @@ Version Headers
 part of the version header.  A @samp{%s} in @var{format} is replaced
 with the file's version control type.
 
+@node Editing Shell Commands
+@subsubsection Editing Shell Commands
+
+@findex vc-edit-next-command
+@kindex C-x v !
+You can use the @kbd{C-x v !} (@code{vc-edit-next-command}) prefix
+command to request an opportunity to edit the VC shell commands that
+Emacs will run.  This is primarily intended to make it possible to
+access version control system-specific functionality without
+complexifying either the VC command set or the backend API.
+
+For example, Git can produce logs of more than one branch, but
+@kbd{C-x v b l} (@code{vc-print-branch-log}) prompts for the name of
+just one branch.  To obtain a log of more than one branch, you can
+type @kbd{C-x v ! C-x v b l} and then append the names of additional
+branches to the end of the 'git log' command that VC prepares.
+
 @node Customizing VC
 @subsection Customizing VC
 
diff --git a/etc/NEWS b/etc/NEWS
index 5b15e9dcb6..d1d3fc6f89 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1722,6 +1722,13 @@ commands.
 This command marks files based on a regexp.  If given a prefix
 argument, unmark instead.
 
++++
+*** New command 'C-x v !' ('vc-edit-next-command')
+This prefix command requests editing of the next VC shell command
+before execution.  For example, in a Git repository, you can produce a
+log of more than one branch by typing 'C-x v ! C-x v b l' and then
+appending additional branch names to the 'git log' command.
+
 ---
 *** 'C-x v v' in a diffs buffer allows to commit only some of the changes.
 This command is intended to allow you to commit only some of the
diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index 7f0d9e4d86..6ad26cfe67 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -882,7 +882,8 @@ vc-prefix-map
   "="   #'vc-diff
   "D"   #'vc-root-diff
   "~"   #'vc-revision-other-window
-  "x"   #'vc-delete-file)
+  "x"   #'vc-delete-file
+  "!"   #'vc-edit-next-command)
 (fset 'vc-prefix-map vc-prefix-map)
 (define-key ctl-x-map "v" 'vc-prefix-map)
 
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 4950a1a32d..e18fd6f5c8 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1918,8 +1918,11 @@ vc-read-revision
       (setq files (cadr vc-fileset))
       (setq backend (car vc-fileset))))
    ((null backend) (setq backend (vc-backend (car files)))))
-  (let ((completion-table
-         (vc-call-backend backend 'revision-completion-table files)))
+  ;; Override any `vc-filter-command-function' value, as user probably
+  ;; doesn't want to edit the command to get the completions.
+  (let* ((vc-filter-command-function #'list)
+         (completion-table
+          (vc-call-backend backend 'revision-completion-table files)))
     (if completion-table
         (completing-read prompt completion-table
                          nil nil initial-input 'vc-revision-history default)
@@ -2745,28 +2748,17 @@ vc-print-root-log
     (setq vc-parent-buffer-name nil)))
 
 ;;;###autoload
-(defun vc-print-branch-log (branch &optional arg)
-  "Show the change log for BRANCH root in a window.
-Optional prefix ARG non-nil requests an opportunity for the user
-to edit the VC shell command that will be run to generate the
-log."
-  ;; The original motivation for ARG was to make it possible to
-  ;; produce a log of more than one Git branch without modifying the
-  ;; print-log VC API.  The user can append the other branches to the
-  ;; command line arguments to 'git log'.  See bug#57807.
+(defun vc-print-branch-log (branch)
+  "Show the change log for BRANCH root in a window."
   (interactive
    (let* ((backend (vc-responsible-backend default-directory))
           (rootdir (vc-call-backend backend 'root default-directory)))
      (list
-      (vc-read-revision "Branch to log: " (list rootdir) backend)
-      current-prefix-arg)))
+      (vc-read-revision "Branch to log: " (list rootdir) backend))))
   (when (equal branch "")
     (error "No branch specified"))
   (let* ((backend (vc-responsible-backend default-directory))
-         (rootdir (vc-call-backend backend 'root default-directory))
-         (vc-filter-command-function (if arg
-                                         #'vc-user-edit-command
-                                       vc-filter-command-function)))
+         (rootdir (vc-call-backend backend 'root default-directory)))
     (vc-print-log-internal backend
                            (list rootdir) branch t
                            (when (> vc-log-show-limit 0) vc-log-show-limit))))
@@ -3244,6 +3236,33 @@ vc-update-change-log
   (vc-call-backend (vc-responsible-backend default-directory)
                    'update-changelog args))
 
+(defvar vc-filter-command-function)
+
+;;;###autoload
+(defun vc-edit-next-command ()
+  "Request editing the next VC shell command before execution.
+This is a prefix command.  It affects only a VC command executed
+immediately after this one."
+  (interactive)
+  (letrec ((minibuffer-depth (minibuffer-depth))
+           (command this-command)
+           (keys (key-description (this-command-keys)))
+           (old vc-filter-command-function)
+           (echofun (lambda () keys))
+           (postfun
+            (lambda ()
+              (unless (or (eq this-command command)
+                          (> (minibuffer-depth) minibuffer-depth))
+                (remove-hook 'post-command-hook postfun)
+                (remove-hook 'prefix-command-echo-keystrokes-functions
+                             echofun)
+                (setq vc-filter-command-function old)))))
+    (add-hook 'post-command-hook postfun)
+    (add-hook 'prefix-command-echo-keystrokes-functions echofun)
+    (setq vc-filter-command-function
+          (lambda (&rest args)
+            (apply #'vc-user-edit-command (apply old args))))))
+
 (defun vc-default-responsible-p (_backend _file)
   "Indicate whether BACKEND is responsible for FILE.
 The default is to return nil always."
-- 
2.30.2

bug#57807: vc-edit-next-command (was bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches)

[Juri Linkov]

>  @menu
> +* Change Logs and VC::     Generating a change log file from log entries.
> +* VC Delete/Rename::       Deleting and renaming version-controlled files.
> +* Revision Tags::          Symbolic names for revisions.
> +* Version Headers::        Inserting version control headers into working files.
> +* Editing Shell Commands:: Editing the VC shell commands that Emacs will run.
>  @end menu

The Info node name "Editing Shell Commands" might not be obvious to
everyone when displayed in the Info completion list with 'g edit TAB'.
Maybe a better unambiguous node name while keeping it short
would be "Editing VC Commands"?

bug#57807: vc-edit-next-command (was bug#57807: 29.0.50; Make vc-print-branch-log able to log multiple branches)

[Sean Whitton]

Hello,

On Wed 28 Sep 2022 at 08:53PM +03, Juri Linkov wrote:

>>  @menu
>> +* Change Logs and VC::     Generating a change log file from log entries.
>> +* VC Delete/Rename::       Deleting and renaming version-controlled files.
>> +* Revision Tags::          Symbolic names for revisions.
>> +* Version Headers::        Inserting version control headers into working files.
>> +* Editing Shell Commands:: Editing the VC shell commands that Emacs will run.
>>  @end menu
>
> The Info node name "Editing Shell Commands" might not be obvious to
> everyone when displayed in the Info completion list with 'g edit TAB'.
> Maybe a better unambiguous node name while keeping it short
> would be "Editing VC Commands"?

Thanks, yes.

-- 
Sean Whitton