Subject: Re: bug#57400: 29.0.50; Support sending patches from VC directly

From: Philip Kaludercic <[email-masked]>
Date: Fri, 07 Oct 2022 15:30:11 +0000

Dmitry Gutov <[email-masked]> writes:

>
> On 07.10.2022 11:03, Philip Kaludercic wrote:
>> Dmitry Gutov <[email-masked]> writes:
>> 
>>> Hi Philip,
>>>
>>> On 06.10.2022 15:38, Philip Kaludercic wrote:
>>>> I'm glad to hear that.  Here is the updated patch:
>>>
>>> This version resolves the main questions I had as well.
>>>
>>> Here's one more, though: the way I normally used 'git format-patch' is
>>> I pass just one ref, and that's usually the name of the branch to
>>> start the history at (the <since> argument from the manual). So I
>>> never had to "type revisions in the Git syntax" for this to work,
>>> something Eli was worried about.
>> It might be tricky to do this in a VC-generic way, but what I can
>> think
>> of would be if the command is invoked with a prefix argument, then
>
> prefix argument? Ok. I would imagine this to be the "default" usage
> scenario, though.

It could be the default too, I guess it depends on how well the feature
works.  I certainly know from my own works flow that I don't always want
to submit all the commits on a branch, but just one or two, so having
the option would be appreciated.

>> instead of querying for specific revisions, we query for one only, then
>> use 'next-revision' to check if it is a predecessor of the current
>> revision.  If so, all the commits in-between are used.
>
> <since> is not necessarily a predecessor: 'git format-patch master'
> works even when master has some more extra commits since the "merge
> base" commit. 'master' will point to the commit that's not present in
> the current branch.
>
> But vc-log-mergebase is fine with such situation. It calls
> (vc-call-backend backend 'mergebase rev1 rev2) to find the most recent
> common revision, and start after it.
>
>> The main issue I see here is that 'next-revision' requires a file
>> argument.  What should that be?
>
> The 'mergebase' and 'print-log' actions don't seem to require it.

I will take a look at those.

>>> Should this new command support this usage as well?
>>>
>>> The range of revisions could be fetched by passing the base revision
>>> as LIMIT to the 'print-log' action (like vc-log-mergebase does), but
>>> how the updated calling convention for vc-prepare-patch will look is
>>> not obvious to me.
>> Even if we do this, the value of the argument "files" still remains
>> an
>> open question.
>
> vc-log-mergebase passes (list rootdir) as FILES to 'print-log'.

That appears to only work if the backend defines a root directory.

>> It is for this reason that I prefer the current approach, especially
>> when combined with the ability to select commits in a log.
>
> I'm definitely not going to insist: I'm not the target audience of
> this feature.

I could imagine falling back onto the manual selection of revisions when
the root directory is not defined, but I fear that would be too
inconsistent and might lead to unintended/unexpected behaviour when
switching between VCSs.