Fixes diff line / previous (line number was wrong)

Fixes scrolling to accurate line numbers when diffing in history

src/commands/diffLineWithPrevious.ts

@@ -40,11 +40,11 @@ export class DiffLineWithPreviousCommand extends ActiveEditorCommand {
                 return Messages.showFileNotUnderSourceControlWarningMessage('Unable to open compare');
             }
 
-            // Since there could be a change in the line number, update it
-            args.line = blame.line.originalLine - 1;
-
             // If the line is uncommitted, change the previous commit
             if (blame.commit.isUncommitted) {
+                // Since there could be a change in the line number, update it
+                args.line = blame.line.originalLine - 1;
+
                 try {
                     const previous = await Container.git.getPreviousUri(
                         gitUri.repoPath!,

src/git/git.ts

@@ -707,7 +707,12 @@ export class Git {
             }
         }
 
-        return git<string>({ cwd: root, configs: ['-c', 'log.showSignature=false'] }, ...params, '--', file);
+        if (startLine == null || renames) {
+            // Don't specify a file spec when using a line number (so say the git docs), unless it is a follow
+            params.push('--', file);
+        }
+
+        return git<string>({ cwd: root, configs: ['-c', 'log.showSignature=false'] }, ...params);
     }
 
     static async log_recent(

src/git/gitService.ts

@@ -1809,7 +1809,10 @@ export class GitService implements Disposable {
         });
         if (data == null || data.length === 0) throw new Error('File has no history');
 
-        const [previousRef, file] = GitLogParser.parseSimple(data, skip);
+        const [previousRef, file] = GitLogParser.parseSimple(data, skip, editorLine !== undefined ? ref : undefined);
+        // If the previous ref matches the ref we asked for assume we are at the end of the history
+        if (ref !== undefined && ref === previousRef) return undefined;
+
         return GitUri.fromFile(file || fileName, repoPath, previousRef || GitService.deletedOrMissingSha);
     }
 

src/git/models/logCommit.ts

@@ -6,6 +6,17 @@ import { GitUri } from '../gitUri';
 import { GitCommit, GitCommitType } from './commit';
 import { GitFile, GitFileStatus } from './file';
 
+export interface GitLogCommitLine {
+    from: {
+        line: number;
+        count: number;
+    };
+    to: {
+        line: number;
+        count: number;
+    };
+}
+
 export class GitLogCommit extends GitCommit {
     nextSha?: string;
     nextFileName?: string;
@@ -25,7 +36,8 @@ export class GitLogCommit extends GitCommit {
         originalFileName?: string | undefined,
         previousSha?: string | undefined,
         previousFileName?: string | undefined,
-        public readonly parentShas?: string[]
+        public readonly parentShas?: string[],
+        public readonly line?: GitLogCommitLine
     ) {
         super(
             type,

src/git/parsers/logParser.ts

@@ -1,7 +1,7 @@
 'use strict';
 import * as paths from 'path';
 import { Range } from 'vscode';
-import { Git, GitAuthor, GitCommitType, GitFile, GitFileStatus, GitLog, GitLogCommit } from '../git';
+import { Git, GitAuthor, GitCommitType, GitFile, GitFileStatus, GitLog, GitLogCommit, GitLogCommitLine } from '../git';
 import { Arrays, debug, Strings } from '../../system';
 
 const emptyEntry: LogEntry = {};
@@ -9,6 +9,8 @@ const emptyStr = '';
 const slash = '/';
 
 const diffRegex = /diff --git a\/(.*) b\/(.*)/;
+const diffRangeRegex = /^@@ -(\d+?),(\d+?) \+(\d+?),(\d+?) @@/;
+
 export const fileStatusRegex = /(\S)\S*\t([^\t\n]+)(?:\t(.+))?/;
 const logFileSimpleRegex = /^<r> (.*)\s*(?:(?:diff --git a\/(.*) b\/(.*))|(?:(\S)\S*\t([^\t\n]+)(?:\t(.+))?))/gm;
 
@@ -35,6 +37,8 @@ interface LogEntry {
     status?: GitFileStatus;
 
     summary?: string;
+
+    line?: GitLogCommitLine;
 }
 
 export class GitLogParser {
@@ -211,6 +215,24 @@ export class GitLogParser {
                                     entry.status = 'R';
                                 }
 
+                                next = lines.next();
+                                next = lines.next();
+                                next = lines.next();
+
+                                match = diffRangeRegex.exec(next.value);
+                                if (match !== null) {
+                                    entry.line = {
+                                        from: {
+                                            line: parseInt(match[1], 10),
+                                            count: parseInt(match[2], 10)
+                                        },
+                                        to: {
+                                            line: parseInt(match[3], 10),
+                                            count: parseInt(match[4], 10)
+                                        }
+                                    };
+                                }
+
                                 while (true) {
                                     next = lines.next();
                                     if (next.done || next.value === '</f>') break;
@@ -356,7 +378,8 @@ export class GitLogParser {
                 originalFileName,
                 type === GitCommitType.Branch ? entry.parentShas![0] : undefined,
                 undefined,
-                entry.parentShas!
+                entry.parentShas!,
+                entry.line
             );
 
             commits.set(entry.ref!, commit);
@@ -381,8 +404,14 @@ export class GitLogParser {
     @debug({ args: false })
     static parseSimple(
         data: string,
-        skip: number
+        skip: number,
+        lineRef?: string
     ): [string | undefined, string | undefined, GitFileStatus | undefined] {
+        // Don't skip 1 extra for line-based previous, as we will be skipping the line ref as needed
+        if (lineRef !== undefined) {
+            skip--;
+        }
+
         let match;
         let ref;
         let file;
@@ -394,6 +423,12 @@ export class GitLogParser {
             if (skip-- > 0) continue;
 
             ref = ` ${match[1]}`.substr(1);
+            if (lineRef === ref) {
+                skip++;
+
+                continue;
+            }
+
             file = ` ${match[3] || match[2] || match[6] || match[5]}`.substr(1);
             status = match[4] ? (` ${match[4]}`.substr(1) as GitFileStatus) : undefined;
         } while (skip >= 0);

src/messages.ts

@@ -24,7 +24,7 @@ export class Messages {
         if (commit === undefined) {
             return Messages.showMessage(
                 'info',
-                'Commit has no previous commit.',
+                'There is no previous commit.',
                 SuppressedMessages.CommitHasNoPreviousCommitWarning
             );
         }

src/views/nodes/commitFileNode.ts

@@ -197,9 +197,17 @@ export class CommitFileNode extends ViewRefFileNode {
     }
 
     getCommand(): Command | undefined {
+        let line;
+        if (this.commit.line !== undefined) {
+            line = this.commit.line.to.line - 1;
+        }
+        else {
+            line = this._selection !== undefined ? this._selection.active.line : 0;
+        }
+
         const commandArgs: DiffWithPreviousCommandArgs = {
             commit: this.commit,
-            line: this._selection !== undefined ? this._selection.active.line : 0,
+            line: line,
             showOptions: {
                 preserveFocus: true,
                 preview: true

src/views/nodes/lineHistoryNode.ts

@@ -46,18 +46,6 @@ export class LineHistoryNode extends SubscribeableViewNode implements PageableVi
                 ? await Container.git.getBlameForRangeContents(this.uri, selection, this._editorContents)
                 : await Container.git.getBlameForRange(this.uri, selection);
             if (blame !== undefined) {
-                const firstLine = blame.lines[0];
-                const lastLine = blame.lines[blame.lines.length - 1];
-
-                // Since there could be a change in the line numbers, update the selection
-                const firstActive = selection.active.line === firstLine.line - 1;
-                selection = new Selection(
-                    (firstActive ? lastLine : firstLine).originalLine - 1,
-                    selection.anchor.character,
-                    (firstActive ? firstLine : lastLine).originalLine - 1,
-                    selection.active.character
-                );
-
                 for (const commit of blame.commits.values()) {
                     if (!commit.isUncommitted) continue;
 
@@ -87,6 +75,18 @@ export class LineHistoryNode extends SubscribeableViewNode implements PageableVi
                         commit.originalFileName || commit.fileName
                     );
 
+                    const firstLine = blame.lines[0];
+                    const lastLine = blame.lines[blame.lines.length - 1];
+
+                    // Since there could be a change in the line numbers, update the selection
+                    const firstActive = selection.active.line === firstLine.line - 1;
+                    selection = new Selection(
+                        (firstActive ? lastLine : firstLine).originalLine - 1,
+                        selection.anchor.character,
+                        (firstActive ? firstLine : lastLine).originalLine - 1,
+                        selection.active.character
+                    );
+
                     children.splice(0, 0, new CommitFileNode(this.view, this, file, uncommitted, displayAs, selection));
 
                     break;