GNU bug report logs - #52349
29.0.50; vc-git and diff-mode: stage hunks

Previous Next

Full log

Message #118 received at 52349 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 52349 <at> debbugs.gnu.org, Sean Whitton <spwhitton <at> spwhitton.name>
Subject: Re: bug#52349: 29.0.50; vc-git and diff-mode: stage hunks
Date: Mon, 19 Sep 2022 09:50:18 +0300

> Do you know if Git use a stable ordering for files?

It seems the ordering is stable unless it's changed explicitly
by command line options.

> If not, here's where the proposed implementation might fail:
>
> Suppose we have the staging area with contents
>
> a/bar b/bar
> +bbb
> a/foo b/foo
> +aaa
> +ccc
>
> and the diff to check in with contents
>
> a/foo b/foo
> +aaa
> a/bar b/bar
> +bbb
> +ccc
>
> ...then the check will succeed, I think.

This check is intended to detect only added/deleted files
that get into the staging area.

> Even if Git always sorts them the same, I suppose an externally-produced
> diff could trigger this scenario. It should be a pretty rare case, though,
> so it's probably fine.
>
> A tweak like the following could fix it, though: instead of replacing the
> chunks with "", maybe keep the file headers. Then the remaining contents
> for the same file in vc-git-patch-string wouldn't be able to stick to the
> previous file's chunks.

This looks too complicated.  And indeed, this is a rare case, so maybe
something like this could be added when this will became a real problem.

>>> Perhaps if we called diff-file-next in addition to (move-beginning-of-line
>>> 1)?
>> diff-file-next doesn't work: it stops in the middle of the diff file
>> header.
>> Therefore the patch navigates git diff file headers explicitly.
>>
>>> But we need to save both strings: and ensure that if there is a match
>>> for that file header, then all hunks are contained in the patch as well
>>> (and nothing extra, for that file).
>> This is implemented by the patch.  Maybe it could help you to see how it
>> works
>> by running it under debugger and stepping through it.
>>
>>> It's complex logic, so if you manage to write a test as well, that would be
>>> excellent.
>> A test could written when someone will create infrastructure for testing
>> git commands with helpers to create a git repository and checking its content.
>
> test/lisp/vc/vc-tests.el actually contains a helper like this.
>
> Every scenario starts with calling vc-test--create-repo-function, and there
> are tests for 'version-diff' at the very end of the file. It's somewhat
> convoluted, so I don't blame you for missing it.

Actually, I looked into test/lisp/vc/vc-git-tests.el that is almost empty.
I expected that since this check is git-specific, it should be in
vc-git-tests.el.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.