Fixes #797 - diff range was incorrect

gitkraken · Jul 23, 2019 · d25fee3 · d25fee3
1 parent 7f86802
commit d25fee3
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 24 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,12 @@ 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]
+
+### Fixed
+
+- Fixes [#797](https://github.com/eamodio/vscode-gitlens/issues/797) - Branch diff against master shows incorrect files in two-dot mode
+
 ## [9.9.0] - 2019-07-21
 
 ### Added

diff --git a/src/views/nodes/compareBranchNode.ts b/src/views/nodes/compareBranchNode.ts
@@ -8,7 +8,7 @@ import { CommandQuickPickItem, ReferencesQuickPick } from '../../quickpicks';
 import { CommitsQueryResults, ResultsCommitsNode } from './resultsCommitsNode';
 import { Container } from '../../container';
 import { log, Strings } from '../../system';
-import { ResultsFilesNode } from './resultsFilesNode';
+import { FilesQueryResults, ResultsFilesNode } from './resultsFilesNode';
 import { ViewShowBranchComparison } from '../../config';
 
 export class CompareBranchNode extends ViewNode<RepositoriesView> {
@@ -56,8 +56,9 @@ export class CompareBranchNode extends ViewNode<RepositoriesView> {
                     this.view,
                     this,
                     this.uri.repoPath!,
-                    this._compareWith.ref,
-                    this.compareWithWorkingTree ? '' : this.branch.ref
+                    (this._compareWith && this._compareWith.ref) || 'HEAD',
+                    this.compareWithWorkingTree ? '' : this.branch.ref,
+                    this.getFilesQuery.bind(this)
                 )
             ];
         }
@@ -140,6 +141,13 @@ export class CompareBranchNode extends ViewNode<RepositoriesView> {
         );
     }
 
+    private get diffComparisonNotation() {
+        // In git diff the range syntax doesn't mean the same thing as with git log -- since git diff is about comparing endpoints not ranges
+        // see https://git-scm.com/docs/git-diff#Documentation/git-diff.txt-emgitdiffemltoptionsgtltcommitgtltcommitgt--ltpathgt82308203
+        // So inverting the range syntax should be about equivalent for the behavior we want
+        return this.comparisonNotation === '...' ? '..' : '...';
+    }
+
     private get comparisonType() {
         return (
             (this._compareWith && this._compareWith.type) ||
@@ -180,14 +188,26 @@ export class CompareBranchNode extends ViewNode<RepositoriesView> {
         const count = log !== undefined ? log.count : 0;
         const truncated = log !== undefined ? log.truncated : false;
 
-        const label = Strings.pluralize('commit', count, { number: truncated ? `${count}+` : undefined, zero: 'No' });
-
         return {
-            label: label,
+            label: Strings.pluralize('commit', count, { number: truncated ? `${count}+` : undefined, zero: 'No' }),
             log: log
         };
     }
 
+    private async getFilesQuery(): Promise<FilesQueryResults> {
+        const diff = await Container.git.getDiffStatus(
+            this.uri.repoPath!,
+            `${(this._compareWith && this._compareWith.ref) || 'HEAD'}${this.diffComparisonNotation}${
+                this.compareWithWorkingTree ? '' : this.branch.ref
+            }`
+        );
+
+        return {
+            label: `${Strings.pluralize('file', diff !== undefined ? diff.length : 0, { zero: 'No' })} changed`,
+            diff: diff
+        };
+    }
+
     private async updateCompareWith(compareWith: BranchComparison | undefined) {
         this._compareWith = compareWith;
 

diff --git a/src/views/nodes/compareResultsNode.ts b/src/views/nodes/compareResultsNode.ts
@@ -6,7 +6,7 @@ import { GitService, GitUri } from '../../git/gitService';
 import { debug, gate, log, Strings } from '../../system';
 import { CompareView } from '../compareView';
 import { CommitsQueryResults, ResultsCommitsNode } from './resultsCommitsNode';
-import { ResultsFilesNode } from './resultsFilesNode';
+import { FilesQueryResults, ResultsFilesNode } from './resultsFilesNode';
 import { ResourceType, SubscribeableViewNode, ViewNode } from './viewNode';
 
 let instanceId = 0;
@@ -63,7 +63,14 @@ export class CompareResultsNode extends SubscribeableViewNode<CompareView> {
                         includeDescription: true
                     }
                 ),
-                new ResultsFilesNode(this.view, this, this.uri.repoPath!, this._ref1.ref, this._ref2.ref)
+                new ResultsFilesNode(
+                    this.view,
+                    this,
+                    this.uri.repoPath!,
+                    this._ref1.ref,
+                    this._ref2.ref || 'HEAD',
+                    this.getFilesQuery.bind(this)
+                )
             ];
         }
         return this._children;
@@ -181,6 +188,13 @@ export class CompareResultsNode extends SubscribeableViewNode<CompareView> {
         return this._comparisonNotation || (Container.config.advanced.useSymmetricDifferenceNotation ? '...' : '..');
     }
 
+    private get diffComparisonNotation() {
+        // In git diff the range syntax doesn't mean the same thing as with git log -- since git diff is about comparing endpoints not ranges
+        // see https://git-scm.com/docs/git-diff#Documentation/git-diff.txt-emgitdiffemltoptionsgtltcommitgtltcommitgt--ltpathgt82308203
+        // So inverting the range syntax should be about equivalent for the behavior we want
+        return this.comparisonNotation === '...' ? '..' : '...';
+    }
+
     private async getCommitsQuery(maxCount: number | undefined): Promise<CommitsQueryResults> {
         const log = await Container.git.getLog(this.uri.repoPath!, {
             maxCount: maxCount,
@@ -190,14 +204,24 @@ export class CompareResultsNode extends SubscribeableViewNode<CompareView> {
         const count = log !== undefined ? log.count : 0;
         const truncated = log !== undefined ? log.truncated : false;
 
-        const label = Strings.pluralize('commit', count, { number: truncated ? `${count}+` : undefined, zero: 'No' });
-
         return {
-            label: label,
+            label: Strings.pluralize('commit', count, { number: truncated ? `${count}+` : undefined, zero: 'No' }),
             log: log
         };
     }
 
+    private async getFilesQuery(): Promise<FilesQueryResults> {
+        const diff = await Container.git.getDiffStatus(
+            this.uri.repoPath!,
+            `${this._ref1.ref}${this.diffComparisonNotation}${this._ref2.ref || 'HEAD'}`
+        );
+
+        return {
+            label: `${Strings.pluralize('file', diff !== undefined ? diff.length : 0, { zero: 'No' })} changed`,
+            diff: diff
+        };
+    }
+
     private getPinnableId() {
         return Strings.sha1(`${this.repoPath}|${this.ref1.ref}|${this.ref2.ref}`);
     }

diff --git a/src/views/nodes/resultsFilesNode.ts b/src/views/nodes/resultsFilesNode.ts
@@ -2,7 +2,6 @@
 import * as paths from 'path';
 import { TreeItem, TreeItemCollapsibleState } from 'vscode';
 import { ViewFilesLayout } from '../../configuration';
-import { Container } from '../../container';
 import { GitFile, GitUri } from '../../git/gitService';
 import { Arrays, debug, gate, Iterables, Promises, Strings } from '../../system';
 import { ViewWithFiles } from '../viewBase';
@@ -21,7 +20,8 @@ export class ResultsFilesNode extends ViewNode<ViewWithFiles> {
         parent: ViewNode,
         public readonly repoPath: string,
         public readonly ref1: string,
-        public readonly ref2: string
+        public readonly ref2: string,
+        private readonly _filesQuery: () => Promise<FilesQueryResults>
     ) {
         super(GitUri.fromRepoPath(repoPath), view, parent);
     }
@@ -92,25 +92,16 @@ export class ResultsFilesNode extends ViewNode<ViewWithFiles> {
     refresh(reset: boolean = false) {
         if (!reset) return;
 
-        this._filesQueryResults = this.getFilesQueryResultsCore();
+        this._filesQueryResults = this._filesQuery();
     }
 
     private _filesQueryResults: Promise<FilesQueryResults> | undefined;
 
     getFilesQueryResults() {
         if (this._filesQueryResults === undefined) {
-            this._filesQueryResults = this.getFilesQueryResultsCore();
+            this._filesQueryResults = this._filesQuery();
         }
 
         return this._filesQueryResults;
     }
-
-    private async getFilesQueryResultsCore(): Promise<FilesQueryResults> {
-        const diff = await Container.git.getDiffStatus(this.uri.repoPath!, this.ref1, this.ref2);
-
-        return {
-            label: `${Strings.pluralize('file', diff !== undefined ? diff.length : 0, { zero: 'No' })} changed`,
-            diff: diff
-        };
-    }
 }