Fixes #756, #809 - use ^ to ensure prev in diffs

CHANGELOG.md

@@ -8,6 +8,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
 
 ### Fixed
 
+- Fixes [#756](https://github.com/eamodio/vscode-gitlens/issues/756) - Merge commit shows only the changes from the last commit on those files
+- Fixes [#809](https://github.com/eamodio/vscode-gitlens/issues/809) - Wrong commit diff in file history
 - Fixes [#685](https://github.com/eamodio/vscode-gitlens/issues/685) - GitLens not loading for a single repository
 - Fixes [#789](https://github.com/eamodio/vscode-gitlens/issues/789) - Line blame annotations not working when vscode root is home dir and .gnupg dir is inaccessible
 - Fixes [#649](https://github.com/eamodio/vscode-gitlens/issues/649) - GitLens can't see the remote but git can

src/commands/diffWithPrevious.ts

@@ -1,7 +1,7 @@
 'use strict';
 import { commands, TextDocumentShowOptions, TextEditor, Uri } from 'vscode';
 import { Container } from '../container';
-import { GitCommit, GitLogCommit, GitService, GitUri } from '../git/gitService';
+import { GitCommit, GitService, GitUri } from '../git/gitService';
 import { Logger } from '../logger';
 import { Messages } from '../messages';
 import { ActiveEditorCommand, command, CommandContext, Commands, getCommandUri, openEditor } from './common';
@@ -50,16 +50,31 @@ export class DiffWithPreviousCommand extends ActiveEditorCommand {
             args.line = editor == null ? 0 : editor.selection.active.line;
         }
 
-        const gitUri = args.commit !== undefined ? GitUri.fromCommit(args.commit) : await GitUri.fromUri(uri);
+        if (args.commit !== undefined) {
+            const diffArgs: DiffWithCommandArgs = {
+                repoPath: args.commit.repoPath,
+                lhs: {
+                    sha: `${args.commit.sha}^`,
+                    uri: args.commit.originalUri
+                },
+                rhs: {
+                    sha: args.commit.sha || '',
+                    uri: args.commit.uri
+                },
+                line: args.line,
+                showOptions: args.showOptions
+            };
+            return commands.executeCommand(Commands.DiffWith, diffArgs);
+        }
+
+        const gitUri = await GitUri.fromUri(uri);
         try {
             const diffUris = await Container.git.getPreviousDiffUris(
                 gitUri.repoPath!,
                 gitUri,
                 gitUri.sha,
                 // If we are in a diff editor, assume we are on the right side, and need to skip back 1 more revisions
-                args.inDiffEditor ? 1 : 0,
-                // If this is a merge commit, walk back using the first parent only
-                args.commit && (args.commit as GitLogCommit).isMerge
+                args.inDiffEditor ? 1 : 0
             );
 
             if (diffUris === undefined || diffUris.previous === undefined) {

src/git/git.ts

@@ -755,17 +755,20 @@ export class Git {
     static async log__file_recent(
         repoPath: string,
         fileName: string,
-        { similarityThreshold }: { similarityThreshold?: number } = {}
+        { ref, similarityThreshold }: { ref?: string; similarityThreshold?: number } = {}
     ) {
-        const data = await git<string>(
-            { cwd: repoPath, errors: GitErrorHandling.Ignore },
+        const params = [
             'log',
             `-M${similarityThreshold == null ? '' : `${similarityThreshold}%`}`,
             '-n1',
-            '--format=%H',
-            '--',
-            fileName
-        );
+            '--format=%H'
+        ];
+
+        if (ref) {
+            params.push(ref);
+        }
+
+        const data = await git<string>({ cwd: repoPath, errors: GitErrorHandling.Ignore }, ...params, '--', fileName);
         return data.length === 0 ? undefined : data.trim();
     }
 

src/git/gitService.ts

@@ -1072,13 +1072,13 @@ export class GitService implements Disposable {
     ) {
         const [branches, tags] = await Promise.all<GitBranch[] | undefined, GitTag[] | undefined>([
             include === 'all' || include === 'branches'
-                ? Container.git.getBranches(repoPath, {
+                ? this.getBranches(repoPath, {
                       ...options,
                       filter: filterBranches && filterBranches
                   })
                 : undefined,
             include === 'all' || include === 'tags'
-                ? Container.git.getTags(repoPath, {
+                ? this.getTags(repoPath, {
                       ...options,
                       filter: filterTags && filterTags
                   })
@@ -1099,8 +1099,8 @@ export class GitService implements Disposable {
     @log()
     async getBranchesAndTagsTipsFn(repoPath: string | undefined, currentName?: string) {
         const [branches, tags] = await Promise.all([
-            Container.git.getBranches(repoPath),
-            Container.git.getTags(repoPath, { includeRefs: true })
+            this.getBranches(repoPath),
+            this.getTags(repoPath, { includeRefs: true })
         ]);
 
         const branchesAndTagsBySha = Arrays.groupByFilterMap(
@@ -1757,7 +1757,7 @@ export class GitService implements Disposable {
 
         const next = await this.getNextUri(repoPath, uri, ref);
         if (next === undefined) {
-            const status = await Container.git.getStatusForFile(repoPath, fileName);
+            const status = await this.getStatusForFile(repoPath, fileName);
             if (status !== undefined) {
                 // If the file is staged, diff with the staged version
                 if (status.indexStatus !== undefined) {
@@ -1847,7 +1847,7 @@ export class GitService implements Disposable {
         // If we are at the working tree (i.e. no ref), we need to dig deeper to figure out where to go
         if (ref === undefined || ref.length === 0) {
             // First, check the file status to see if there is anything staged
-            const status = await Container.git.getStatusForFile(repoPath, fileName);
+            const status = await this.getStatusForFile(repoPath, fileName);
             if (status !== undefined) {
                 // If the file is staged with working changes, diff working with staged (index)
                 // If the file is staged without working changes, diff staged with HEAD
@@ -1939,7 +1939,7 @@ export class GitService implements Disposable {
                 // If the document is dirty (unsaved), use the status to determine where to go
                 if (document.isDirty) {
                     // Check the file status to see if there is anything staged
-                    const status = await Container.git.getStatusForFile(repoPath, fileName);
+                    const status = await this.getStatusForFile(repoPath, fileName);
                     if (status !== undefined) {
                         // If the file is staged, diff working with staged (index)
                         // If the file is not staged, diff working with HEAD
@@ -2057,7 +2057,7 @@ export class GitService implements Disposable {
                 GitErrors.invalidLineCount.test(ex.message)
             ) {
                 if (ref === undefined) {
-                    const status = await Container.git.getStatusForFile(repoPath, fileName);
+                    const status = await this.getStatusForFile(repoPath, fileName);
                     if (status !== undefined && status.indexStatus !== undefined) {
                         return GitUri.fromFile(fileName, repoPath, GitService.uncommittedStagedSha);
                     }
@@ -2663,19 +2663,15 @@ export class GitService implements Disposable {
             return (await Git.rev_parse(repoPath, ref)) || ref;
         }
 
+        const fileName = Strings.normalizePath(paths.relative(repoPath, uri.fsPath));
+
         const match = Git.shaParentRegex.exec(ref);
         if (match != null) {
-            const previousUri = await Container.git.getPreviousUri(repoPath, uri, match[1]);
-            if (previousUri !== undefined && previousUri.sha !== undefined) {
-                return previousUri.sha;
-            }
+            const parentRef = await Git.log__file_recent(repoPath, fileName, { ref: ref });
+            if (parentRef !== undefined) return parentRef;
         }
 
-        const ensuredRef = await Git.cat_file__resolve(
-            repoPath,
-            Strings.normalizePath(paths.relative(repoPath, uri.fsPath)),
-            ref
-        );
+        const ensuredRef = await Git.cat_file__resolve(repoPath, fileName, ref);
         if (ensuredRef === undefined) return ref;
 
         return ensuredRef;

src/git/models/commit.ts

@@ -115,6 +115,11 @@ export abstract class GitCommit {
         return Git.isUncommittedStaged(this.sha);
     }
 
+    @memoize()
+    get originalUri(): Uri {
+        return this.originalFileName ? GitUri.resolveToUri(this.originalFileName, this.repoPath) : this.uri;
+    }
+
     get previousFileSha(): string {
         return `${this.sha}^`;
     }