From 12caa017a96a64d919b5c181789c8faea180c47a Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Sat, 24 Jun 2017 17:51:47 -0400 Subject: [PATCH] Fixes issues with changes showing wrong diff Refactors diff parsing --- src/annotations/annotations.ts | 26 +++---- .../recentChangesAnnotationProvider.ts | 9 ++- src/git/models/diff.ts | 29 +++----- src/git/parsers/diffParser.ts | 67 ++++++++++++++++--- src/gitService.ts | 32 ++------- 5 files changed, 86 insertions(+), 77 deletions(-) diff --git a/src/annotations/annotations.ts b/src/annotations/annotations.ts index 3e422e7..b188787 100644 --- a/src/annotations/annotations.ts +++ b/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 { - 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 diff --git a/src/annotations/recentChangesAnnotationProvider.ts b/src/annotations/recentChangesAnnotationProvider.ts index dd111d3..0d1475a 100644 --- a/src/annotations/recentChangesAnnotationProvider.ts +++ b/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({ diff --git a/src/git/models/diff.ts b/src/git/models/diff.ts index e774782..5685c9f 100644 --- a/src/git/models/diff.ts +++ b/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(); + get lines(): GitDiffChunkLine[] { + if (this._lines === undefined) { + this._lines = GitDiffParser.parseChunk(this._chunk!); + this._chunk = undefined; } - return this._current!; - } - - get previous(): (GitDiffLine | undefined)[] { - if (this._chunk !== undefined) { - this.parseChunk(); - } - - return this._previous!; - } - - private parseChunk() { - [this._current, this._previous] = GitDiffParser.parseChunk(this._chunk!); - this._chunk = undefined; + return this._lines; } } diff --git a/src/git/parsers/diffParser.ts b/src/git/parsers/diffParser.ts index 9266822..d3ab82b 100644 --- a/src/git/parsers/diffParser.ts +++ b/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; } } \ No newline at end of file diff --git a/src/gitService.ts b/src/gitService.ts index d776c65..c2ca7ba 100644 --- a/src/gitService.ts +++ b/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 { 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]; + if (chunk === undefined) return 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++; - } - - 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; } }