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

[Sean Whitton]

Hello,

On Sat 24 Dec 2022 at 04:50PM +02, Dmitry Gutov wrote:

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

Now that we understand clearly what we want it to do, I bet the code in
vc-checkin-git could be simplified (vc-git--stash-staged-changes is
fine).  So I'll see about doing that, with some tests, as you suggest.

Let me ask you about the parsing of the 'diff --git' lines.  I wasn't
happy with my regexp approach to extracting the filename.  I'm not sure
it can actually fail, but the current codes assumes it can, and that
adds complexity.

The --src-prefix, --dst-prefix and --no-prefix options to git-diff(1)
might be relevant, but then we couldn't use a simple string-match to
find hunks in vc-git-patch-string.

Do you have any better ideas of how to extract the filename from the git
--diff line, or perhaps a proof that my approach can't fail? :)

-- 
Sean Whitton