Fixes issues with next commit navigation (renames)

Adds sha to log model to know if it is a full log or not

src/commands/diffWithNext.ts

@@ -42,7 +42,7 @@ export class DiffWithNextCommand extends ActiveEditorCommand {
 
                 const sha = (commit && commit.sha) || gitUri.sha;
 
-                const log = await this.git.getLogForFile(gitUri.repoPath, gitUri.fsPath, undefined, rangeOrLine as Range, sha ? undefined : 2);
+                const log = await this.git.getLogForFile(gitUri.repoPath, gitUri.fsPath, undefined, sha ? undefined : 2, rangeOrLine as Range);
                 if (!log) return window.showWarningMessage(`Unable to open diff. File is probably not under source control`);
 
                 commit = (sha && log.commits.get(sha)) || Iterables.first(log.commits.values());

src/commands/diffWithPrevious.ts

@@ -43,7 +43,7 @@ export class DiffWithPreviousCommand extends ActiveEditorCommand {
 
                 const sha = (commit && commit.sha) || gitUri.sha;
 
-                const log = await this.git.getLogForFile(gitUri.repoPath, gitUri.fsPath, undefined, rangeOrLine as Range, sha ? undefined : 2);
+                const log = await this.git.getLogForFile(gitUri.repoPath, gitUri.fsPath, undefined, sha ? undefined : 2, rangeOrLine as Range);
                 if (!log) return window.showWarningMessage(`Unable to open diff. File is probably not under source control`);
 
                 commit = (sha && log.commits.get(sha)) || Iterables.first(log.commits.values());

src/commands/showQuickFileHistory.ts

@@ -28,7 +28,7 @@ export class ShowQuickFileHistoryCommand extends ActiveEditorCachedCommand {
         const progressCancellation = FileHistoryQuickPick.showProgress(gitUri);
         try {
             if (!log) {
-                log = await this.git.getLogForFile(gitUri.repoPath, gitUri.fsPath, gitUri.sha, range, maxCount);
+                log = await this.git.getLogForFile(gitUri.repoPath, gitUri.fsPath, gitUri.sha, maxCount, range);
                 if (!log) return window.showWarningMessage(`Unable to show file history. File is probably not under source control`);
             }
 

src/git/models/log.ts

@@ -8,6 +8,7 @@ export interface IGitLog {
     authors: Map<string, IGitAuthor>;
     commits: Map<string, GitLogCommit>;
 
+    sha: string | undefined;
     maxCount: number | undefined;
     range: Range;
     truncated: boolean;

src/git/parsers/logParser.ts

@@ -183,7 +183,7 @@ export class GitLogParser {
         return entries;
     }
 
-    static parse(data: string, type: GitLogType, fileNameOrRepoPath: string, maxCount: number | undefined, isRepoPath: boolean, reverse: boolean, range: Range): IGitLog {
+    static parse(data: string, type: GitLogType, fileNameOrRepoPath: string, sha: string | undefined, maxCount: number | undefined, isRepoPath: boolean, reverse: boolean, range: Range): IGitLog {
         const entries = this._parseEntries(data, isRepoPath, maxCount, reverse);
         if (!entries) return undefined;
 
@@ -271,6 +271,7 @@ export class GitLogParser {
             authors: sortedAuthors,
             // commits: sortedCommits,
             commits: commits,
+            sha: sha,
             maxCount: maxCount,
             range: range,
             truncated: !!(maxCount && entries.length >= maxCount)

src/gitService.ts

@@ -224,8 +224,54 @@ export class GitService extends Disposable {
         }
     }
 
+    private async _fileExists(repoPath: string, fileName: string): Promise<boolean> {
+        return await new Promise<boolean>((resolve, reject) => fs.exists(path.resolve(repoPath, fileName), e => resolve(e)));
+    }
+
+    async findNextCommit(repoPath: string, fileName: string, sha?: string): Promise<GitLogCommit> {
+        let log = await this.getLogForFile(repoPath, fileName, sha, 1, undefined, { follow: true, reverse: true });
+        let commit = log && Iterables.first(log.commits.values());
+        if (commit) return commit;
+
+        fileName = await this.findNextFileName(repoPath, fileName, sha);
+        if (fileName) {
+            log = await this.getLogForFile(repoPath, fileName, sha, 1, undefined, { follow: true, reverse: true });
+            commit = log && Iterables.first(log.commits.values());
+        }
+
+        return commit;
+    }
+
+    async findNextFileName(repoPath: string, fileName: string, sha?: string): Promise<string> {
+        [fileName, repoPath] = Git.splitPath(fileName, repoPath);
+
+        return (await this._fileExists(repoPath, fileName))
+            ? fileName
+            : await this._findNextFileName(repoPath, fileName, sha);
+    }
+
+    async _findNextFileName(repoPath: string, fileName: string, sha?: string): Promise<string> {
+        if (sha === undefined) {
+            // Get the most recent commit for this file name
+            const c = await this.getLogCommit(repoPath, fileName);
+            if (!c) return undefined;
+
+            sha = c.sha;
+        }
+
+        // Get the full commit (so we can see if there are any matching renames in the file statuses)
+        const log = await this.getLogForRepo(repoPath, sha, 1);
+        if (!log) return undefined;
+
+        const c = Iterables.first(log.commits.values());
+        const status = c.fileStatuses.find(_ => _.originalFileName === fileName);
+        if (!status) return undefined;
+
+        return status.fileName;
+    }
+
     async findWorkingFileName(commit: GitCommit): Promise<string>;
-    async findWorkingFileName(repoPath: string, fileName: string): Promise<string>
+    async findWorkingFileName(repoPath: string, fileName: string): Promise<string>;
     async findWorkingFileName(commitOrRepoPath: GitCommit | string, fileName?: string): Promise<string> {
         let repoPath: string;
         if (typeof commitOrRepoPath === 'string') {
@@ -242,26 +288,11 @@ export class GitService extends Disposable {
         while (true) {
             if (await this._fileExists(repoPath, fileName)) return fileName;
 
-            // Get the most recent commit for this file name
+            fileName = await this._findNextFileName(repoPath, fileName);
-            let c = await this.getLogCommit(repoPath, fileName);
+            if (fileName === undefined) return undefined;
-            if (!c) return undefined;
-
-            // Get the full commit (so we can see if there are any matching renames in the file statuses)
-            let log = await this.getLogForRepo(repoPath, c.sha, 1);
-            if (!log) return undefined;
-
-            c = Iterables.first(log.commits.values());
-            const status = c.fileStatuses.find(_ => _.originalFileName === fileName);
-            if (!status) return undefined;
-
-            fileName = status.fileName;
         }
     }
 
-    private async _fileExists(repoPath: string, fileName: string): Promise<boolean> {
-        return await new Promise<boolean>((resolve, reject) => fs.exists(path.resolve(repoPath, fileName), e => resolve(e)));
-    }
-
     public getBlameability(fileName: string): boolean {
         if (!this.UseGitCaching) return true;
 
@@ -482,22 +513,6 @@ export class GitService extends Disposable {
         return entry && entry.uri;
     }
 
-    async getLogForRepo(repoPath: string, sha?: string, maxCount?: number, reverse: boolean = false): Promise<IGitLog | undefined> {
-        Logger.log(`getLogForRepo('${repoPath}', ${sha}, ${maxCount})`);
-
-        if (maxCount == null) {
-            maxCount = this.config.advanced.maxQuickHistory || 0;
-        }
-
-        try {
-            const data = await Git.log(repoPath, sha, maxCount, reverse);
-            return GitLogParser.parse(data, 'repo', repoPath, maxCount, true, reverse, undefined);
-        }
-        catch (ex) {
-            return undefined;
-        }
-    }
-
     async getLogCommit(repoPath: string, fileName: string, options?: { firstIfMissing?: boolean, previous?: boolean }): Promise<GitLogCommit | undefined>;
     async getLogCommit(repoPath: string, fileName: string, sha: string, options?: { firstIfMissing?: boolean, previous?: boolean }): Promise<GitLogCommit | undefined>;
     async getLogCommit(repoPath: string, fileName: string, shaOrOptions?: string | { firstIfMissing?: boolean, previous?: boolean }, options?: { firstIfMissing?: boolean, previous?: boolean }): Promise<GitLogCommit | undefined> {
@@ -511,20 +526,36 @@ export class GitService extends Disposable {
 
         options = options || {};
 
-        const log = await this.getLogForFile(repoPath, fileName, sha, undefined, options.previous ? 2 : 1);
+        const log = await this.getLogForFile(repoPath, fileName, sha, options.previous ? 2 : 1);
         if (!log) return undefined;
 
         const commit = sha && log.commits.get(sha);
-        if (!commit && !options.firstIfMissing) return undefined;
+        if (!commit && sha && !options.firstIfMissing) return undefined;
 
         return commit || Iterables.first(log.commits.values());
     }
 
-    getLogForFile(repoPath: string, fileName: string, sha?: string, range?: Range, maxCount?: number, reverse: boolean = false): Promise<IGitLog | undefined> {
+    async getLogForRepo(repoPath: string, sha?: string, maxCount?: number, reverse: boolean = false): Promise<IGitLog | undefined> {
-        Logger.log(`getLogForFile('${repoPath}', '${fileName}', ${sha}, ${range && `[${range.start.line}, ${range.end.line}]`}, ${maxCount}, ${reverse})`);
+        Logger.log(`getLogForRepo('${repoPath}', ${sha}, ${maxCount})`);
+
+        if (maxCount == null) {
+            maxCount = this.config.advanced.maxQuickHistory || 0;
+        }
+
+        try {
+            const data = await Git.log(repoPath, sha, maxCount, reverse);
+            return GitLogParser.parse(data, 'repo', repoPath, sha, maxCount, true, reverse, undefined);
+        }
+        catch (ex) {
+            return undefined;
+        }
+    }
+
+    getLogForFile(repoPath: string, fileName: string, sha?: string, maxCount?: number, range?: Range, options: { follow?: boolean, reverse?: boolean } = {}): Promise<IGitLog | undefined> {
+        Logger.log(`getLogForFile('${repoPath}', '${fileName}', ${sha}, ${maxCount}, ${range && `[${range.start.line}, ${range.end.line}]`}, ${options})`);
 
         let entry: GitCacheEntry | undefined;
-        if (this.UseGitCaching && !sha && !range && !maxCount && !reverse) {
+        if (this.UseGitCaching && !sha && !range && !maxCount && !options.reverse) {
             const cacheKey = this.getCacheEntryKey(fileName);
             entry = this._gitCache.get(cacheKey);
 
@@ -534,7 +565,7 @@ export class GitService extends Disposable {
             }
         }
 
-        const promise = this._getLogForFile(repoPath, fileName, sha, range, maxCount, reverse, entry);
+        const promise = this._getLogForFile(repoPath, fileName, sha, range, maxCount, options, entry);
 
         if (entry) {
             Logger.log(`Add log cache for '${entry.key}'`);
@@ -550,7 +581,7 @@ export class GitService extends Disposable {
         return promise;
     }
 
-    private async _getLogForFile(repoPath: string, fileName: string, sha: string, range: Range, maxCount: number, reverse: boolean, entry: GitCacheEntry | undefined): Promise<IGitLog> {
+    private async _getLogForFile(repoPath: string, fileName: string, sha: string, range: Range, maxCount: number, options: { follow?: boolean, reverse?: boolean } = {}, entry: GitCacheEntry | undefined): Promise<IGitLog> {
         const [file, root] = Git.splitPath(fileName, repoPath, false);
 
         const ignore = await this._gitignore;
@@ -560,8 +591,8 @@ export class GitService extends Disposable {
         }
 
         try {
-            const data = await Git.log_file(root, file, sha, maxCount, reverse, range && range.start.line + 1, range && range.end.line + 1);
+            const data = await Git.log_file(root, file, sha, maxCount, options.reverse, range && range.start.line + 1, range && range.end.line + 1);
-            return GitLogParser.parse(data, 'file', root || file, maxCount, !!root, reverse, range);
+            return GitLogParser.parse(data, 'file', root || file, sha, maxCount, !!root, options.reverse, range);
         }
         catch (ex) {
             // Trap and cache expected log errors
@@ -626,7 +657,7 @@ export class GitService extends Disposable {
     }
 
     async getRepoPathFromFile(fileName: string): Promise<string | undefined> {
-        const log = await this.getLogForFile(undefined, fileName, undefined, undefined, 1);
+        const log = await this.getLogForFile(undefined, fileName, undefined, 1);
         return log && log.repoPath;
     }
 

src/quickPicks/branchHistory.ts

@@ -31,7 +31,7 @@ export class BranchHistoryQuickPick {
 
         let previousPageCommand: CommandQuickPickItem;
 
-        if ((log.truncated || (uri && uri.sha))) {
+        if (log.truncated || log.sha) {
             if (log.truncated) {
                 items.splice(0, 0, new CommandQuickPickItem({
                     label: `$(sync) Show All Commits`,

src/quickPicks/commitDetails.ts

@@ -84,7 +84,7 @@ export class CommitDetailsQuickPick {
         let previousCommand: CommandQuickPickItem | (() => Promise<CommandQuickPickItem>);
         let nextCommand: CommandQuickPickItem | (() => Promise<CommandQuickPickItem>);
         // If we have the full history, we are good
-        if (repoLog && !repoLog.truncated) {
+        if (repoLog && !repoLog.truncated && !repoLog.sha) {
             previousCommand = commit.previousSha && new KeyCommandQuickPickItem(Commands.ShowQuickCommitDetails, [commit.previousUri, commit.previousSha, undefined, goBackCommand, repoLog]);
             nextCommand = commit.nextSha && new KeyCommandQuickPickItem(Commands.ShowQuickCommitDetails, [commit.nextUri, commit.nextSha, undefined, goBackCommand, repoLog]);
         }
@@ -103,7 +103,7 @@ export class CommitDetailsQuickPick {
                         c.nextSha = commit.nextSha;
                     }
                 }
-                if (!c) return KeyNoopCommand;
+                if (!c || !c.previousSha) return KeyNoopCommand;
                 return new KeyCommandQuickPickItem(Commands.ShowQuickCommitDetails, [c.previousUri, c.previousSha, undefined, goBackCommand, log]);
             };
 
@@ -124,7 +124,7 @@ export class CommitDetailsQuickPick {
                         c.nextSha = next.sha;
                     }
                 }
-                if (!c) return KeyNoopCommand;
+                if (!c || !c.nextSha) return KeyNoopCommand;
                 return new KeyCommandQuickPickItem(Commands.ShowQuickCommitDetails, [c.nextUri, c.nextSha, undefined, goBackCommand, log]);
             };
         }

src/quickPicks/commitFileDetails.ts

@@ -1,5 +1,5 @@
 'use strict';
-import { Arrays, Iterables } from '../system';
+import { Arrays } from '../system';
 import { QuickPickItem, QuickPickOptions, Uri, window } from 'vscode';
 import { Commands, Keyboard, KeyNoopCommand } from '../commands';
 import { GitCommit, GitLogCommit, GitService, GitUri, IGitLog } from '../gitService';
@@ -105,7 +105,7 @@ export class CommitFileDetailsQuickPick {
         let previousCommand: CommandQuickPickItem | (() => Promise<CommandQuickPickItem>);
         let nextCommand: CommandQuickPickItem | (() => Promise<CommandQuickPickItem>);
         // If we have the full history, we are good
-        if (fileLog && !fileLog.truncated) {
+        if (fileLog && !fileLog.truncated && !fileLog.sha) {
             previousCommand = commit.previousSha && new KeyCommandQuickPickItem(Commands.ShowQuickCommitFileDetails, [commit.previousUri, commit.previousSha, undefined, goBackCommand, fileLog]);
             nextCommand = commit.nextSha && new KeyCommandQuickPickItem(Commands.ShowQuickCommitFileDetails, [commit.nextUri, commit.nextSha, undefined, goBackCommand, fileLog]);
         }
@@ -116,7 +116,7 @@ export class CommitFileDetailsQuickPick {
 
                 // If we can't find the commit or the previous commit isn't available (since it isn't trustworthy)
                 if (!c || !c.previousSha) {
-                    log = await git.getLogForFile(commit.repoPath, uri.fsPath, commit.sha, undefined, git.config.advanced.maxQuickHistory);
+                    log = await git.getLogForFile(commit.repoPath, uri.fsPath, commit.sha, git.config.advanced.maxQuickHistory);
                     c = log && log.commits.get(commit.sha);
 
                     if (c) {
@@ -125,7 +125,7 @@ export class CommitFileDetailsQuickPick {
                         c.nextFileName = commit.nextFileName;
                     }
                 }
-                if (!c) return KeyNoopCommand;
+                if (!c || !c.previousSha) return KeyNoopCommand;
                 return new KeyCommandQuickPickItem(Commands.ShowQuickCommitFileDetails, [c.previousUri, c.previousSha, undefined, goBackCommand, log]);
             };
 
@@ -139,15 +139,14 @@ export class CommitFileDetailsQuickPick {
                     c = undefined;
 
                     // Try to find the next commit
-                    const nextLog = await git.getLogForFile(commit.repoPath, uri.fsPath, commit.sha, undefined, 1, true, true);
+                    const next = await git.findNextCommit(commit.repoPath, uri.fsPath, commit.sha);
-                    const next = nextLog && Iterables.first(nextLog.commits.values());
                     if (next && next.sha !== commit.sha) {
                         c = commit;
                         c.nextSha = next.sha;
                         c.nextFileName = next.originalFileName || next.fileName;
                     }
                 }
-                if (!c) return KeyNoopCommand;
+                if (!c || !c.nextSha) return KeyNoopCommand;
                 return new KeyCommandQuickPickItem(Commands.ShowQuickCommitFileDetails, [c.nextUri, c.nextSha, undefined, goBackCommand, log]);
             };
         }

src/quickPicks/fileHistory.ts

@@ -23,7 +23,7 @@ export class FileHistoryQuickPick {
         let previousPageCommand: CommandQuickPickItem;
 
         let index = 0;
-        if (log.truncated || uri.sha) {
+        if (log.truncated || log.sha) {
             if (log.truncated) {
                 index++;
                 items.splice(0, 0, new CommandQuickPickItem({