jisaacks / GitGutter

A Sublime Text 2/3 plugin to see git diff in gutter
MIT License
3.88k stars 224 forks source link

Bug: Lines marked as "modified" instead of "inserted" #345

Closed nwoltman closed 4 years ago

nwoltman commented 7 years ago

When new lines are inserted directly after a line that is modified, all of the inserted lines are marked as modified.

Original file contents

a

b

c

Modified file contents

a
1

b

c2
d
d
d
d
d

Current behaviour

current

Expected behaviour

expected

You can see the example from the screenshots in this demo repo.

A similar issue was opened in the Atom git-diff package which was closed because they were limited by libgit2. This package seems to use git directly so perhaps that isn't a limitation.

I noticed that #173 was closed because "GitGutter works on blocks of changes." That was 2 years ago so perhaps things are different now?

It looks like the code already uses the output of git diff --unified=0, so in the case above the output would include the line:

@@ -5 +6,6 @@ b

So from the line numbers (-5 and +6) it can be calculated that 1 line was removed and then +6,6 means that 6 lines were inserted. So 1 removed line + 6 inserted lines yields 1 modified line + 5 inserted lines , so line 6 can be marked as modified and the next 5 lines can be marked as inserted.

r-stein commented 7 years ago

First of all: This not a bug, but the expected behavior. Hence it is an enhancement.

The reasoning with the blocks is still valid and the solution is not as simple as suggested. E.g. if you have

c

then both

c2
d
d

and

d
c2
d

will result in @@ -5 +5,3 @@ b, so can not assume that the first line changed and the rest was just added, but we need to analyse the whole hunk.

nwoltman commented 7 years ago

First of all: This not a bug, but the expected behavior. Hence it is an enhancement.

I'd say it depends on your perspective. Perhaps there could be a setting so people could choose which functionality they want?

I see your point with that example though.

Running git diff --word-diff --unified=0 on

d
c2
d

yields

@@ -5 +5,3 @@ b
[-c-]{+d+}
{+c2+}
{+d+}

so at least what I was suggesting is the same as what git would come up with. (+1 for my suggestion) But then if the text is

d
c 2
d

the word-diff is

@@ -5 +5,3 @@ b
{+d+}
c {+2+}
{+d+}

which requires parsing the output lines. (-1 for my suggestion)

Would it be reasonable for performance to parse the word-diff output only for sections where the number of removed lines is not specified and the number of added lines is positive? i.e. Only parse lines if the diff sections starts with a line of the form @@ -x +y,n @@ where n is a positive integer.

r-stein commented 7 years ago

Using the word-diff option sounds reasonable, but if we change this we also need to change the diff popup. Since it works on the same git output and changing this output will break the popup. However using that output might improve it.