line number of diff is incorrect.

[stevemao]

git show fbf440dd9c32a60f9bc97693a6bd7b5ca87ec9fc
commit fbf440dd9c32a60f9bc97693a6bd7b5ca87ec9fc
Author: Steve Mao <maochenyan@gmail.com>
Date:   Wed Mar 9 19:21:27 2016 +1100

   0.6.2

─────────────────────────────────────────────────────────────────────────────────────────────────────
modified: package.json
─────────────────────────────────────────────────────────────────────────────────────────────────────
@ package.json:4 @
{
  "name": "diff-so-fancy",
  "version": "0.6.1",
  "version": "0.6.2",
  "description": "Good-lookin' diffs with diff-highlight and more",
  "bin": {
    "diff-so-fancy": "diff-so-fancy",

@ package.json:4 @ should be @ package.json:3 @

[scottchiefbaker]

This is a known bug that I don't have a good work around for. The raw chunk header for that request is @@ -1,6 +1,6 @@ which indicates the line changes start on line one. We assume there are three lines of context so we add three to the start line (one) and end up with four. If the changes are in the first three or last three lines of a file there will be less context and the math will be off.

I haven't found a good work around unless we want to scrape the entire diff first to calculate the real offsets. But that would add a lot of overhead for very little gain.

[paulirish]

# of lines of context

We assume there are three lines of context

This number is configurable via some flags to git diff, but I didnt' see a way that we could query for it.

hunk context

so we add three to the start line

I was skeptical this was a good idea, but after trying to make use of the line numbers without the +3, it was really confusing.

btw, the hunk context doesnt necessarily show the source from that line, but will often show the function name. this depends on the language used. see http://stackoverflow.com/questions/28111035/where-does-the-excerpt-in-the-git-diff-hunk-header-come-from AFAICT, the latest answer to this is here: https://github.com/git/git/blob/master/xdiff/xemit.c#L128-L149

also, there appear to be some language-specific parsers in git's userdiff.cthat assist for this:

( i threw this stuff into our wiki to document it: https://github.com/so-fancy/diff-so-fancy/wiki )

ANYWAY, since the context line can be showing something that's VERY different with the file.ext:4 location, its better that the line position doesn't show the unmodified hunk_start number.

changes in the first lines of a file

If the changes are in the first three or last three lines of a file

detecting that we're in the first three is easy enough. We can fix those values, IMO.

I think we should do that.

[paulirish]

@scottchiefbaker can we adjust the line calculation if N is <= 3?

[scottchiefbaker]

I'll need some help with the logic. If I have a change on the first line the (raw) diff looks like this:

diff --git a/appveyor.yml b/appveyor.yml
index a6fb95e..8e6e261 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -1,4 +1,4 @@
-install:
+INSTALL:
   - git clone --depth 1 https://github.com/sstephenson/bats.git

 build: false

Currently we iterate over each line of the file, when we find a diff hunk we mangle that to output the new header. I won't know how many lines of context unless we parse the actual body of the diff, which gets a lot more complex. Or is there another way I'm missing.

[scottchiefbaker]

If I make a change on line two I get: @@ -1,5 +1,5 @@, if I make a change on line three I get @@ -1,6 +1,6 @@. It's not until the 5th line does the header indicate anything other than +1/-1

[scottchiefbaker]

Ok I think maybe I got it... What do you think about this raw data:

1: @@ -1,4 +1,4 @@
2: @@ -1,5 +1,5 @@
3: @@ -1,6 +1,6 @@
4: @@ -1,7 +1,7 @@
5: @@ -2,6 +2,7 @@
6: @@ -3,7 +3,7 @@

# Starting from the end (last line (10), second to last line (9))...
-1: @@ -7,4 +7,4 @@
-2: @@ -6,5 +6,5 @@
-3: @@ -5,6 +5,6 @@
-4: @@ -4,7 +4,7 @@

Using that logic I wrote this function:

sub line_calc {
    my ($line_num,$diff_context) = @_;

    # Git defaults to three lines of context
    my $default_context_lines = 3;
    # Three lines on either side, and the line itself = 7
    my $expected_context      = ($default_context_lines * 2 + 1);

    # The first three lines
    if ($line_num == 1 && $diff_context < $expected_context) {
        return $diff_context - $default_context_lines;
    } else {
        return $line_num + $default_context_lines;
    }
}

Just to clarify the current method only falls apart on the first 3 lines, everything else is fine. This should solve all the corners cases, except files that are less than 7 lines.

[scottchiefbaker]

Ok I'm replying to myself because all the cool kids are doing that these days. I think I have the logic applied in a branch on my repo. Will someone please test this out and see if it does what you want it to: d1a4c91

If so I'll submit a pull request.

[stevemao]

I'll add tests for this.

[scottchiefbaker]

👍

[laggardkernel]

The bug still exists in diff-so-fancy 1.4.3.

git show without diff-so-fancy

diff --git a/dotfiles/.config/git/config b/dotfiles/.config/git/config
index eaf6af0..17c61af 100644
--- a/dotfiles/.config/git/config
+++ b/dotfiles/.config/git/config
@@ -1,12 +1,12 @@
+; vim: fdm=marker foldlevel=0 sw=0 ts=4 sts=4 noet
+; Note:
+; - section could be defined multiple times, the latter overwrite the former.
 [user]

With diff-so-fancy

────────────────────────────────────────────────────────────────────────────────────>
modified: dotfiles/.config/git/config
────────────────────────────────────────────────────────────────────────────────────>
@ dotfiles/.config/git/config:4 @
; vim: fdm=marker foldlevel=0 sw=0 ts=4 sts=4 noet
; Note:
; - section could be defined multiple times, the latter overwrite the former.
[user]

Line number 1 becomes 4.

Related git config for diff-so-fancy

; diff-so-fancy {{{
; https://github.com/so-fancy/diff-so-fancy
[core]
	; Remove -F from less, which implies -X only on less 530+
	; -X means no screen clearing. -F will not clear content
	pager = diff-so-fancy | less -g -i -J -M -R -S -w -x4 -z-4
[interactive]
	diffFilter = diff-so-fancy --patch
[color]
	ui = true
[color "diff"]
	meta         = 11
	frag         = magenta bold
	commit       = yellow bold
	old          = red bold
	new          = green bold
	whitespace   = red reverse
	func         = 146 bold
[color "diff-highlight"]
	oldHighlight = red bold 52
	newNormal    = green bold
	newHighlight = green bold 22
	oldNormal    = red bold
; }}}

[OJFord]

@laggardkernel Is that the whole diff? Seems like it's missing another 8 lines of context and 3 removed?