bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Dmitry Gutov
Subject: bug#52349: 29.0.50; vc-git and diff-mode: stage hunks
Date: Mon, 19 Sep 2022 15:57:17 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0

On 19.09.2022 09:50, Juri Linkov wrote:
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.

It doesn't seem like it's going to differentiate between "add whole file" chunks and any other kinds of chunks.

Which is not a bad thing by itself, probably. But can increase the odds of something like the above happening.

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.

Sure, sounds good to me.

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.

Not sure how to best share the setup/teardown logic between vc-tests.el and vc-git-test.el.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]