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

From: Sean Whitton <[email-masked]>
Date: Tue, 27 Sep 2022 18:15:01 -0700

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
>From 381c04afb4f741fbbf2f4dfcda3973c3738d0442 Mon Sep 17 00:00:00 2001
From: Sean Whitton <[email-masked]>
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