Fixes #716 - reworks diff w/ next

Disables commands for the deleted or missing sha
Adds getNextDiffUris & getNextUri
Renames getDiffWithPreviousForFile to getPreviousDiffUris
Renames getPreviousRevisionUri to getPreviousUri

src/commands/diffLineWithPrevious.ts

@@ -46,7 +46,7 @@ export class DiffLineWithPreviousCommand extends ActiveEditorCommand {
             // If the line is uncommitted, change the previous commit
             if (blame.commit.isUncommitted) {
                 try {
-                    const previous = await Container.git.getPreviousRevisionUri(
+                    const previous = await Container.git.getPreviousUri(
                         gitUri.repoPath!,
                         gitUri,
                         gitUri.sha,
@@ -77,7 +77,7 @@ export class DiffLineWithPreviousCommand extends ActiveEditorCommand {
                     Logger.error(
                         ex,
                         'DiffLineWithPreviousCommand',
-                        `getPreviousRevisionUri(${gitUri.repoPath}, ${gitUri.fsPath}, ${gitUri.sha})`
+                        `getPreviousUri(${gitUri.repoPath}, ${gitUri.fsPath}, ${gitUri.sha})`
                     );
                     return Messages.showGenericErrorMessage('Unable to open compare');
                 }
@@ -85,7 +85,7 @@ export class DiffLineWithPreviousCommand extends ActiveEditorCommand {
         }
 
         try {
-            const diffWith = await Container.git.getDiffWithPreviousForFile(
+            const diffUris = await Container.git.getPreviousDiffUris(
                 gitUri.repoPath!,
                 gitUri,
                 gitUri.sha,
@@ -93,19 +93,19 @@ export class DiffLineWithPreviousCommand extends ActiveEditorCommand {
                 args.line
             );
 
-            if (diffWith === undefined || diffWith.previous === undefined) {
+            if (diffUris === undefined || diffUris.previous === undefined) {
                 return Messages.showCommitHasNoPreviousCommitWarningMessage();
             }
 
             const diffArgs: DiffWithCommandArgs = {
-                repoPath: diffWith.current.repoPath,
+                repoPath: diffUris.current.repoPath,
                 lhs: {
-                    sha: diffWith.previous.sha || '',
-                    uri: diffWith.previous.documentUri()
+                    sha: diffUris.previous.sha || '',
+                    uri: diffUris.previous.documentUri()
                 },
                 rhs: {
-                    sha: diffWith.current.sha || '',
-                    uri: diffWith.current.documentUri()
+                    sha: diffUris.current.sha || '',
+                    uri: diffUris.current.documentUri()
                 },
                 line: args.line,
                 showOptions: args.showOptions
@@ -116,7 +116,7 @@ export class DiffLineWithPreviousCommand extends ActiveEditorCommand {
             Logger.error(
                 ex,
                 'DiffLineWithPreviousCommand',
-                `getDiffWithPreviousForFile(${gitUri.repoPath}, ${gitUri.fsPath}, ${gitUri.sha})`
+                `getPreviousDiffUris(${gitUri.repoPath}, ${gitUri.fsPath}, ${gitUri.sha})`
             );
             return Messages.showGenericErrorMessage('Unable to open compare');
         }

src/commands/diffWithNext.ts

@@ -1,10 +1,9 @@
 'use strict';
 import { commands, Range, TextDocumentShowOptions, TextEditor, Uri } from 'vscode';
 import { Container } from '../container';
-import { GitLogCommit, GitService, GitStatusFile, GitUri } from '../git/gitService';
+import { GitLogCommit, GitUri } from '../git/gitService';
 import { Logger } from '../logger';
 import { Messages } from '../messages';
-import { Iterables } from '../system';
 import { ActiveEditorCommand, command, CommandContext, Commands, getCommandUri } from './common';
 import { DiffWithCommandArgs } from './diffWith';
 import { UriComparer } from '../comparers';
@@ -52,101 +51,34 @@ export class DiffWithNextCommand extends ActiveEditorCommand {
             args.line = editor == null ? 0 : editor.selection.active.line;
         }
 
-        const gitUri = await GitUri.fromUri(uri);
-        let status: GitStatusFile | undefined;
-
-        if (args.commit === undefined || !(args.commit instanceof GitLogCommit) || args.range !== undefined) {
-            try {
-                const sha = args.commit === undefined ? gitUri.sha! : args.commit.sha;
-
-                if (GitService.isStagedUncommitted(sha)) {
-                    const diffArgs: DiffWithCommandArgs = {
-                        repoPath: gitUri.repoPath!,
-                        lhs: {
-                            sha: sha,
-                            uri: gitUri
-                        },
-                        rhs: {
-                            sha: '',
-                            uri: gitUri
-                        },
-                        line: args.line,
-                        showOptions: args.showOptions
-                    };
-                    return commands.executeCommand(Commands.DiffWith, diffArgs);
-                }
-
-                let log = await Container.git.getLogForFile(gitUri.repoPath, gitUri.fsPath, {
-                    maxCount: sha !== undefined ? undefined : 2,
-                    range: args.range!,
-                    renames: true
-                });
-                if (log === undefined) {
-                    const fileName = await Container.git.findNextFileName(gitUri.repoPath!, gitUri.fsPath);
-                    if (fileName !== undefined) {
-                        log = await Container.git.getLogForFile(gitUri.repoPath, fileName, {
-                            maxCount: sha !== undefined ? undefined : 2,
-                            range: args.range!,
-                            renames: true
-                        });
-                    }
-
-                    if (log === undefined) {
-                        return Messages.showFileNotUnderSourceControlWarningMessage('Unable to open compare');
-                    }
-                }
-
-                args.commit = (sha && log.commits.get(sha)) || Iterables.first(log.commits.values());
-
-                // If the sha is missing or the file is uncommitted, treat it as a DiffWithWorking
-                if (gitUri.sha === undefined) {
-                    status = await Container.git.getStatusForFile(gitUri.repoPath!, gitUri.fsPath);
-                    if (status !== undefined) return commands.executeCommand(Commands.DiffWithWorking, uri);
-                }
-            }
-            catch (ex) {
-                Logger.error(ex, 'DiffWithNextCommand', `getLogForFile(${gitUri.repoPath}, ${gitUri.fsPath})`);
-                return Messages.showGenericErrorMessage('Unable to open compare');
-            }
+        const gitUri = args.commit !== undefined ? GitUri.fromCommit(args.commit) : await GitUri.fromUri(uri);
+        try {
+            const diffUris = await Container.git.getNextDiffUris(gitUri.repoPath!, gitUri, gitUri.sha);
+
+            if (diffUris === undefined || diffUris.next === undefined) return undefined;
+
+            const diffArgs: DiffWithCommandArgs = {
+                repoPath: diffUris.current.repoPath,
+                lhs: {
+                    sha: diffUris.current.sha || '',
+                    uri: diffUris.current.documentUri()
+                },
+                rhs: {
+                    sha: diffUris.next.sha || '',
+                    uri: diffUris.next.documentUri()
+                },
+                line: args.line,
+                showOptions: args.showOptions
+            };
+            return commands.executeCommand(Commands.DiffWith, diffArgs);
         }
-
-        if (args.commit.nextSha === undefined) {
-            // Check if the file is staged
-            status = status || (await Container.git.getStatusForFile(gitUri.repoPath!, gitUri.fsPath));
-            if (status !== undefined && status.indexStatus === 'M') {
-                const diffArgs: DiffWithCommandArgs = {
-                    repoPath: args.commit.repoPath,
-                    lhs: {
-                        sha: args.commit.sha,
-                        uri: args.commit.uri
-                    },
-                    rhs: {
-                        sha: GitService.stagedUncommittedSha,
-                        uri: args.commit.uri
-                    },
-                    line: args.line,
-                    showOptions: args.showOptions
-                };
-
-                return commands.executeCommand(Commands.DiffWith, diffArgs);
-            }
-
-            return commands.executeCommand(Commands.DiffWithWorking, uri);
+        catch (ex) {
+            Logger.error(
+                ex,
+                'DiffWithNextCommand',
+                `getNextDiffUris(${gitUri.repoPath}, ${gitUri.fsPath}, ${gitUri.sha})`
+            );
+            return Messages.showGenericErrorMessage('Unable to open compare');
         }
-
-        const diffArgs: DiffWithCommandArgs = {
-            repoPath: args.commit.repoPath,
-            lhs: {
-                sha: args.commit.sha,
-                uri: args.commit.uri
-            },
-            rhs: {
-                sha: args.commit.nextSha,
-                uri: args.commit.nextUri
-            },
-            line: args.line,
-            showOptions: args.showOptions
-        };
-        return commands.executeCommand(Commands.DiffWith, diffArgs);
     }
 }

src/commands/diffWithPrevious.ts

@@ -52,7 +52,7 @@ export class DiffWithPreviousCommand extends ActiveEditorCommand {
 
         const gitUri = args.commit !== undefined ? GitUri.fromCommit(args.commit) : await GitUri.fromUri(uri);
         try {
-            const diffWith = await Container.git.getDiffWithPreviousForFile(
+            const diffUris = await Container.git.getPreviousDiffUris(
                 gitUri.repoPath!,
                 gitUri,
                 gitUri.sha,
@@ -60,19 +60,19 @@ export class DiffWithPreviousCommand extends ActiveEditorCommand {
                 args.inDiffEditor ? 1 : 0
             );
 
-            if (diffWith === undefined || diffWith.previous === undefined) {
+            if (diffUris === undefined || diffUris.previous === undefined) {
                 return Messages.showCommitHasNoPreviousCommitWarningMessage();
             }
 
             const diffArgs: DiffWithCommandArgs = {
-                repoPath: diffWith.current.repoPath,
+                repoPath: diffUris.current.repoPath,
                 lhs: {
-                    sha: diffWith.previous.sha || '',
-                    uri: diffWith.previous.documentUri()
+                    sha: diffUris.previous.sha || '',
+                    uri: diffUris.previous.documentUri()
                 },
                 rhs: {
-                    sha: diffWith.current.sha || '',
-                    uri: diffWith.current.documentUri()
+                    sha: diffUris.current.sha || '',
+                    uri: diffUris.current.documentUri()
                 },
                 line: args.line,
                 showOptions: args.showOptions
@@ -83,7 +83,7 @@ export class DiffWithPreviousCommand extends ActiveEditorCommand {
             Logger.error(
                 ex,
                 'DiffWithPreviousCommand',
-                `getDiffWithPreviousForFile(${gitUri.repoPath}, ${gitUri.fsPath}, ${gitUri.sha})`
+                `getPreviousDiffUris(${gitUri.repoPath}, ${gitUri.fsPath}, ${gitUri.sha})`
             );
             return Messages.showGenericErrorMessage('Unable to open compare');
         }

src/git/git.ts

@@ -8,12 +8,16 @@ import { Logger } from '../logger';
 import { Objects, Strings } from '../system';
 import { findGitPath, GitLocation } from './locator';
 import { run, RunOptions } from './shell';
+import { GitLogParser, GitStashParser } from './parsers/parsers';
+import { GitFileStatus } from './models/file';
 
 export { GitLocation } from './locator';
 export * from './models/models';
 export * from './parsers/parsers';
 export * from './remotes/provider';
 
+export type GitLogDiffFilter = Exclude<GitFileStatus, '!' | '?'>;
+
 const emptyArray = (Object.freeze([]) as any) as any[];
 const emptyObj = Object.freeze({});
 const emptyStr = '';
@@ -22,46 +26,6 @@ const slash = '/';
 // This is a root sha of all git repo's if using sha1
 const rootSha = '4b825dc642cb6eb9a060e54bf8d69288fbee4904';
 
-const defaultBlameParams = ['blame', '--root', '--incremental'];
-
-// Using %x00 codes because some shells seem to try to expand things if not
-const lb = '%x3c'; // `%x${'<'.charCodeAt(0).toString(16)}`;
-const rb = '%x3e'; // `%x${'>'.charCodeAt(0).toString(16)}`;
-const sl = '%x2f'; // `%x${'/'.charCodeAt(0).toString(16)}`;
-const sp = '%x20'; // `%x${' '.charCodeAt(0).toString(16)}`;
-
-const logFormat = [
-    `${lb}${sl}f${rb}`,
-    `${lb}r${rb}${sp}%H`, // ref
-    `${lb}a${rb}${sp}%aN`, // author
-    `${lb}e${rb}${sp}%aE`, // email
-    `${lb}d${rb}${sp}%at`, // date
-    `${lb}c${rb}${sp}%ct`, // committed date
-    `${lb}p${rb}${sp}%P`, // parents
-    `${lb}s${rb}`,
-    '%B', // summary
-    `${lb}${sl}s${rb}`,
-    `${lb}f${rb}`
-].join('%n');
-
-const logSimpleFormat = `${lb}r${rb}${sp}%H`;
-
-const defaultLogParams = ['log', '--name-status', `--format=${logFormat}`];
-
-const stashFormat = [
-    `${lb}${sl}f${rb}`,
-    `${lb}r${rb}${sp}%H`, // ref
-    `${lb}d${rb}${sp}%at`, // date
-    `${lb}c${rb}${sp}%ct`, // committed date
-    `${lb}l${rb}${sp}%gd`, // reflog-selector
-    `${lb}s${rb}`,
-    '%B', // summary
-    `${lb}${sl}s${rb}`,
-    `${lb}f${rb}`
-].join('%n');
-
-const defaultStashParams = ['stash', 'list', '--name-status', '-M', `--format=${stashFormat}`];
-
 const GitErrors = {
     badRevision: /bad revision '(.*?)'/i,
     notAValidObjectName: /Not a valid object name/i
@@ -360,7 +324,7 @@ export class Git {
     ) {
         const [file, root] = Git.splitPath(fileName, repoPath);
 
-        const params = [...defaultBlameParams];
+        const params = ['blame', '--root', '--incremental'];
 
         if (options.ignoreWhitespace) {
             params.push('-w');
@@ -403,7 +367,7 @@ export class Git {
     ) {
         const [file, root] = Git.splitPath(fileName, repoPath);
 
-        const params = [...defaultBlameParams];
+        const params = ['blame', '--root', '--incremental'];
 
         if (options.ignoreWhitespace) {
             params.push('-w');
@@ -625,22 +589,30 @@ export class Git {
         return git<string>({ cwd: repoPath }, ...params);
     }
 
-    static log(repoPath: string, options: { authors?: string[]; maxCount?: number; ref?: string; reverse?: boolean }) {
-        const params = [...defaultLogParams, '--full-history', '-M', '-m'];
-        if (options.authors) {
-            params.push('--use-mailmap', ...options.authors.map(a => `--author=${a}`));
+    static log(
+        repoPath: string,
+        ref: string | undefined,
+        { authors, maxCount, reverse }: { authors?: string[]; maxCount?: number; reverse?: boolean }
+    ) {
+        const params = ['log', '--name-status', `--format=${GitLogParser.defaultFormat}`, '--full-history', '-M', '-m'];
+        if (maxCount && !reverse) {
+            params.push(`-n${maxCount}`);
         }
-        if (options.maxCount && !options.reverse) {
-            params.push(`-n${options.maxCount}`);
+
+        if (authors) {
+            params.push('--use-mailmap', ...authors.map(a => `--author=${a}`));
         }
-        if (options.ref && !Git.isStagedUncommitted(options.ref)) {
-            if (options.reverse) {
-                params.push('--reverse', '--ancestry-path', `${options.ref}..HEAD`);
+
+        if (ref && !Git.isStagedUncommitted(ref)) {
+            // If we are reversing, we must add a range (with HEAD) because we are using --ancestry-path for better reverse walking
+            if (reverse) {
+                params.push('--reverse', '--ancestry-path', `${ref}..HEAD`);
             }
             else {
-                params.push(options.ref);
+                params.push(ref);
             }
         }
+
         return git<string>(
             { cwd: repoPath, configs: ['-c', 'diff.renameLimit=0', '-c', 'log.showSignature=false'] },
             ...params,
@@ -651,56 +623,57 @@ export class Git {
     static log_file(
         repoPath: string,
         fileName: string,
-        options: {
+        ref: string | undefined,
+        {
+            filters,
+            format = GitLogParser.defaultFormat,
+            maxCount,
+            renames = true,
+            reverse = false,
+            startLine,
+            endLine
+        }: {
+            filters?: GitLogDiffFilter[];
+            format?: string;
             maxCount?: number;
-            ref?: string;
             renames?: boolean;
             reverse?: boolean;
             startLine?: number;
             endLine?: number;
-        } = { renames: true, reverse: false }
+        } = {}
     ) {
         const [file, root] = Git.splitPath(fileName, repoPath);
 
-        const params = [...defaultLogParams];
-        if (options.maxCount && !options.reverse) {
-            params.push(`-n${options.maxCount}`);
-        }
-        params.push(options.renames ? '--follow' : '-m');
+        const params = ['log', '--name-status', `--format=${format}`];
 
-        if (options.ref && !Git.isStagedUncommitted(options.ref)) {
-            if (options.reverse) {
-                params.push('--reverse', '--ancestry-path', `${options.ref}..HEAD`);
-            }
-            else {
-                params.push(options.ref);
-            }
+        if (maxCount && !reverse) {
+            params.push(`-n${maxCount}`);
         }
+        params.push(renames ? '--follow' : '-m');
 
-        if (options.startLine != null && options.endLine != null) {
-            params.push(`-L ${options.startLine},${options.endLine}:${file}`);
+        if (filters != null && filters.length !== 0) {
+            params.push(`--diff-filter=${filters.join(emptyStr)}`);
         }
 
-        return git<string>({ cwd: root }, ...params, '--', file);
-    }
-
-    static log_file_simple(repoPath: string, fileName: string, ref?: string, count: number = 2, line?: number) {
-        const [file, root] = Git.splitPath(fileName, repoPath);
-
-        const params = ['log', `--format=${logSimpleFormat}`, `-n${count}`, '--follow'];
-        if (ref && !Git.isStagedUncommitted(ref)) {
-            params.push(ref);
+        if (startLine == null) {
+            params.push('--name-status');
         }
-
-        if (line != null) {
+        else {
             // Don't include --name-status or -s because Git won't honor it
-            params.push(/*'-s',*/ `-L ${line},${line}:${file}`);
+            params.push(`-L ${startLine},${endLine == null ? startLine : endLine}:${file}`);
         }
-        else {
-            params.push('--name-status');
+
+        if (ref && !Git.isStagedUncommitted(ref)) {
+            // If we are reversing, we must add a range (with HEAD) because we are using --ancestry-path for better reverse walking
+            if (reverse) {
+                params.push('--reverse', '--ancestry-path', `${ref}..HEAD`);
+            }
+            else {
+                params.push(ref);
+            }
         }
 
-        return git<string>({ cwd: root }, ...params, '--', file);
+        return git<string>({ cwd: root, configs: ['-c', 'log.showSignature=false'] }, ...params, '--', file);
     }
 
     static async log_recent(repoPath: string, fileName: string) {
@@ -716,10 +689,10 @@ export class Git {
         return data.length === 0 ? undefined : data.trim();
     }
 
-    static log_search(repoPath: string, search: string[] = emptyArray, options: { maxCount?: number } = {}) {
-        const params = [...defaultLogParams];
-        if (options.maxCount) {
-            params.push(`-n${options.maxCount}`);
+    static log_search(repoPath: string, search: string[] = emptyArray, { maxCount }: { maxCount?: number } = {}) {
+        const params = ['log', '--name-status', `--format=${GitLogParser.defaultFormat}`];
+        if (maxCount) {
+            params.push(`-n${maxCount}`);
         }
 
         return git<string>({ cwd: repoPath }, ...params, ...search);
@@ -909,8 +882,8 @@ export class Git {
         return git<string>({ cwd: repoPath }, 'stash', 'drop', stashName);
     }
 
-    static stash_list(repoPath: string) {
-        return git<string>({ cwd: repoPath }, ...defaultStashParams);
+    static stash_list(repoPath: string, { format = GitStashParser.defaultFormat }: { format?: string } = {}) {
+        return git<string>({ cwd: repoPath }, 'stash', 'list', '--name-status', '-M', `--format=${format}`);
     }
 
     static stash_push(repoPath: string, pathspecs: string[], message?: string) {

src/git/gitService.ts

@@ -49,6 +49,7 @@ import {
     GitFile,
     GitLog,
     GitLogCommit,
+    GitLogDiffFilter,
     GitLogParser,
     GitRemote,
     GitRemoteParser,
@@ -1371,15 +1372,14 @@ export class GitService implements Disposable {
     @log()
     async getLog(
         repoPath: string,
-        options: { authors?: string[]; maxCount?: number; ref?: string; reverse?: boolean } = {}
+        { ref, ...options }: { authors?: string[]; maxCount?: number; ref?: string; reverse?: boolean } = {}
     ): Promise<GitLog | undefined> {
         const maxCount = options.maxCount == null ? Container.config.advanced.maxListItems || 0 : options.maxCount;
 
         try {
-            const data = await Git.log(repoPath, {
+            const data = await Git.log(repoPath, ref, {
                 authors: options.authors,
                 maxCount: maxCount,
-                ref: options.ref,
                 reverse: options.reverse
             });
             const log = GitLogParser.parse(
@@ -1387,7 +1387,7 @@ export class GitService implements Disposable {
                 GitCommitType.Branch,
                 repoPath,
                 undefined,
-                options.ref,
+                ref,
                 await this.getCurrentUser(repoPath),
                 maxCount,
                 options.reverse!,
@@ -1395,7 +1395,7 @@ export class GitService implements Disposable {
             );
 
             if (log !== undefined) {
-                const opts = { ...options };
+                const opts = { ...options, ref: ref };
                 log.query = (maxCount: number | undefined) => this.getLog(repoPath, { ...opts, maxCount: maxCount });
             }
 
@@ -1590,12 +1590,16 @@ export class GitService implements Disposable {
     private async getLogForFileCore(
         repoPath: string | undefined,
         fileName: string,
-        options: { maxCount?: number; range?: Range; ref?: string; renames?: boolean; reverse?: boolean },
+        {
+            ref,
+            range,
+            ...options
+        }: { maxCount?: number; range?: Range; ref?: string; renames?: boolean; reverse?: boolean },
         document: TrackedDocument<GitDocumentState>,
         key: string,
         cc: LogCorrelationContext | undefined
     ): Promise<GitLog | undefined> {
-        if (!(await this.isTracked(fileName, repoPath, { ref: options.ref }))) {
+        if (!(await this.isTracked(fileName, repoPath, { ref: ref }))) {
             Logger.log(cc, `Skipping log; '${fileName}' is not tracked`);
             return GitService.emptyPromise as Promise<GitLog>;
         }
@@ -1603,16 +1607,14 @@ export class GitService implements Disposable {
         const [file, root] = Git.splitPath(fileName, repoPath, false);
 
         try {
-            // eslint-disable-next-line prefer-const
-            let { range, ...opts } = options;
             if (range !== undefined && range.start.line > range.end.line) {
                 range = new Range(range.end, range.start);
             }
 
             const maxCount = options.maxCount == null ? Container.config.advanced.maxListItems || 0 : options.maxCount;
 
-            const data = await Git.log_file(root, file, {
-                ...opts,
+            const data = await Git.log_file(root, file, ref, {
+                ...options,
                 maxCount: maxCount,
                 startLine: range === undefined ? undefined : range.start.line + 1,
                 endLine: range === undefined ? undefined : range.end.line + 1
@@ -1622,15 +1624,15 @@ export class GitService implements Disposable {
                 GitCommitType.File,
                 root,
                 file,
-                opts.ref,
+                ref,
                 await this.getCurrentUser(root),
                 maxCount,
-                opts.reverse!,
+                options.reverse!,
                 range
             );
 
             if (log !== undefined) {
-                const opts = { ...options };
+                const opts = { ...options, ref: ref, range: range };
                 log.query = (maxCount: number | undefined) =>
                     this.getLogForFile(repoPath, fileName, { ...opts, maxCount: maxCount });
             }
@@ -1639,7 +1641,7 @@ export class GitService implements Disposable {
         }
         catch (ex) {
             // Trap and cache expected log errors
-            if (document.state !== undefined && options.range === undefined && !options.reverse) {
+            if (document.state !== undefined && range === undefined && !options.reverse) {
                 const msg = ex && ex.toString();
                 Logger.debug(cc, `Cache replace (with empty promise): '${key}'`);
 
@@ -1693,6 +1695,184 @@ export class GitService implements Disposable {
     }
 
     @log()
+    async getNextDiffUris(
+        repoPath: string,
+        uri: Uri,
+        ref: string | undefined
+    ): Promise<{ current: GitUri; next: GitUri | undefined; deleted?: boolean } | undefined> {
+        // If we have no ref (or staged ref) there is no next commit
+        if (ref === undefined || ref.length === 0) return undefined;
+
+        const fileName = GitUri.getRelativePath(uri, repoPath);
+
+        if (Git.isStagedUncommitted(ref)) {
+            return {
+                current: GitUri.fromFile(fileName, repoPath, ref),
+                next: GitUri.fromFile(fileName, repoPath, undefined)
+            };
+        }
+
+        const next = await this.getNextUri(repoPath, uri, ref);
+        if (next === undefined) {
+            const status = await Container.git.getStatusForFile(repoPath, fileName);
+            if (status !== undefined) {
+                // If the file is staged, diff with the staged version
+                if (status.indexStatus !== undefined) {
+                    return {
+                        current: GitUri.fromFile(fileName, repoPath, ref),
+                        next: GitUri.fromFile(fileName, repoPath, GitService.stagedUncommittedSha)
+                    };
+                }
+            }
+
+            return {
+                current: GitUri.fromFile(fileName, repoPath, ref),
+                next: GitUri.fromFile(fileName, repoPath, undefined)
+            };
+        }
+
+        return {
+            current: GitUri.fromFile(fileName, repoPath, ref),
+            next: next
+        };
+    }
+
+    @log()
+    async getNextUri(
+        repoPath: string,
+        uri: Uri,
+        ref?: string,
+        skip: number = 0
+        // editorLine?: number
+    ): Promise<GitUri | undefined> {
+        // If we have no ref (or staged ref) there is no next commit
+        if (ref === undefined || ref.length === 0 || Git.isStagedUncommitted(ref)) return undefined;
+
+        let filters: GitLogDiffFilter[] | undefined;
+        if (ref === GitService.deletedOrMissingSha) {
+            // If we are trying to move next from a deleted or missing ref then get the first commit
+            ref = undefined;
+            filters = ['A'];
+        }
+
+        const fileName = GitUri.getRelativePath(uri, repoPath);
+        let data = await Git.log_file(repoPath, fileName, ref, {
+            filters: filters,
+            format: GitLogParser.simpleFormat,
+            maxCount: skip + 1,
+            // startLine: editorLine !== undefined ? editorLine + 1 : undefined,
+            reverse: true
+        });
+        if (data == null || data.length === 0) return undefined;
+
+        const [nextRef, file, status] = GitLogParser.parseSimple(data, skip);
+        // If the file was deleted, check for a possible rename
+        if (status === 'D') {
+            data = await Git.log_file(repoPath, '.', nextRef, {
+                filters: ['R'],
+                format: GitLogParser.simpleFormat,
+                maxCount: 1
+                // startLine: editorLine !== undefined ? editorLine + 1 : undefined
+            });
+            if (data == null || data.length === 0) {
+                return GitUri.fromFile(file || fileName, repoPath, nextRef);
+            }
+
+            const [nextRenamedRef, renamedFile] = GitLogParser.parseSimpleRenamed(data, file || fileName);
+            return GitUri.fromFile(
+                renamedFile || file || fileName,
+                repoPath,
+                nextRenamedRef || nextRef || GitService.deletedOrMissingSha
+            );
+        }
+
+        return GitUri.fromFile(file || fileName, repoPath, nextRef);
+    }
+
+    @log()
+    async getPreviousDiffUris(
+        repoPath: string,
+        uri: Uri,
+        ref: string | undefined,
+        skip: number = 0,
+        editorLine?: number
+    ): Promise<{ current: GitUri; previous: GitUri | undefined } | undefined> {
+        if (ref === GitService.deletedOrMissingSha) return undefined;
+
+        const fileName = GitUri.getRelativePath(uri, repoPath);
+
+        // If the ref is missing (i.e. working tree), check the file status to see if there is anything staged
+        if ((ref === undefined || ref.length === 0) && editorLine === undefined) {
+            const status = await Container.git.getStatusForFile(repoPath, fileName);
+            if (status !== undefined) {
+                // If the file is staged, diff with the staged version
+                if (status.indexStatus !== undefined) {
+                    if (skip === 0) {
+                        return {
+                            current: GitUri.fromFile(fileName, repoPath, ref),
+                            previous: GitUri.fromFile(fileName, repoPath, GitService.stagedUncommittedSha)
+                        };
+                    }
+
+                    return {
+                        current: GitUri.fromFile(fileName, repoPath, GitService.stagedUncommittedSha),
+                        previous: await this.getPreviousUri(repoPath, uri, ref, skip - 1, editorLine)
+                    };
+                }
+            }
+        }
+        else if (GitService.isStagedUncommitted(ref)) {
+            const current =
+                skip === 0
+                    ? GitUri.fromFile(fileName, repoPath, ref)
+                    : (await this.getPreviousUri(repoPath, uri, undefined, skip - 1, editorLine))!;
+            if (current.sha === GitService.deletedOrMissingSha) return undefined;
+
+            return {
+                current: current,
+                previous: await this.getPreviousUri(repoPath, uri, undefined, skip, editorLine)
+            };
+        }
+
+        const current =
+            skip === 0
+                ? GitUri.fromFile(fileName, repoPath, ref)
+                : (await this.getPreviousUri(repoPath, uri, ref, skip - 1, editorLine))!;
+        if (current.sha === GitService.deletedOrMissingSha) return undefined;
+
+        return {
+            current: current,
+            previous: await this.getPreviousUri(repoPath, uri, ref, skip, editorLine)
+        };
+    }
+
+    @log()
+    async getPreviousUri(
+        repoPath: string,
+        uri: Uri,
+        ref?: string,
+        skip: number = 0,
+        editorLine?: number
+    ): Promise<GitUri | undefined> {
+        if (ref === GitService.deletedOrMissingSha) return undefined;
+
+        if (ref !== undefined) {
+            skip++;
+        }
+
+        const fileName = GitUri.getRelativePath(uri, repoPath);
+        const data = await Git.log_file(repoPath, fileName, ref, {
+            format: GitLogParser.simpleFormat,
+            maxCount: skip + 1,
+            startLine: editorLine !== undefined ? editorLine + 1 : undefined
+        });
+        if (data == null || data.length === 0) throw new Error('File has no history');
+
+        const [previousRef, file] = GitLogParser.parseSimple(data, skip);
+        return GitUri.fromFile(file || fileName, repoPath, previousRef || GitService.deletedOrMissingSha);
+    }
+
+    @log()
     async getRemotes(repoPath: string | undefined, options: { includeAll?: boolean } = {}): Promise<GitRemote[]> {
         if (repoPath === undefined) return [];
 
@@ -2157,91 +2337,6 @@ export class GitService implements Disposable {
     }
 
     @log()
-    async getDiffWithPreviousForFile(
-        repoPath: string,
-        uri: Uri,
-        ref?: string,
-        skip: number = 0,
-        editorLine?: number
-    ): Promise<{ current: GitUri; previous: GitUri | undefined } | undefined> {
-        if (ref === GitService.deletedOrMissingSha) return undefined;
-
-        const fileName = GitUri.getRelativePath(uri, repoPath);
-
-        // If the ref is missing (i.e. working tree), check the file status to see if there is anything staged
-        if (ref === undefined && editorLine === undefined) {
-            const status = await Container.git.getStatusForFile(repoPath, fileName);
-            if (status !== undefined) {
-                // If the file is staged, diff with the staged version
-                if (status.indexStatus !== undefined) {
-                    if (skip === 0) {
-                        return {
-                            current: GitUri.fromFile(fileName, repoPath, ref),
-                            previous: GitUri.fromFile(fileName, repoPath, GitService.stagedUncommittedSha)
-                        };
-                    }
-
-                    return {
-                        current: GitUri.fromFile(fileName, repoPath, GitService.stagedUncommittedSha),
-                        previous: await this.getPreviousRevisionUri(repoPath, uri, ref, skip - 1, editorLine)
-                    };
-                }
-            }
-        }
-        else if (GitService.isStagedUncommitted(ref)) {
-            const current =
-                skip === 0
-                    ? GitUri.fromFile(fileName, repoPath, ref)
-                    : (await this.getPreviousRevisionUri(repoPath, uri, undefined, skip - 1, editorLine))!;
-            if (current.sha === GitService.deletedOrMissingSha) return undefined;
-
-            return {
-                current: current,
-                previous: await this.getPreviousRevisionUri(repoPath, uri, undefined, skip, editorLine)
-            };
-        }
-
-        const current =
-            skip === 0
-                ? GitUri.fromFile(fileName, repoPath, ref)
-                : (await this.getPreviousRevisionUri(repoPath, uri, ref, skip - 1, editorLine))!;
-        if (current.sha === GitService.deletedOrMissingSha) return undefined;
-
-        return {
-            current: current,
-            previous: await this.getPreviousRevisionUri(repoPath, uri, ref, skip, editorLine)
-        };
-    }
-
-    @log()
-    async getPreviousRevisionUri(
-        repoPath: string,
-        uri: Uri,
-        ref?: string,
-        skip: number = 0,
-        editorLine?: number
-    ): Promise<GitUri | undefined> {
-        if (ref === GitService.deletedOrMissingSha) return undefined;
-
-        if (ref !== undefined) {
-            skip++;
-        }
-
-        const fileName = GitUri.getRelativePath(uri, repoPath);
-        const data = await Git.log_file_simple(
-            repoPath,
-            fileName,
-            ref,
-            skip + 1,
-            editorLine !== undefined ? editorLine + 1 : undefined
-        );
-        if (data == null || data.length === 0) throw new Error('File has no history');
-
-        const [previousRef, file] = GitLogParser.parseSimple(data, skip);
-        return GitUri.fromFile(file || fileName, repoPath, previousRef || GitService.deletedOrMissingSha);
-    }
-
-    @log()
     async resolveReference(repoPath: string, ref: string, uri?: Uri) {
         const resolved = Git.isSha(ref) || !Git.isShaLike(ref) || ref.endsWith('^3');
         if (uri == null) return resolved ? ref : (await Git.revparse(repoPath, ref)) || ref;

src/git/parsers/logParser.ts

@@ -4,9 +4,20 @@ import { Range } from 'vscode';
 import { Arrays, Strings } from '../../system';
 import { Git, GitAuthor, GitCommitType, GitFile, GitFileStatus, GitLog, GitLogCommit } from '../git';
 
+const emptyEntry: LogEntry = {};
 const emptyStr = '';
 const slash = '/';
 
+const diffRegex = /diff --git a\/(.*) b\/(.*)/;
+const fileStatusRegex = /(\S)\S*\t([^\t\n]+)(?:\t(.+))?/;
+const logFileSimpleRegex = /^<r> (.*)\s*(?:(?:diff --git a\/(.*) b\/(.*))|(?:(\S)\S*\t([^\t\n]+)(?:\t(.+))?))/gm;
+
+// Using %x00 codes because some shells seem to try to expand things if not
+const lb = '%x3c'; // `%x${'<'.charCodeAt(0).toString(16)}`;
+const rb = '%x3e'; // `%x${'>'.charCodeAt(0).toString(16)}`;
+const sl = '%x2f'; // `%x${'/'.charCodeAt(0).toString(16)}`;
+const sp = '%x20'; // `%x${' '.charCodeAt(0).toString(16)}`;
+
 interface LogEntry {
     ref?: string;
 
@@ -26,13 +37,23 @@ interface LogEntry {
     summary?: string;
 }
 
-const diffRegex = /diff --git a\/(.*) b\/(.*)/;
-const fileStatusRegex = /(\S)\S*\t([^\t\n]+)(?:\t(.+))?/;
-const logFileSimpleRegex = /^<r> (.*)\s*(?:(?:diff --git a\/(.*) b\/(.*))|(?:\S+\t([^\t\n]+)(?:\t(.+))?))/gm;
-
-const emptyEntry: LogEntry = {};
-
 export class GitLogParser {
+    static defaultFormat = [
+        `${lb}${sl}f${rb}`,
+        `${lb}r${rb}${sp}%H`, // ref
+        `${lb}a${rb}${sp}%aN`, // author
+        `${lb}e${rb}${sp}%aE`, // email
+        `${lb}d${rb}${sp}%at`, // date
+        `${lb}c${rb}${sp}%ct`, // committed date
+        `${lb}p${rb}${sp}%P`, // parents
+        `${lb}s${rb}`,
+        '%B', // summary
+        `${lb}${sl}s${rb}`,
+        `${lb}f${rb}`
+    ].join('%n');
+
+    static simpleFormat = `${lb}r${rb}${sp}%H`;
+
     static parse(
         data: string,
         type: GitCommitType,
@@ -372,10 +393,14 @@ export class GitLogParser {
         }
     }
 
-    static parseSimple(data: string, skip: number): [string | undefined, string | undefined] {
+    static parseSimple(
+        data: string,
+        skip: number
+    ): [string | undefined, string | undefined, GitFileStatus | undefined] {
         let match;
         let ref;
         let file;
+        let status: GitFileStatus | undefined;
         do {
             match = logFileSimpleRegex.exec(data);
             if (match == null) break;
@@ -383,12 +408,38 @@ export class GitLogParser {
             if (skip-- > 0) continue;
 
             ref = ` ${match[1]}`.substr(1);
-            file = ` ${match[3] || match[2] || match[5] || match[4]}`.substr(1);
+            file = ` ${match[3] || match[2] || match[6] || match[5]}`.substr(1);
+            status = match[4] ? (` ${match[4]}`.substr(1) as GitFileStatus) : undefined;
         } while (skip >= 0);
 
         // Ensure the regex state is reset
         logFileSimpleRegex.lastIndex = 0;
 
-        return [ref, file];
+        return [ref, file, status];
+    }
+
+    static parseSimpleRenamed(
+        data: string,
+        originalFileName: string
+    ): [string | undefined, string | undefined, GitFileStatus | undefined] {
+        let match;
+        let ref;
+        let file;
+        let status: GitFileStatus | undefined;
+        do {
+            match = logFileSimpleRegex.exec(data);
+            if (match == null) break;
+
+            if (originalFileName !== (match[2] || match[5])) continue;
+
+            ref = ` ${match[1]}`.substr(1);
+            file = ` ${match[3] || match[2] || match[6] || match[5]}`.substr(1);
+            status = match[4] ? (` ${match[4]}`.substr(1) as GitFileStatus) : undefined;
+        } while (match != null);
+
+        // Ensure the regex state is reset
+        logFileSimpleRegex.lastIndex = 0;
+
+        return [ref, file, status];
     }
 }

src/git/parsers/stashParser.ts

@@ -3,7 +3,14 @@ import { Arrays, Strings } from '../../system';
 import { GitCommitType, GitFile, GitFileStatus, GitLogParser, GitStash, GitStashCommit } from '../git';
 // import { Logger } from './logger';
 
+// Using %x00 codes because some shells seem to try to expand things if not
+const lb = '%x3c'; // `%x${'<'.charCodeAt(0).toString(16)}`;
+const rb = '%x3e'; // `%x${'>'.charCodeAt(0).toString(16)}`;
+const sl = '%x2f'; // `%x${'/'.charCodeAt(0).toString(16)}`;
+const sp = '%x20'; // `%x${' '.charCodeAt(0).toString(16)}`;
+
 const emptyStr = '';
+const emptyEntry: StashEntry = {};
 
 interface StashEntry {
     ref?: string;
@@ -15,9 +22,19 @@ interface StashEntry {
     stashName?: string;
 }
 
-const emptyEntry: StashEntry = {};
-
 export class GitStashParser {
+    static defaultFormat = [
+        `${lb}${sl}f${rb}`,
+        `${lb}r${rb}${sp}%H`, // ref
+        `${lb}d${rb}${sp}%at`, // date
+        `${lb}c${rb}${sp}%ct`, // committed date
+        `${lb}l${rb}${sp}%gd`, // reflog-selector
+        `${lb}s${rb}`,
+        '%B', // summary
+        `${lb}${sl}s${rb}`,
+        `${lb}f${rb}`
+    ].join('%n');
+
     static parse(data: string, repoPath: string): GitStash | undefined {
         if (!data) return undefined;
 

src/trackers/trackedDocument.ts

@@ -2,7 +2,7 @@
 import { Disposable, Event, EventEmitter, TextDocument, TextEditor, Uri } from 'vscode';
 import { CommandContext, getEditorIfActive, isActiveDocument, setCommandContext } from '../constants';
 import { Container } from '../container';
-import { GitUri, Repository, RepositoryChange, RepositoryChangeEvent } from '../git/gitService';
+import { GitService, GitUri, Repository, RepositoryChange, RepositoryChangeEvent } from '../git/gitService';
 import { Logger } from '../logger';
 import { Functions } from '../system';
 
@@ -100,7 +100,9 @@ export class TrackedDocument implements Disposable {
     }
 
     get isRevision() {
-        return this._uri !== undefined ? Boolean(this._uri.sha) : false;
+        return this._uri !== undefined
+            ? Boolean(this._uri.sha) && this._uri.sha !== GitService.deletedOrMissingSha
+            : false;
     }
 
     private _isTracked: boolean = false;