Skip to content

Fix a bug in the hunk-splitting code of the built-in git add -p #2368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 24, 2019

Conversation

dscho
Copy link
Member

@dscho dscho commented Oct 24, 2019

This bug was reported over at #2364: the code mishandled comment lines in the diffs (i.e. lines starting with a backslash).

While in that part of the code already, I also added some defensive code, and some comments.

dscho added 5 commits October 24, 2019 11:55
When counting how many lines a hunk can be split into, we need to be
careful to take line comments in the diff into account, i.e. lines
starting with a backslash. Example:

	[...]
	-}
	\ No newline at end of file
	+}

Logically, these line comments belong to the line preceding them,
therefore we need to pretend that they have the same diff marker (space,
minus or plus character).

This fixes git-for-windows#2364.

Signed-off-by: Johannes Schindelin <[email protected]>
Let's be a bit more defensive in the code that finds the next line: if
we are already at the end of the buffer, we definitely do _not_ want to
return an offset that is outside the buffer!

Signed-off-by: Johannes Schindelin <[email protected]>
A recently-reported bug had its root cause in the `splittable_into`
variable overcounting the number of potential hunks (the symptom was
misleading, it said that there was an unhandled diff marker).

Let's make sure to catch these things earlier, with a less misleading
error message.

Signed-off-by: Johannes Schindelin <[email protected]>
While it is true that the diffs Git generates can only contain comment
lines at the end of a hunk, and really only when indicating that a line
does not end in a Line Feed, comment lines _are_ a valid diff construct.

Let's play it safe by handling those when splitting hunks.

Signed-off-by: Johannes Schindelin <[email protected]>
While in the vicinity of this code, let's document what the rather
convoluted conditions in the hunk-splitting loop actually _mean_: while
they look simple enough, it took a bit of brain-twisting even for the
developer who wrote this code to understand after a few months what is
going on in this part. And if even the person who wrote this has a hard
time understanding the code... then it needs commenting ;-)

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho added this to the v2.23.0(2) milestone Oct 24, 2019
@dscho dscho mentioned this pull request Oct 24, 2019
dscho added a commit to git-for-windows/build-extra that referenced this pull request Oct 24, 2019
The (still experimental) built-in `git add
-p` [no longer gets confused about incomplete
lines](git-for-windows/git#2368) (i.e. a
file's l last line that does not end in a Line Feed).

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho merged commit e10734d into git-for-windows:master Oct 24, 2019
@dscho dscho deleted the add-p-fix-split-hunks branch October 24, 2019 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant