Fixes #584, #523 - file history with merge commits, renames, etc

CHANGELOG.md

@@ -23,9 +23,11 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
 
 - Fixes an issue where gravatar icons would sometimes not show up — thanks to [PR #579](https://github.com/eamodio/vscode-gitlens/pull/579) by sgtwilko ([@sgtwilko](https://github.com/sgtwilko))
 - Fixes [#501](https://github.com/eamodio/vscode-gitlens/issues/501) — Azure DevOps ssh remotes aren't handled properly
+- Fixes [#523](https://github.com/eamodio/vscode-gitlens/issues/523) — File History doesn't show all commits on file
 - Fixes [#552](https://github.com/eamodio/vscode-gitlens/issues/552) — "Open Line Changes with..." doesn't work for renamed files
 - Fixes [#566](https://github.com/eamodio/vscode-gitlens/issues/566) — History error with UNC
 - Fixes [#572](https://github.com/eamodio/vscode-gitlens/issues/572) — Explorer cant expand some branch folders
+- Fixes [#584](https://github.com/eamodio/vscode-gitlens/issues/584) — Unexpected results when opening diff from file history
 - Fixes an issue where comparing a file with its staged revision doesn't show any content
 - Fixes an issue where the workspace folder added by the _Explore Repository from Here_ command (`gitlens.views.exploreRepoRevision`) would fail to load in certain cases
 

src/commands/diffWithPrevious.ts

@@ -57,8 +57,9 @@ export class DiffWithPreviousCommand extends ActiveEditorCommand {
                 }
 
                 // If we are in a diff editor, assume we are on the right side, and need to move back 2 revisions
+                const originalSha = sha;
                 if (args.inDiffEditor && sha !== undefined) {
-                    sha = sha + '^';
+                    sha = `${sha}^`;
                 }
 
                 args.commit = undefined;
@@ -81,7 +82,7 @@ export class DiffWithPreviousCommand extends ActiveEditorCommand {
                     // Check for renames
                     log = await Container.git.getLogForFile(gitUri.repoPath, gitUri.fsPath, {
                         maxCount: 3,
-                        ref: sha.substring(0, sha.length - 1),
+                        ref: originalSha,
                         renames: true
                     });
 
@@ -92,6 +93,10 @@ export class DiffWithPreviousCommand extends ActiveEditorCommand {
                     args.commit =
                         Iterables.next(Iterables.skip(log.commits.values(), 1)) ||
                         Iterables.first(log.commits.values());
+
+                    if (args.commit.sha === originalSha) {
+                        return Messages.showCommitHasNoPreviousCommitWarningMessage();
+                    }
                 }
 
                 // If the sha is missing (i.e. working tree), check the file status

src/git/git.ts

@@ -592,7 +592,7 @@ export class Git {
     ) {
         const [file, root] = Git.splitPath(fileName, repoPath);
 
-        const params = [...defaultLogParams];
+        const params = [...defaultLogParams, '-m'];
         if (options.maxCount && !options.reverse) {
             params.push(`-n${options.maxCount}`);
         }
@@ -630,20 +630,6 @@ export class Git {
         return data.length === 0 ? undefined : data.trim();
     }
 
-    static async log_resolve(repoPath: string, fileName: string, ref: string) {
-        const data = await git<string>(
-            { cwd: repoPath, errors: GitErrorHandling.Ignore },
-            'log',
-            '-M',
-            '-n1',
-            '--format=%H',
-            ref,
-            '--',
-            fileName
-        );
-        return data.length === 0 ? undefined : data.trim();
-    }
-
     static log_search(repoPath: string, search: string[] = emptyArray, options: { maxCount?: number } = {}) {
         const params = [...defaultLogParams];
         if (options.maxCount) {

src/git/gitService.ts

@@ -1471,10 +1471,46 @@ export class GitService implements Disposable {
                         }
 
                         Logger.debug(cc, `Cache ?: '${key}'`);
-                        const log = await cachedLog.item;
-                        if (log !== undefined && log.commits.has(options.ref)) {
+                        let log = await cachedLog.item;
+                        if (log !== undefined && !log.truncated && log.commits.has(options.ref)) {
                             Logger.debug(cc, `Cache hit: '${key}'`);
-                            return cachedLog.item;
+
+                            // Create a copy of the log starting at the requested commit
+                            let skip = true;
+                            let i = 0;
+                            const authors = new Map<string, GitAuthor>();
+                            const commits = new Map(
+                                Iterables.filterMap<[string, GitLogCommit], [string, GitLogCommit]>(
+                                    log.commits.entries(),
+                                    ([ref, c]) => {
+                                        if (skip) {
+                                            if (ref !== options.ref) return undefined;
+                                            skip = false;
+                                        }
+
+                                        i++;
+                                        if (options.maxCount !== undefined && i > options.maxCount) {
+                                            return undefined;
+                                        }
+
+                                        authors.set(c.author, log.authors.get(c.author)!);
+                                        return [ref, c];
+                                    }
+                                )
+                            );
+
+                            const opts = { ...options };
+                            log = {
+                                ...log,
+                                maxCount: options.maxCount,
+                                count: commits.size,
+                                commits: commits,
+                                authors: authors,
+                                query: (maxCount: number | undefined) =>
+                                    this.getLogForFile(repoPath, fileName, { ...opts, maxCount: maxCount })
+                            };
+
+                            return log;
                         }
                     }
                 }
@@ -2053,14 +2089,11 @@ export class GitService implements Disposable {
         const resolved = Git.isSha(ref) || !Git.isShaLike(ref) || ref.endsWith('^3');
         if (uri == null) return resolved ? ref : (await Git.revparse(repoPath, ref)) || ref;
 
-        const fileName = Strings.normalizePath(paths.relative(repoPath, uri.fsPath));
-
-        let resolvedRef;
-        if (!resolved) {
-            resolvedRef = await Git.log_resolve(repoPath, fileName, ref);
-        }
-
-        const ensuredRef = await Git.cat_file_validate(repoPath, fileName, resolvedRef || ref);
+        const ensuredRef = await Git.cat_file_validate(
+            repoPath,
+            Strings.normalizePath(paths.relative(repoPath, uri.fsPath)),
+            ref
+        );
         if (ensuredRef === undefined) return ref;
 
         return ensuredRef;

src/git/parsers/logParser.ts

@@ -300,7 +300,7 @@ export class GitLogParser {
                 entry.files || [],
                 entry.status,
                 originalFileName,
-                undefined,
+                `${entry.ref!}^`,
                 undefined,
                 entry.parentShas!
             );
@@ -312,8 +312,6 @@ export class GitLogParser {
         // }
 
         if (recentCommit !== undefined) {
-            recentCommit.previousSha = commit.sha;
-
             // If the commit sha's match (merge commit), just forward it along
             commit.nextSha = commit.sha !== recentCommit.sha ? recentCommit.sha : recentCommit.nextSha;