bug#60126: 30.0.50; vc-git-checkin: Offer to unstage conflicting changes

[Dmitry Gutov <dgutov@yandex.ru>]

On 24/12/2022 04:02, Sean Whitton wrote:
> Hello,
> 
> On Sat 24 Dec 2022 at 01:18AM +02, Dmitry Gutov wrote:
> 
>> On 23/12/2022 05:59, Sean Whitton wrote:
>>> It works, except that sometimes the let-binding of process-environment
>>> fails, such that the commands affect the normal index rather than the
>>> temporary index.  Can you see what I'm doing wrong there?
>> Could you describe be "sometimes" occurrences? Does that happen through
>> repeating a similar action several times? Or slightly different actions?
>>
>> The process-environment setup seems fine. We did corrupt it in 1-2 places in
>> the past using 'setenv', but I don't see anything like that in the current
>> codebase. And the effect would probably be different anyway.
> Thank you for looking.  Slightly embarassingly, I can't reproduce the
> problem today.  So I've gone ahead and pushed.

Thanks!

> I am pretty happy with this approach, in the end.  Compared with other
> possible uses of 'git stash', it's quite clean:
> 
> - it doesn't touch the worktree copies of the files not involved in the
>    commit
> 
> - it stashes a single diff, rather than two diffs (one for the worktree
>    and one for the index), which is less for the user to deal with if
>    manual recovery becomes required.

It does indeed feel like we ended up in a good place. The code was 
pretty complex, though, and got more so.

We don't have tests covering vc-git-checkin-patch at all. Any chance 
you'll fancy working on adding those? Even if you only cover the 
scenarios where the user doesn't get prompted at all (either there's 
nothing staged, or the staged contents match the committed patch).

Writing (and debugging) a test could also help sort out fiddly 
behaviors, like the one you may have seen yesterday.

We have a default implementation for checkin-patch, so adding generic 
test for all major backends could work (in vc-tests.el). One or two 
extra tests could be also predicated on (eq backend 'Git), so that we 
don't yet need to copy the repository setup/teardown code to vc-git.el.