Fixes issues with changes showing wrong diff

Refactors diff parsing

src/annotations/annotations.ts

@@ -2,7 +2,7 @@ import { Strings } from '../system';
 import { DecorationInstanceRenderOptions, DecorationOptions, ThemableDecorationRenderOptions } from 'vscode';
 import { IThemeConfig, themeDefaults } from '../configuration';
 import { GlyphChars } from '../constants';
-import { CommitFormatter, GitCommit, GitDiffLine, GitService, GitUri, ICommitFormatOptions } from '../gitService';
+import { CommitFormatter, GitCommit, GitDiffChunkLine, GitService, GitUri, ICommitFormatOptions } from '../gitService';
 import * as moment from 'moment';
 
 interface IHeatmapConfig {
@@ -66,32 +66,26 @@ export class Annotations {
         return `\`${commit.shortSha}\`   __${commit.author}__, ${moment(commit.date).fromNow()}   _(${moment(commit.date).format(dateFormat)})_${message}`;
     }
 
-    static getHoverDiffMessage(commit: GitCommit, previous: GitDiffLine | undefined, current: GitDiffLine | undefined): string | undefined {
-        if (previous === undefined && current === undefined) return undefined;
+    static getHoverDiffMessage(commit: GitCommit, chunkLine: GitDiffChunkLine | undefined): string | undefined {
+        if (chunkLine === undefined) return undefined;
 
-        const codeDiff = this._getCodeDiff(previous, current);
+        const codeDiff = this._getCodeDiff(chunkLine);
         return commit.isUncommitted
             ? `\`Changes\`   ${GlyphChars.Dash}   _uncommitted_\n${codeDiff}`
             : `\`Changes\`   ${GlyphChars.Dash}   \`${commit.previousShortSha}\` ${GlyphChars.ArrowLeftRight} \`${commit.shortSha}\`\n${codeDiff}`;
     }
 
-    private static _getCodeDiff(previous: GitDiffLine | undefined, current: GitDiffLine | undefined): string {
+    private static _getCodeDiff(chunkLine: GitDiffChunkLine): string {
+        const previous = chunkLine.previous === undefined ? undefined : chunkLine.previous[0];
         return `\`\`\`
--  ${previous === undefined ? '' : previous.line.trim()}
-+  ${current === undefined ? '' : current.line.trim()}
+-  ${previous === undefined || previous.line === undefined ? '' : previous.line.trim()}
++  ${chunkLine.line === undefined ? '' : chunkLine.line.trim()}
 \`\`\``;
     }
 
     static async changesHover(commit: GitCommit, line: number, uri: GitUri, git: GitService): Promise<DecorationOptions> {
-        let message: string | undefined = undefined;
-        if (commit.isUncommitted) {
-            const [previous, current] = await git.getDiffForLine(uri, line + uri.offset);
-            message = this.getHoverDiffMessage(commit, previous, current);
-        }
-        else if (commit.previousSha !== undefined) {
-            const [previous, current] = await git.getDiffForLine(uri, line + uri.offset, commit.previousSha);
-            message = this.getHoverDiffMessage(commit, previous, current);
-        }
+        const chunkLine = await git.getDiffForLine(uri, line + uri.offset, commit.isUncommitted ? undefined : commit.previousSha);
+        const message = this.getHoverDiffMessage(commit, chunkLine);
 
         return {
             hoverMessage: message

src/annotations/recentChangesAnnotationProvider.ts

@@ -27,12 +27,12 @@ export class RecentChangesAnnotationProvider extends AnnotationProviderBase {
 
         for (const chunk of diff.chunks) {
             let count = chunk.currentPosition.start - 2;
-            for (const change of chunk.current) {
-                if (change === undefined) continue;
+            for (const line of chunk.lines) {
+                if (line.line === undefined) continue;
 
                 count++;
 
-                if (change.state === 'unchanged') continue;
+                if (line.state === 'unchanged') continue;
 
                 let endingIndex = 0;
                 if (cfg.hover.details || cfg.hover.changes) {
@@ -50,8 +50,7 @@ export class RecentChangesAnnotationProvider extends AnnotationProviderBase {
 
                 let message: string | undefined = undefined;
                 if (cfg.hover.changes) {
-                    const previousPos = count - (chunk.previousPosition.start + (chunk.previousPosition.start - chunk.currentPosition.start)) + 1;
-                    message = Annotations.getHoverDiffMessage(commit, chunk.previous[previousPos], change);
+                    message = Annotations.getHoverDiffMessage(commit, line);
                 }
 
                 decorators.push({

src/git/models/diff.ts

@@ -6,35 +6,26 @@ export interface GitDiffLine {
     state: 'added' | 'removed' | 'unchanged';
 }
 
+export interface GitDiffChunkLine extends GitDiffLine {
+    previous?: (GitDiffLine | undefined)[];
+}
+
 export class GitDiffChunk {
 
     private _chunk: string | undefined;
-    private _current: (GitDiffLine | undefined)[] | undefined;
-    private _previous: (GitDiffLine | undefined)[] | undefined;
+    private _lines: GitDiffChunkLine[] | undefined;
 
     constructor(chunk: string, public currentPosition: { start: number, end: number }, public previousPosition: { start: number, end: number }) {
         this._chunk = chunk;
      }
 
-    get current(): (GitDiffLine | undefined)[] {
-        if (this._chunk !== undefined) {
-            this.parseChunk();
-        }
-
-        return this._current!;
-    }
-
-    get previous(): (GitDiffLine | undefined)[] {
-        if (this._chunk !== undefined) {
-            this.parseChunk();
+    get lines(): GitDiffChunkLine[] {
+        if (this._lines === undefined) {
+            this._lines = GitDiffParser.parseChunk(this._chunk!);
+            this._chunk = undefined;
         }
 
-        return this._previous!;
-    }
-
-    private parseChunk() {
-        [this._current, this._previous] = GitDiffParser.parseChunk(this._chunk!);
-        this._chunk = undefined;
+        return this._lines;
     }
 }
 

src/git/parsers/diffParser.ts

@@ -1,6 +1,6 @@
 'use strict';
 import { Iterables, Strings } from '../../system';
-import { GitDiff, GitDiffChunk, GitDiffLine } from './../git';
+import { GitDiff, GitDiffChunk, GitDiffChunkLine, GitDiffLine } from './../git';
 
 const unifiedDiffRegex = /^@@ -([\d]+),([\d]+) [+]([\d]+),([\d]+) @@([\s\S]*?)(?=^@@)/gm;
 
@@ -39,36 +39,81 @@ export class GitDiffParser {
         return diff;
     }
 
-    static parseChunk(chunk: string): [(GitDiffLine | undefined)[], (GitDiffLine | undefined)[]] {
+    static parseChunk(chunk: string): GitDiffChunkLine[] {
         const lines = Iterables.skip(Strings.lines(chunk), 1);
 
-        const current: (GitDiffLine | undefined)[] = [];
-        const previous: (GitDiffLine | undefined)[] = [];
+        const currentLines: (GitDiffLine | undefined)[] = [];
+        const previousLines: (GitDiffLine | undefined)[] = [];
+
+        let removed = 0;
         for (const l of lines) {
             switch (l[0]) {
                 case '+':
-                    current.push({
+                    currentLines.push({
                         line: ` ${l.substring(1)}`,
                         state: 'added'
                     });
-                    previous.push(undefined);
+
+                    if (removed > 0) {
+                        removed--;
+                    }
+                    else {
+                        previousLines.push(undefined);
+                    }
+
                     break;
 
                 case '-':
-                    current.push(undefined);
-                    previous.push({
+                    removed++;
+
+                    previousLines.push({
                         line: ` ${l.substring(1)}`,
                         state: 'removed'
                     });
+
                     break;
 
                 default:
-                    current.push({ line: l, state: 'unchanged' });
-                    previous.push({ line: l, state: 'unchanged' });
+                    while (removed > 0) {
+                        removed--;
+                        currentLines.push(undefined);
+                    }
+
+                    currentLines.push({ line: l, state: 'unchanged' });
+                    previousLines.push({ line: l, state: 'unchanged' });
+
                     break;
             }
         }
 
-        return [current, previous];
+        const chunkLines: GitDiffChunkLine[] = [];
+
+        let chunkLine: GitDiffChunkLine | undefined = undefined;
+        let current: GitDiffLine | undefined = undefined;
+
+        for (let i = 0; i < currentLines.length; i++) {
+            current = currentLines[i];
+            if (current === undefined) {
+                // Don't think we need to worry about this case because the diff will always have "padding" (i.e. unchanged lines) around each chunk
+                if (chunkLine === undefined) continue;
+
+                if (chunkLine.previous === undefined) {
+                    chunkLine.previous = [previousLines[i]];
+                    continue;
+                }
+
+                chunkLine.previous.push(previousLines[i]);
+                continue;
+            }
+
+            chunkLine = {
+                line: current.line,
+                state: current.state,
+                previous: [previousLines[i]]
+            };
+            chunkLines.push(chunkLine);
+        }
+
+        return chunkLines;
     }
 }

src/gitService.ts

@@ -3,7 +3,7 @@ import { Iterables, Objects } from './system';
 import { Disposable, Event, EventEmitter, ExtensionContext, FileSystemWatcher, languages, Location, Position, Range, TextDocument, TextDocumentChangeEvent, TextEditor, Uri, workspace } from 'vscode';
 import { IConfig } from './configuration';
 import { CommandContext, DocumentSchemes, ExtensionKey, GlyphChars, setCommandContext } from './constants';
-import { Git, GitAuthor, GitBlame, GitBlameCommit, GitBlameLine, GitBlameLines, GitBlameParser, GitBranch, GitCommit, GitDiff, GitDiffLine, GitDiffParser, GitLog, GitLogCommit, GitLogParser, GitRemote, GitStash, GitStashParser, GitStatus, GitStatusFile, GitStatusParser, IGit, setDefaultEncoding } from './git/git';
+import { Git, GitAuthor, GitBlame, GitBlameCommit, GitBlameLine, GitBlameLines, GitBlameParser, GitBranch, GitCommit, GitDiff, GitDiffChunkLine, GitDiffParser, GitLog, GitLogCommit, GitLogParser, GitRemote, GitStash, GitStashParser, GitStatus, GitStatusFile, GitStatusParser, IGit, setDefaultEncoding } from './git/git';
 import { GitUri, IGitCommitInfo, IGitUriData } from './git/gitUri';
 import { GitCodeLensProvider } from './gitCodeLensProvider';
 import { Logger } from './logger';
@@ -647,38 +647,18 @@ export class GitService extends Disposable {
         }
     }
 
-    async getDiffForLine(uri: GitUri, line: number, sha1?: string, sha2?: string): Promise<[GitDiffLine | undefined, GitDiffLine | undefined]> {
+    async getDiffForLine(uri: GitUri, line: number, sha1?: string, sha2?: string): Promise<GitDiffChunkLine | undefined> {
         try {
             const diff = await this.getDiffForFile(uri, sha1, sha2);
-            if (diff === undefined) return [undefined, undefined];
+            if (diff === undefined) return undefined;
 
             const chunk = diff.chunks.find(_ => _.currentPosition.start <= line && _.currentPosition.end >= line);
-            if (chunk === undefined) return [undefined, undefined];
-
-            // Search for the line (skipping deleted lines -- since they don't currently exist in the editor)
-            // Keep track of the deleted lines for the original version
-            line = line - chunk.currentPosition.start + 1;
-            let count = 0;
-            let deleted = 0;
-            for (const l of chunk.current) {
-                if (l === undefined) {
-                    deleted++;
-                    if (count === line) break;
-
-                    continue;
-                }
-
-                if (count === line) break;
-                count++;
-            }
+            if (chunk === undefined) return undefined;
 
-            return [
-                chunk.previous[line + deleted - 1],
-                chunk.current[line + deleted + (chunk.currentPosition.start - chunk.previousPosition.start)]
-            ];
+            return chunk.lines[line - chunk.currentPosition.start + 1];
         }
         catch (ex) {
-            return [undefined, undefined];
+            return undefined;
         }
     }