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

[Stefan Monnier]

>> 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`.
>
> Each command that uses this would have to ensure that the prompting
> function is removed again once used, right?  How would you suggest
> handling that?  Perhaps we could have a macro doing something like what
> we see in vc-exec-after, `add-once-function' or something.

I was thinking of having the caller simply let-bind the variable, or, as
your code does, have the function set the var's value back to
`identity`.

>>> @@ -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.
>
> Possibly vc-do-async-command and vc-git--pushpull can invoke the
> prompting function themselves, thereby obtaining the new command and
> arguments, and then add-function a constant function so that
> vc-do-command gets the new values.  Is that something like what you have
> in mind?

I don't have anything in mind, I just see that you added two "hooks",
one specifically for prompting the user to modify the command and another
more open ended.  Currently neither subsumes the other, but I wonder if
we couldn't provide a single hook that could satisfy both needs.

>> 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?
>
> Previously it was inserted without going via the hook.

But why did you change the code so it goes via the hook?

>> 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.
>
> It's only the use of vc-git-program in compile-command that now works
> differently, right?  The value passed to vc-do-async-command should
> still be the same buffer-local value if there is one.

I think so, yes.


        Stefan