Closes #195 - use diff.guitool

gitkraken · Nov 10, 2017 · 95f590f · 95f590f
1 parent 68d7282
commit 95f590f
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## [Unreleased]
+### Changed
+- Changes to use `diff.guitool` first if available, before falling back to `diff.tool` -- closes [#195](https://github.com/eamodio/vscode-gitlens/issues/195)
+
+### Fixed
+- Fixes issue where failed git commands would get stuck in the pending queue causing future similar commands to also fail
+
 ## [6.0.0] - 2017-11-08
 
 ### Added

diff --git a/src/commands/diffDirectory.ts b/src/commands/diffDirectory.ts
@@ -32,14 +32,6 @@ export class DiffDirectoryCommand extends ActiveEditorCommand {
     }
 
     async execute(editor?: TextEditor, uri?: Uri, args: DiffDirectoryCommandCommandArgs = {}): Promise<any> {
-        const diffTool = await this.git.getConfig('diff.tool');
-        if (!diffTool) {
-            const result = await window.showWarningMessage(`Unable to open directory compare because there is no Git diff tool configured`, 'View Git Docs');
-            if (!result) return undefined;
-
-            return commands.executeCommand(BuiltInCommands.Open, Uri.parse('https://git-scm.com/docs/git-config#git-config-difftool'));
-        }
-
         uri = getCommandUri(uri, editor);
 
         try {
@@ -66,6 +58,14 @@ export class DiffDirectoryCommand extends ActiveEditorCommand {
             return undefined;
         }
         catch (ex) {
+            const msg = ex && ex.toString();
+            if (msg === 'No diff tool found') {
+                const result = await window.showWarningMessage(`Unable to open directory compare because there is no Git diff tool configured`, 'View Git Docs');
+                if (!result) return undefined;
+
+                return commands.executeCommand(BuiltInCommands.Open, Uri.parse('https://git-scm.com/docs/git-config#git-config-difftool'));
+            }
+
             Logger.error(ex, 'DiffDirectoryCommand');
             return window.showErrorMessage(`Unable to open directory compare. See output channel for more details`);
         }

diff --git a/src/commands/externalDiff.ts b/src/commands/externalDiff.ts
@@ -86,17 +86,17 @@ export class ExternalDiffCommand extends Command {
 
     async execute(args: ExternalDiffCommandArgs = {}) {
         try {
-            const diffTool = await this.git.getConfig('diff.tool');
-            if (!diffTool) {
+            const repoPath = await this.git.getRepoPath(undefined);
+            if (!repoPath) return Messages.showNoRepositoryWarningMessage(`Unable to open changed files`);
+
+            const tool = await this.git.getDiffTool(repoPath);
+            if (tool === undefined) {
                 const result = await window.showWarningMessage(`Unable to open file compare because there is no Git diff tool configured`, 'View Git Docs');
                 if (!result) return undefined;
 
                 return commands.executeCommand(BuiltInCommands.Open, Uri.parse('https://git-scm.com/docs/git-config#git-config-difftool'));
             }
 
-            const repoPath = await this.git.getRepoPath(undefined);
-            if (!repoPath) return Messages.showNoRepositoryWarningMessage(`Unable to open changed files`);
-
             if (args.files === undefined) {
                 const status = await this.git.getStatusForRepo(repoPath);
                 if (status === undefined) return window.showWarningMessage(`Unable to open changed files`);
@@ -115,7 +115,7 @@ export class ExternalDiffCommand extends Command {
             }
 
             for (const file of args.files) {
-                this.git.openDiffTool(repoPath, file.uri, file.staged);
+                this.git.openDiffTool(repoPath, file.uri, file.staged, tool);
             }
 
             return undefined;

diff --git a/src/git/git.ts b/src/git/git.ts
@@ -244,7 +244,7 @@ export class Git {
             return data.trim();
         }
         catch {
-            return '';
+            return undefined;
         }
     }
 
@@ -283,17 +283,17 @@ export class Git {
         return gitCommand({ cwd: repoPath }, ...params);
     }
 
-    static difftool_dirDiff(repoPath: string, sha1: string, sha2?: string) {
-        const params = [`difftool`, `--dir-diff`, sha1];
+    static difftool_dirDiff(repoPath: string, tool: string, sha1: string, sha2?: string) {
+        const params = [`difftool`, `--dir-diff`, `--tool=${tool}`, sha1];
         if (sha2) {
             params.push(sha2);
         }
 
         return gitCommand({ cwd: repoPath }, ...params);
     }
 
-    static difftool_fileDiff(repoPath: string, fileName: string, staged: boolean) {
-        const params = [`difftool`, `--no-prompt`];
+    static difftool_fileDiff(repoPath: string, fileName: string, tool: string, staged: boolean) {
+        const params = [`difftool`, `--no-prompt`, `--tool=${tool}`];
         if (staged) {
             params.push('--staged');
         }

diff --git a/src/gitService.ts b/src/gitService.ts
@@ -576,7 +576,7 @@ export class GitService extends Disposable {
         return GitDiffParser.parseShortStat(data);
     }
 
-    async getConfig(key: string, repoPath?: string): Promise<string> {
+    async getConfig(key: string, repoPath?: string): Promise<string | undefined> {
         Logger.log(`getConfig('${key}', '${repoPath}')`);
 
         return await Git.config_get(key, repoPath);
@@ -1086,16 +1086,30 @@ export class GitService extends Disposable {
         return tracked;
     }
 
-    openDiffTool(repoPath: string, uri: Uri, staged: boolean) {
-        Logger.log(`openDiffTool('${repoPath}', '${uri.fsPath}', ${staged})`);
+    async getDiffTool(repoPath?: string) {
+        return await Git.config_get('diff.guitool', repoPath) || await Git.config_get('diff.tool', repoPath);
+    }
+
+    async openDiffTool(repoPath: string, uri: Uri, staged: boolean, tool?: string) {
+        if (!tool) {
+            tool = await this.getDiffTool(repoPath);
+            if (tool === undefined) throw new Error('No diff tool found');
+        }
 
-        return Git.difftool_fileDiff(repoPath, uri.fsPath, staged);
+        Logger.log(`openDiffTool('${repoPath}', '${uri.fsPath}', ${staged}, '${tool}')`);
+
+        return Git.difftool_fileDiff(repoPath, uri.fsPath, tool, staged);
     }
 
-    openDirectoryDiff(repoPath: string, sha1: string, sha2?: string) {
-        Logger.log(`openDirectoryDiff('${repoPath}', '${sha1}', '${sha2}')`);
+    async openDirectoryDiff(repoPath: string, sha1: string, sha2?: string, tool?: string) {
+        if (!tool) {
+            tool = await this.getDiffTool(repoPath);
+            if (tool === undefined) throw new Error('No diff tool found');
+        }
+
+        Logger.log(`openDirectoryDiff('${repoPath}', '${sha1}', '${sha2}', '${tool}')`);
 
-        return Git.difftool_dirDiff(repoPath, sha1, sha2);
+        return Git.difftool_dirDiff(repoPath, tool, sha1, sha2);
     }
 
     stopWatchingFileSystem() {