From 9c75a749efd438289de70f5602e049b65ee5517c Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Thu, 2 May 2019 22:59:21 -0400 Subject: [PATCH] Fixes diff line / previous (line number was wrong) Fixes scrolling to accurate line numbers when diffing in history --- src/commands/diffLineWithPrevious.ts | 6 +++--- src/git/git.ts | 7 +++++- src/git/gitService.ts | 5 ++++- src/git/models/logCommit.ts | 14 +++++++++++- src/git/parsers/logParser.ts | 41 +++++++++++++++++++++++++++++++++--- src/messages.ts | 2 +- src/views/nodes/commitFileNode.ts | 10 ++++++++- src/views/nodes/lineHistoryNode.ts | 24 ++++++++++----------- 8 files changed, 86 insertions(+), 23 deletions(-) diff --git a/src/commands/diffLineWithPrevious.ts b/src/commands/diffLineWithPrevious.ts index 1e7d26f..72380b8 100644 --- a/src/commands/diffLineWithPrevious.ts +++ b/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!, diff --git a/src/git/git.ts b/src/git/git.ts index 70184b2..0a494d0 100644 --- a/src/git/git.ts +++ b/src/git/git.ts @@ -707,7 +707,12 @@ export class Git { } } - return git({ 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({ cwd: root, configs: ['-c', 'log.showSignature=false'] }, ...params); } static async log_recent( diff --git a/src/git/gitService.ts b/src/git/gitService.ts index c1e89cb..9320b57 100644 --- a/src/git/gitService.ts +++ b/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); } diff --git a/src/git/models/logCommit.ts b/src/git/models/logCommit.ts index c44d076..f26eacb 100644 --- a/src/git/models/logCommit.ts +++ b/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, diff --git a/src/git/parsers/logParser.ts b/src/git/parsers/logParser.ts index 3280110..c4b0c16 100644 --- a/src/git/parsers/logParser.ts +++ b/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 = /^ (.*)\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 === '') 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); diff --git a/src/messages.ts b/src/messages.ts index 553b8b7..70804d1 100644 --- a/src/messages.ts +++ b/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 ); } diff --git a/src/views/nodes/commitFileNode.ts b/src/views/nodes/commitFileNode.ts index 4cf0fe5..d8181fc 100644 --- a/src/views/nodes/commitFileNode.ts +++ b/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 diff --git a/src/views/nodes/lineHistoryNode.ts b/src/views/nodes/lineHistoryNode.ts index 58a1b63..21992f4 100644 --- a/src/views/nodes/lineHistoryNode.ts +++ b/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;