Re: master 101f3cf5b9: Add support for user edits to VC command arguments

[Stefan Monnier]

> @@ -296,8 +305,27 @@ FILE-OR-LIST is the name of a working file; it may be a 
> list of
>  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.

This prompting seems a bit too specific to particular callers of this
function, so I suggest you replace it with some kind of hook which takes
the command as arg and returns the command to use instead.  It should be
a `<foo>-function` with a default value of `identity`.

>  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 '("--"))))))

So this code can then be conveniently placed in a separate function that
callers can simply use in `<foo>-function` variable.

> @@ -327,6 +355,8 @@ case, and the process object in the asynchronous case."
>                      (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)

Any chance this hook can be merged with the one I propose above?
If not, please clarify (in their doc) how&why they differ.

> @@ -386,22 +416,25 @@ Send the output to BUFFER, which should be a buffer or 
> the name
>  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 "\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 "\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)))

Your commit message says:

    (vc-do-async-command): Use the new hook to insert into BUFFER the
    command that's next to be run.

but I don't understand what this changes does.
Wasn't it already inserted into BUFFER?  What's changed?

>  (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'.

IIUC the (git-program vc-git-program) binding allowed that var to have
different values in different buffers.  This change instead presumes
`vc-git-program` is the same in all buffers.  Not sure it matters, but
we've been bitten by similar things in VC.


        Stefan