Skip to content
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

Fix %(lineno) computation for hunk postimages that span only one line #1062

Merged
merged 1 commit into from
Jan 9, 2021

Conversation

krobelus
Copy link
Contributor

Usually diff hunk headers look like "@@ -a,b +c,d @@" where "a" is the
starting line number and "b" is the number of lines in the preimage of the
hunk. Same for "c" and "d" except they refer to lines in the postimage.

Git omits "b" and "d" if they are 1. Tig already treated "b" as optional but
not "d". We can simply ignore a missing number because "header->new.lines"
is already initialized to 1 in parse_chunk_header().

$ echo old line 1 >? old
$ echo new line 1 >? new
$ git diff --no-index old new | tail -n3
@@ -1 +1 @@
-old line 1
+new line 1

test_setup_work_dir()
{
git_commit --allow-empty-message
git_add README.md 'new line 1\n'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering the definition of git_add() in libgit.sh, I must admit I don't understand how the test can work... It does, though.

For clarity, what about:

diff --git a/test/diff/line-number-test b/test/diff/line-number-test
index 72dac70..c44873e 100755
--- a/test/diff/line-number-test
+++ b/test/diff/line-number-test
@@ -9,18 +9,17 @@ export LINES=3
 steps '
        :view-diff
        :move-last-line
-       :move-up
        :!echo %(lineno)
        :save-display added-line-number.screen
 '

-
 git_init "$work_dir"

 test_setup_work_dir()
 {
        git_commit --allow-empty-message
-       git_add README.md 'new line 1\n'
+       printf 'new line 1\n' > README.md
+       git add README.md
 }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this a bad abstraction; will change it (also I hadn't realized there was an extra newline).

git_add() calls file() to create the file. file() is defined in libtest.sh. That's really surprising because file usually means something else.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I missed file() [very bad idea IMHO] so git_add() looked a bit obscure but it makes sense now. Actually it was not an extra newline but git_add didn't interpret \n and so there was no new line at all. git show complaining about the lack of new line was the cause of the second line.

Usually diff hunk headers look like "@@ -a,b +c,d @@" where "a" is the
starting line number and "b" is the number of lines in the preimage of the
hunk. Same for "c" and "d" except they refer to lines in the postimage.

Git omits "b" and "d" if they are 1.  Tig already treated "b" as optional but
not "d". We can simply ignore a missing number because "header->new.lines"
is already initialized to 1 in parse_chunk_header().

	$ echo old line 1 >? old
	$ echo new line 1 >? new
	$ git diff --no-index old new | tail -n3
	@@ -1 +1 @@
	-old line 1
	+new line 1
@koutcher koutcher merged commit e4207c4 into jonas:master Jan 9, 2021
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.

2 participants