Fixes #155 - prev/next navigation works in diff editors

Also adds icons for prev/next and places them into the editor toolbar

CHANGELOG.md

@@ -7,8 +7,11 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
 ## [Unreleased]
 ### Added
 - Adds a tree layout option to tags in the *GitLens* explorer — closes [#358](https://github.com/eamodio/vscode-gitlens/issues/358)
+- Adds an icon for the *Compare File with Previous Revision* command (`gitlens.diffWithPrevious`) and moves it into the editor toolbar
+- Adds an icon for the *Compare File with Next Revision* command (`gitlens.diffWithNext`) and moves it into the editor toolbar
 
 ### Fixed
+- Fixes [#155](https://github.com/eamodio/vscode-gitlens/issues/155) - Navigating file diffs with `alt+,` gets stuck
 - Fixes [#359](https://github.com/eamodio/vscode-gitlens/issues/359) - Show changes of an added file in the first commit
 - Fixes issue where comparing previous revision during a merge/rebase conflict failed to show the correct contents
 - Fixes various issues when not on a branch

images/dark/icon-next-commit.svg

@@ -0,0 +1,3 @@
+<svg width="16" height="22" xmlns="http://www.w3.org/2000/svg">
+  <path fill="#C5C5C5" d="M1.66 10a4 4 0 0 1 3.86-3 4 4 0 0 1 3.86 3h3.14v2H9.38a4 4 0 0 1-3.86 3 4 4 0 0 1-3.86-3H.28v-2h1.38zm3.86 3.2a2.2 2.2 0 0 0 2.2-2.2 2.2 2.2 0 0 0-2.2-2.2 2.2 2.2 0 0 0-2.2 2.2c0 1.22.98 2.2 2.2 2.2zm4.5-3.2h1.7l-3-3h3l4 4-4 4h-3l3-3H10l.02-2z"/>
+</svg>

images/dark/icon-prev-commit.svg

@@ -0,0 +1,3 @@
+<svg width="16" height="22" xmlns="http://www.w3.org/2000/svg">
+  <path fill="#C5C5C5" d="M14.34 10a4 4 0 0 0-3.86-3 4 4 0 0 0-3.86 3H3.48v2h3.14a4 4 0 0 0 3.86 3 4 4 0 0 0 3.86-3h1.38v-2h-1.38zm-3.86 3.2a2.2 2.2 0 0 1-2.2-2.2c0-1.22.98-2.2 2.2-2.2 1.22 0 2.2.98 2.2 2.2a2.2 2.2 0 0 1-2.2 2.2zM5.98 10h-1.7l3-3h-3l-4 4 4 4h3l-3-3H6l-.02-2z"/>
+</svg>

images/light/icon-next-commit.svg

@@ -0,0 +1,3 @@
+<svg width="16" height="22" xmlns="http://www.w3.org/2000/svg">
+  <path fill="#424242" d="M1.66 10a4 4 0 0 1 3.86-3 4 4 0 0 1 3.86 3h3.14v2H9.38a4 4 0 0 1-3.86 3 4 4 0 0 1-3.86-3H.28v-2h1.38zm3.86 3.2a2.2 2.2 0 0 0 2.2-2.2 2.2 2.2 0 0 0-2.2-2.2 2.2 2.2 0 0 0-2.2 2.2c0 1.22.98 2.2 2.2 2.2zm4.5-3.2h1.7l-3-3h3l4 4-4 4h-3l3-3H10l.02-2z"/>
+</svg>

images/light/icon-prev-commit.svg

@@ -0,0 +1,3 @@
+<svg width="16" height="22" xmlns="http://www.w3.org/2000/svg">
+  <path fill="#424242" d="M14.34 10a4 4 0 0 0-3.86-3 4 4 0 0 0-3.86 3H3.48v2h3.14a4 4 0 0 0 3.86 3 4 4 0 0 0 3.86-3h1.38v-2h-1.38zm-3.86 3.2a2.2 2.2 0 0 1-2.2-2.2c0-1.22.98-2.2 2.2-2.2 1.22 0 2.2.98 2.2 2.2a2.2 2.2 0 0 1-2.2 2.2zM5.98 10h-1.7l3-3h-3l-4 4 4 4h3l-3-3H6l-.02-2z"/>
+</svg>

package.json

@@ -1191,12 +1191,29 @@
             {
                 "command": "gitlens.diffWithNext",
                 "title": "Compare File with Next Revision",
-                "category": "GitLens"
+                "category": "GitLens",
+                "icon": {
+                    "dark": "images/dark/icon-next-commit.svg",
+                    "light": "images/light/icon-next-commit.svg"
+                }
             },
             {
                 "command": "gitlens.diffWithPrevious",
                 "title": "Compare File with Previous Revision",
-                "category": "GitLens"
+                "category": "GitLens",
+                "icon": {
+                    "dark": "images/dark/icon-prev-commit.svg",
+                    "light": "images/light/icon-prev-commit.svg"
+                }
+            },
+            {
+                "command": "gitlens.diffWithPreviousInDiff",
+                "title": "Compare File with Previous Revision",
+                "category": "GitLens",
+                "icon": {
+                    "dark": "images/dark/icon-prev-commit.svg",
+                    "light": "images/light/icon-prev-commit.svg"
+                }
             },
             {
                 "command": "gitlens.diffLineWithPrevious",
@@ -1797,11 +1814,15 @@
                 },
                 {
                     "command": "gitlens.diffWithNext",
-                    "when": "gitlens:activeIsTracked"
+                    "when": "gitlens:activeIsTracked && gitlens:activeIsRevision"
                 },
                 {
                     "command": "gitlens.diffWithPrevious",
-                    "when": "gitlens:activeIsTracked"
+                    "when": "!isInDiffEditor && gitlens:activeIsTracked"
+                },
+                {
+                    "command": "gitlens.diffWithPreviousInDiff",
+                    "when": "isInDiffEditor && gitlens:activeIsTracked"
                 },
                 {
                     "command": "gitlens.diffLineWithPrevious",
@@ -2260,6 +2281,21 @@
                     "group": "navigation@1"
                 },
                 {
+                    "command": "gitlens.diffWithPrevious",
+                    "when": "editorTextFocus && !isInDiffEditor && gitlens:activeIsTracked && config.gitlens.advanced.menus.editorTitle.fileDiff",
+                    "group": "navigation@98"
+                },
+                {
+                    "command": "gitlens.diffWithPreviousInDiff",
+                    "when": "isInDiffEditor && gitlens:activeIsTracked && config.gitlens.advanced.menus.editorTitle.fileDiff",
+                    "group": "navigation@98"
+                },
+                {
+                    "command": "gitlens.diffWithNext",
+                    "when": "editorTextFocus && gitlens:activeIsTracked && gitlens:activeIsTracked && gitlens:activeIsRevision && config.gitlens.advanced.menus.editorTitle.fileDiff",
+                    "group": "navigation@99"
+                },
+                {
                     "command": "gitlens.toggleFileBlame",
                     "alt": "gitlens.toggleFileRecentChanges",
                     "when": "gitlens:activeIsBlameable && !gitlens:annotationStatus && config.gitlens.advanced.menus.editorTitle.blame",
@@ -2923,7 +2959,12 @@
             {
                 "command": "gitlens.diffWithPrevious",
                 "key": "alt+,",
-                "when": "gitlens:keymap == alternate && editorTextFocus && gitlens:activeIsTracked"
+                "when": "gitlens:keymap == alternate && editorTextFocus && !isInDiffEditor && gitlens:activeIsTracked"
+            },
+            {
+                "command": "gitlens.diffWithPreviousInDiff",
+                "key": "alt+,",
+                "when": "gitlens:keymap == alternate && isInDiffEditor && gitlens:activeIsTracked"
             },
             {
                 "command": "gitlens.diffLineWithWorking",
@@ -2999,7 +3040,12 @@
                 "command": "gitlens.diffWithPrevious",
                 "key": "ctrl+shift+g ,",
                 "mac": "cmd+alt+g ,",
-                "when": "gitlens:keymap == chorded && editorTextFocus && gitlens:activeIsTracked"
+                "when": "gitlens:keymap == chorded && editorTextFocus && !isInDiffEditor && gitlens:activeIsTracked"
+            },
+            {
+                "command": "gitlens.diffWithPreviousInDiff",
+                "key": "alt+,",
+                "when": "gitlens:keymap == chorded && isInDiffEditor && gitlens:activeIsTracked"
             },
             {
                 "command": "gitlens.diffLineWithWorking",

src/commands/common.ts

@@ -22,6 +22,7 @@ export enum Commands {
     DiffWithBranch = 'gitlens.diffWithBranch',
     DiffWithNext = 'gitlens.diffWithNext',
     DiffWithPrevious = 'gitlens.diffWithPrevious',
+    DiffWithPreviousInDiff = 'gitlens.diffWithPreviousInDiff',
     DiffLineWithPrevious = 'gitlens.diffLineWithPrevious',
     DiffWithRevision = 'gitlens.diffWithRevision',
     DiffWithWorking = 'gitlens.diffWithWorking',

src/commands/diffWithNext.ts

@@ -38,8 +38,22 @@ export class DiffWithNextCommand extends ActiveEditorCommand {
             try {
                 const sha = args.commit === undefined ? gitUri.sha : args.commit.sha;
 
-                // If we are a fake "staged" sha, treat it as a DiffWithWorking
-                if (GitService.isStagedUncommitted(sha!)) return commands.executeCommand(Commands.DiffWithWorking, uri);
+                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);
+                }
 
                 const log = await Container.git.getLogForFile(gitUri.repoPath, gitUri.fsPath, { maxCount: sha !== undefined ? undefined : 2, range: args.range!, renames: true });
                 if (log === undefined) return Messages.showFileNotUnderSourceControlWarningMessage('Unable to open compare');

src/commands/diffWithPrevious.ts

@@ -1,118 +1,135 @@
-'use strict';
-import { Iterables } from '../system';
-import { commands, TextDocumentShowOptions, TextEditor, Uri, window } from 'vscode';
-import { ActiveEditorCommand, Commands, getCommandUri } from './common';
-import { Container } from '../container';
-import { DiffWithCommandArgs } from './diffWith';
-import { DiffWithWorkingCommandArgs } from './diffWithWorking';
-import { GitCommit, GitService, GitUri } from '../gitService';
-import { Logger } from '../logger';
-import { Messages } from '../messages';
-
-export interface DiffWithPreviousCommandArgs {
-    commit?: GitCommit;
-
-    line?: number;
-    showOptions?: TextDocumentShowOptions;
-}
-
-export class DiffWithPreviousCommand extends ActiveEditorCommand {
-
-    constructor() {
-        super(Commands.DiffWithPrevious);
-    }
-
-    async execute(editor?: TextEditor, uri?: Uri, args: DiffWithPreviousCommandArgs = {}): Promise<any> {
-        uri = getCommandUri(uri, editor);
-        if (uri === undefined) return undefined;
-
-        args = { ...args };
-        if (args.line === undefined) {
-            args.line = editor === undefined ? 0 : editor.selection.active.line;
-        }
-
-        if (args.commit === undefined || !args.commit.isFile) {
-            const gitUri = await GitUri.fromUri(uri);
-
-            try {
-                let sha = args.commit === undefined ? gitUri.sha : args.commit.sha;
-                if (sha === GitService.deletedSha) return Messages.showCommitHasNoPreviousCommitWarningMessage();
-
-                // If we are a fake "staged" sha, remove it
-                let isStagedUncommitted = false;
-                if (GitService.isStagedUncommitted(sha!)) {
-                    gitUri.sha = sha = undefined;
-                    isStagedUncommitted = true;
-                }
-
-                const log = await Container.git.getLogForFile(gitUri.repoPath, gitUri.fsPath, { maxCount: 2, ref: sha, 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 and the file is uncommitted, then treat it as a DiffWithWorking
-                if (gitUri.sha === undefined) {
-                    const status = await Container.git.getStatusForFile(gitUri.repoPath!, gitUri.fsPath);
-                    if (status !== undefined) {
-                        if (isStagedUncommitted) {
-                            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);
-                        }
-
-                        // Check if the file is staged
-                        if (status.indexStatus !== undefined) {
-                            const diffArgs: DiffWithCommandArgs = {
-                                repoPath: args.commit.repoPath,
-                                lhs: {
-                                    sha: GitService.stagedUncommittedSha,
-                                    uri: args.commit.uri
-                                },
-                                rhs: {
-                                    sha: '',
-                                    uri: args.commit.uri
-                                },
-                                line: args.line,
-                                showOptions: args.showOptions
-                            };
-
-                            return commands.executeCommand(Commands.DiffWith, diffArgs);
-                        }
-
-                        return commands.executeCommand(Commands.DiffWithWorking, uri, { commit: args.commit, showOptions: args.showOptions } as DiffWithWorkingCommandArgs);
-                    }
-                }
-            }
-            catch (ex) {
-                Logger.error(ex, 'DiffWithPreviousCommand', `getLogForFile(${gitUri.repoPath}, ${gitUri.fsPath})`);
-                return window.showErrorMessage(`Unable to open compare. See output channel for more details`);
-            }
-        }
-
-        const diffArgs: DiffWithCommandArgs = {
-            repoPath: args.commit.repoPath,
-            lhs: {
-                sha: args.commit.previousSha !== undefined ? args.commit.previousSha : GitService.deletedSha,
-                uri: args.commit.previousUri
-            },
-            rhs: {
-                sha: args.commit.sha,
-                uri: args.commit.uri
-            },
-            line: args.line,
-            showOptions: args.showOptions
-        };
-        return commands.executeCommand(Commands.DiffWith, diffArgs);
-    }
+'use strict';
+import { Iterables } from '../system';
+import { commands, TextDocumentShowOptions, TextEditor, Uri, window } from 'vscode';
+import { ActiveEditorCommand, CommandContext, Commands, getCommandUri } from './common';
+import { Container } from '../container';
+import { DiffWithCommandArgs } from './diffWith';
+import { DiffWithWorkingCommandArgs } from './diffWithWorking';
+import { GitCommit, GitService, GitUri } from '../gitService';
+import { Logger } from '../logger';
+import { Messages } from '../messages';
+
+export interface DiffWithPreviousCommandArgs {
+    commit?: GitCommit;
+
+    inDiffEditor?: boolean;
+    line?: number;
+    showOptions?: TextDocumentShowOptions;
+}
+
+export class DiffWithPreviousCommand extends ActiveEditorCommand {
+
+    constructor() {
+        super([Commands.DiffWithPrevious, Commands.DiffWithPreviousInDiff]);
+    }
+
+    protected async preExecute(context: CommandContext, args: DiffWithPreviousCommandArgs = {}): Promise<any> {
+        if (context.command === Commands.DiffWithPreviousInDiff) {
+            args.inDiffEditor = true;
+        }
+
+        return this.execute(context.editor, context.uri, args);
+    }
+
+    async execute(editor?: TextEditor, uri?: Uri, args: DiffWithPreviousCommandArgs = {}): Promise<any> {
+        uri = getCommandUri(uri, editor);
+        if (uri === undefined) return undefined;
+
+        args = { ...args };
+        if (args.line === undefined) {
+            args.line = editor === undefined ? 0 : editor.selection.active.line;
+        }
+
+        if (args.commit === undefined || !args.commit.isFile) {
+            const gitUri = await GitUri.fromUri(uri);
+
+            try {
+                let sha = args.commit === undefined ? gitUri.sha : args.commit.sha;
+                if (sha === GitService.deletedSha) return Messages.showCommitHasNoPreviousCommitWarningMessage();
+
+                // If we are a fake "staged" sha, remove it
+                let isStagedUncommitted = false;
+                if (GitService.isStagedUncommitted(sha!)) {
+                    gitUri.sha = sha = undefined;
+                    isStagedUncommitted = true;
+                }
+
+                // If we are in a diff editor, assume we are on the right side, and need to move back 2 revisions
+                if (args.inDiffEditor && sha !== undefined) {
+                    sha = sha + '^';
+                }
+
+                const log = await Container.git.getLogForFile(gitUri.repoPath, gitUri.fsPath, { maxCount: 2, ref: sha, 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 (i.e. working tree), check the file status
+                // If file is uncommitted, then treat it as a DiffWithWorking
+                if (gitUri.sha === undefined) {
+                    const status = await Container.git.getStatusForFile(gitUri.repoPath!, gitUri.fsPath);
+                    if (status !== undefined) {
+                        if (isStagedUncommitted) {
+                            const diffArgs: DiffWithCommandArgs = {
+                                repoPath: args.commit.repoPath,
+                                lhs: {
+                                    sha: args.inDiffEditor ? args.commit.previousSha || GitService.deletedSha : args.commit.sha,
+                                    uri: args.inDiffEditor ? args.commit.previousUri : args.commit.uri
+                                },
+                                rhs: {
+                                    sha: args.inDiffEditor ? args.commit.sha : GitService.stagedUncommittedSha,
+                                    uri: args.commit.uri
+                                },
+                                line: args.line,
+                                showOptions: args.showOptions
+                            };
+                            return commands.executeCommand(Commands.DiffWith, diffArgs);
+                        }
+
+                        // Check if the file is staged
+                        if (status.indexStatus !== undefined) {
+                            const diffArgs: DiffWithCommandArgs = {
+                                repoPath: args.commit.repoPath,
+                                lhs: {
+                                    sha: args.inDiffEditor ? args.commit.sha : GitService.stagedUncommittedSha,
+                                    uri: args.commit.uri
+                                },
+                                rhs: {
+                                    sha: args.inDiffEditor ? GitService.stagedUncommittedSha : '',
+                                    uri: args.commit.uri
+                                },
+                                line: args.line,
+                                showOptions: args.showOptions
+                            };
+
+                            return commands.executeCommand(Commands.DiffWith, diffArgs);
+                        }
+
+                        if (!args.inDiffEditor) {
+                            return commands.executeCommand(Commands.DiffWithWorking, uri, { commit: args.commit, showOptions: args.showOptions } as DiffWithWorkingCommandArgs);
+                        }
+                    }
+                }
+            }
+            catch (ex) {
+                Logger.error(ex, 'DiffWithPreviousCommand', `getLogForFile(${gitUri.repoPath}, ${gitUri.fsPath})`);
+                return window.showErrorMessage(`Unable to open compare. See output channel for more details`);
+            }
+        }
+
+        const diffArgs: DiffWithCommandArgs = {
+            repoPath: args.commit.repoPath,
+            lhs: {
+                sha: args.commit.previousSha !== undefined ? args.commit.previousSha : GitService.deletedSha,
+                uri: args.commit.previousUri
+            },
+            rhs: {
+                sha: args.commit.sha,
+                uri: args.commit.uri
+            },
+            line: args.line,
+            showOptions: args.showOptions
+        };
+        return commands.executeCommand(Commands.DiffWith, diffArgs);
+    }
 }