From 7fa35659409534d96dcd78cdea1d20beda6e88eb Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Sun, 6 Feb 2022 02:13:30 -0500 Subject: [PATCH] Deals with resolved vs unresolved previous shas --- src/commands/diffLineWithWorking.ts | 3 +- src/commands/diffWith.ts | 10 ++++--- src/commands/externalDiff.ts | 5 ++-- src/commands/gitCommands.actions.ts | 11 ++++---- src/commands/openCommitOnRemote.ts | 2 +- src/commands/openFileAtRevision.ts | 17 ++++++----- src/commands/openRevisionFile.ts | 7 ++++- src/env/node/git/localGitProvider.ts | 10 +++++-- src/git/formatters/commitFormatter.ts | 6 ++-- src/git/gitProviderService.ts | 7 ++++- src/git/models/commit.ts | 44 +++++++++++++++++++++++++++-- src/hovers/hovers.ts | 9 ++++-- src/premium/github/githubGitProvider.ts | 14 ++++++--- src/views/nodes/branchTrackingStatusNode.ts | 3 +- src/views/viewCommands.ts | 4 +-- 15 files changed, 112 insertions(+), 40 deletions(-) diff --git a/src/commands/diffLineWithWorking.ts b/src/commands/diffLineWithWorking.ts index 20b3ce0..4b99c76 100644 --- a/src/commands/diffLineWithWorking.ts +++ b/src/commands/diffLineWithWorking.ts @@ -58,7 +58,8 @@ export class DiffLineWithWorkingCommand extends ActiveEditorCommand { args.commit.repoPath, ); } else { - lhsSha = args.commit.file!.previousSha ?? GitRevision.deletedOrMissing; + // Don't need to worry about verifying the previous sha, as the DiffWith command will + lhsSha = args.commit.unresolvedPreviousSha; lhsUri = args.commit.file!.originalUri ?? args.commit.file!.uri; } } else { diff --git a/src/commands/diffWith.ts b/src/commands/diffWith.ts index 36a5e88..8959020 100644 --- a/src/commands/diffWith.ts +++ b/src/commands/diffWith.ts @@ -30,7 +30,7 @@ export class DiffWithCommand extends Command { let args: DiffWithCommandArgs | GitCommit; if (GitCommit.is(argsOrCommit)) { const commit = argsOrCommit; - if (commit.file == null) { + if (commit.file == null || commit.unresolvedPreviousSha == null) { debugger; throw new Error('Commit has no file'); } @@ -52,7 +52,7 @@ export class DiffWithCommand extends Command { args = { repoPath: commit.repoPath, lhs: { - sha: commit.file.previousSha ?? GitRevision.deletedOrMissing, + sha: commit.unresolvedPreviousSha, uri: commit.file.originalUri ?? commit.file.uri, }, rhs: { @@ -91,10 +91,12 @@ export class DiffWithCommand extends Command { [args.lhs.sha, args.rhs.sha] = await Promise.all([ await this.container.git.resolveReference(args.repoPath, args.lhs.sha, args.lhs.uri, { - timeout: 100, + // If the ref looks like a sha, don't wait too long, since it should work + timeout: GitRevision.isSha(args.lhs.sha) ? 100 : undefined, }), await this.container.git.resolveReference(args.repoPath, args.rhs.sha, args.rhs.uri, { - timeout: 100, + // If the ref looks like a sha, don't wait too long, since it should work + timeout: GitRevision.isSha(args.rhs.sha) ? 100 : undefined, }), ]); diff --git a/src/commands/externalDiff.ts b/src/commands/externalDiff.ts index e9d0fa3..de80995 100644 --- a/src/commands/externalDiff.ts +++ b/src/commands/externalDiff.ts @@ -38,9 +38,8 @@ export class ExternalDiffCommand extends Command { args = { ...args }; if (isCommandContextViewNodeHasFileCommit(context)) { - const ref1 = GitRevision.isUncommitted(context.node.commit.previousSha) - ? '' - : context.node.commit.previousSha; + const previousSha = await context.node.commit.getPreviousSha(); + const ref1 = GitRevision.isUncommitted(previousSha) ? '' : previousSha; const ref2 = context.node.commit.isUncommitted ? '' : context.node.commit.sha; args.files = [ diff --git a/src/commands/gitCommands.actions.ts b/src/commands/gitCommands.actions.ts index 0ea6b04..e40f1dd 100644 --- a/src/commands/gitCommands.actions.ts +++ b/src/commands/gitCommands.actions.ts @@ -219,7 +219,8 @@ export namespace GitActions { files = commitOrFiles.files ?? []; refs = { repoPath: commitOrFiles.repoPath, - ref1: commitOrFiles.previousSha != null ? commitOrFiles.previousSha : GitRevision.deletedOrMissing, + // Don't need to worry about verifying the previous sha, as the DiffWith command will + ref1: commitOrFiles.unresolvedPreviousSha, ref2: commitOrFiles.sha, }; @@ -361,8 +362,8 @@ export namespace GitActions { const refs = GitCommit.is(commitOrRefs) ? { repoPath: commitOrRefs.repoPath, - ref1: - commitOrRefs.previousSha != null ? commitOrRefs.previousSha : GitRevision.deletedOrMissing, + // Don't need to worry about verifying the previous sha, as the DiffWith command will + ref1: commitOrRefs.unresolvedPreviousSha, ref2: commitOrRefs.sha, } : commitOrRefs; @@ -548,7 +549,7 @@ export namespace GitActions { } uri = Container.instance.git.getRevisionUri( - file.status === 'D' ? commit.previousSha : commit.sha, + file.status === 'D' ? (await commit.getPreviousSha()) ?? GitRevision.deletedOrMissing : commit.sha, file, commit.repoPath, ); @@ -633,7 +634,7 @@ export namespace GitActions { files = commitOrFiles.files ?? []; repoPath = commitOrFiles.repoPath; ref1 = commitOrFiles.sha; - ref2 = commitOrFiles.previousSha; + ref2 = await commitOrFiles.getPreviousSha(); } else { files = commitOrFiles; } diff --git a/src/commands/openCommitOnRemote.ts b/src/commands/openCommitOnRemote.ts index c6ae481..abda1ea 100644 --- a/src/commands/openCommitOnRemote.ts +++ b/src/commands/openCommitOnRemote.ts @@ -82,7 +82,7 @@ export class OpenCommitOnRemoteCommand extends ActiveEditorCommand { // If the line is uncommitted, use previous commit args.sha = blame.commit.isUncommitted - ? blame.commit.file!.previousSha ?? GitRevision.deletedOrMissing + ? (await blame.commit.getPreviousSha()) ?? GitRevision.deletedOrMissing : blame.commit.sha; } diff --git a/src/commands/openFileAtRevision.ts b/src/commands/openFileAtRevision.ts index 7cfd49b..effd015 100644 --- a/src/commands/openFileAtRevision.ts +++ b/src/commands/openFileAtRevision.ts @@ -60,18 +60,21 @@ export class OpenFileAtRevisionCommand extends ActiveEditorCommand { const blame = await this.container.git.getBlameForLine(gitUri, editorLine); if (blame != null) { if (blame.commit.isUncommitted) { - const diffUris = await blame.commit.getPreviousComparisonUrisForLine(editorLine); - if (diffUris?.previous != null) { - args.revisionUri = this.container.git.getRevisionUri(diffUris.previous); + const comparisonUris = await blame.commit.getPreviousComparisonUrisForLine(editorLine); + if (comparisonUris?.previous != null) { + args.revisionUri = this.container.git.getRevisionUri(comparisonUris.previous); } else { void Messages.showCommitHasNoPreviousCommitWarningMessage(blame.commit); return undefined; } - } else if (blame?.commit.previousSha != null) { - args.revisionUri = this.container.git.getRevisionUri(blame.commit.getGitUri(true)); } else { - void Messages.showCommitHasNoPreviousCommitWarningMessage(blame.commit); - return undefined; + const previousSha = blame != null ? await blame?.commit.getPreviousSha() : undefined; + if (previousSha != null) { + args.revisionUri = this.container.git.getRevisionUri(blame.commit.getGitUri(true)); + } else { + void Messages.showCommitHasNoPreviousCommitWarningMessage(blame.commit); + return undefined; + } } } } catch (ex) { diff --git a/src/commands/openRevisionFile.ts b/src/commands/openRevisionFile.ts index 53d990e..401e9fb 100644 --- a/src/commands/openRevisionFile.ts +++ b/src/commands/openRevisionFile.ts @@ -2,6 +2,7 @@ import { TextDocumentShowOptions, TextEditor, Uri } from 'vscode'; import { FileAnnotationType } from '../configuration'; import type { Container } from '../container'; import { GitUri } from '../git/gitUri'; +import { GitRevision } from '../git/models'; import { Logger } from '../logger'; import { Messages } from '../messages'; import { ActiveEditorCommand, command, Commands, getCommandUri } from './common'; @@ -39,7 +40,11 @@ export class OpenRevisionFileCommand extends ActiveEditorCommand { args.revisionUri = commit?.file?.status === 'D' - ? this.container.git.getRevisionUri(commit.previousSha, commit.file, commit.repoPath) + ? this.container.git.getRevisionUri( + (await commit.getPreviousSha()) ?? GitRevision.deletedOrMissing, + commit.file, + commit.repoPath, + ) : this.container.git.getRevisionUri(gitUri); } else { args.revisionUri = this.container.git.getRevisionUri(gitUri); diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 264cc6e..96e0611 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -3502,12 +3502,18 @@ export class LocalGitProvider implements GitProvider, Disposable { @log() async resolveReference(repoPath: string, ref: string, pathOrUri?: string | Uri, options?: { timeout?: number }) { - if (!ref || ref === GitRevision.deletedOrMissing || GitRevision.isUncommitted(ref)) { + if ( + !ref || + ref === GitRevision.deletedOrMissing || + (pathOrUri == null && GitRevision.isSha(ref)) || + (pathOrUri != null && GitRevision.isUncommitted(ref)) + ) { return ref; } if (pathOrUri == null) { - if (GitRevision.isSha(ref) || !GitRevision.isShaLike(ref) || ref.endsWith('^3')) return ref; + // If it doesn't look like a sha at all (e.g. branch name) or is a stash ref (^3) don't try to resolve it + if (!GitRevision.isShaLike(ref) || ref.endsWith('^3')) return ref; return (await this.git.rev_parse__verify(repoPath, ref)) ?? ref; } diff --git a/src/git/formatters/commitFormatter.ts b/src/git/formatters/commitFormatter.ts index 37fa4bd..f21515e 100644 --- a/src/git/formatters/commitFormatter.ts +++ b/src/git/formatters/commitFormatter.ts @@ -319,10 +319,10 @@ export class CommitFormatter extends Formatter { this._options.editor?.line, )} "Open Changes with Previous Revision")`; - if (this._item.previousSha != null && this._item.file?.originalPath != null) { + if (this._item.file != null && this._item.unresolvedPreviousSha != null) { const uri = Container.instance.git.getRevisionUri( - this._item.previousSha, - this._item.file.originalPath, + this._item.unresolvedPreviousSha, + this._item.file.originalPath ?? this._item.file?.path, this._item.repoPath, ); commands += `   [$(versions)](${OpenFileAtRevisionCommand.getMarkdownCommandArgs( diff --git a/src/git/gitProviderService.ts b/src/git/gitProviderService.ts index 0742770..9c1be5c 100644 --- a/src/git/gitProviderService.ts +++ b/src/git/gitProviderService.ts @@ -1845,7 +1845,12 @@ export class GitProviderService implements Disposable { pathOrUri?: string | Uri, options?: { timeout?: number }, ) { - if (!ref || ref === GitRevision.deletedOrMissing || GitRevision.isUncommitted(ref)) { + if ( + !ref || + ref === GitRevision.deletedOrMissing || + (pathOrUri == null && GitRevision.isSha(ref)) || + (pathOrUri != null && GitRevision.isUncommitted(ref)) + ) { return ref; } diff --git a/src/git/models/commit.ts b/src/git/models/commit.ts index bbcf257..26ccd1e 100644 --- a/src/git/models/commit.ts +++ b/src/git/models/commit.ts @@ -176,8 +176,11 @@ export class GitCommit implements GitRevisionReference { return this._summary; } - get previousSha(): string { - return this.file?.previousSha ?? this.parents[0] ?? `${this.sha}^`; + private _resolvedPreviousSha: string | undefined; + get unresolvedPreviousSha(): string { + if (this._resolvedPreviousSha != null) return this._resolvedPreviousSha; + if (this.file != null) return this.file.previousSha ?? `${this.sha}^`; + return this.parents[0] ?? `${this.sha}^`; } @gate() @@ -189,6 +192,7 @@ export class GitCommit implements GitRevisionReference { // Check for any untracked files -- since git doesn't return them via `git stash list` :( // See https://stackoverflow.com/questions/12681529/ this.stashName ? this.container.git.getCommit(this.repoPath, `${this.stashName}^3`) : undefined, + this.getPreviousSha(), ]); if (commitResult.status !== 'fulfilled' || commitResult.value == null) return; @@ -403,7 +407,7 @@ export class GitCommit implements GitRevisionReference { return new GitUri(this._file?.originalUri ?? uri, { repoPath: this.repoPath, - sha: this.previousSha, + sha: this.unresolvedPreviousSha, }); } @@ -422,6 +426,40 @@ export class GitCommit implements GitRevisionReference { : Promise.resolve(undefined); } + private _previousShaPromise: Promise | undefined; + async getPreviousSha(): Promise { + if (this._previousShaPromise == null) { + async function getCore(this: GitCommit) { + if (this.file != null) { + if (this.file.previousSha != null && GitRevision.isSha(this.file.previousSha)) { + return this.file.previousSha; + } + + const sha = await this.container.git.resolveReference( + this.repoPath, + GitRevision.isUncommitted(this.sha, true) ? 'HEAD' : `${this.sha}^`, + this.file.originalPath ?? this.file.path, + ); + return sha; + } + + const parent = this.parents[0]; + if (parent != null && GitRevision.isSha(parent)) return parent; + + const sha = await this.container.git.resolveReference( + this.repoPath, + GitRevision.isUncommitted(this.sha, true) ? 'HEAD' : `${this.sha}^`, + ); + return sha; + } + + this._previousShaPromise = getCore.call(this).then(sha => (this._resolvedPreviousSha = sha)); + } + + return this._previousShaPromise; + } + + @gate() async isPushed(): Promise { return this.container.git.hasCommitBeenPushed(this.repoPath, this.ref); } diff --git a/src/hovers/hovers.ts b/src/hovers/hovers.ts index 9e6d5af..1838df2 100644 --- a/src/hovers/hovers.ts +++ b/src/hovers/hovers.ts @@ -20,6 +20,8 @@ export namespace Hovers { ): Promise { const documentRef = uri.sha; + let previousSha = null; + async function getDiff() { if (commit.file == null) return undefined; @@ -30,7 +32,8 @@ export namespace Hovers { ref = documentRef; } } else { - ref = commit.file.previousSha; + previousSha = await commit.getPreviousSha(); + ref = previousSha; if (ref == null) { return `\`\`\`diff\n+ ${document.lineAt(editorLine).text}\n\`\`\``; } @@ -115,7 +118,9 @@ export namespace Hovers { editorLine, )} "Open Changes")`; - const previousSha = commit.file?.previousSha; + if (previousSha === null) { + previousSha = await commit.getPreviousSha(); + } if (previousSha) { previous = `  [$(git-commit) ${GitRevision.shorten( previousSha, diff --git a/src/premium/github/githubGitProvider.ts b/src/premium/github/githubGitProvider.ts index 605a7fd..2148f61 100644 --- a/src/premium/github/githubGitProvider.ts +++ b/src/premium/github/githubGitProvider.ts @@ -2151,15 +2151,21 @@ export class GitHubGitProvider implements GitProvider, Disposable { @log() async resolveReference(repoPath: string, ref: string, pathOrUri?: string | Uri, _options?: { timeout?: number }) { - if (!ref || ref === GitRevision.deletedOrMissing || GitRevision.isUncommitted(ref)) { + if ( + !ref || + ref === GitRevision.deletedOrMissing || + (pathOrUri == null && GitRevision.isSha(ref)) || + (pathOrUri != null && GitRevision.isUncommitted(ref)) + ) { return ref; } let relativePath; - if (pathOrUri == null) { - if (GitRevision.isSha(ref) || !GitRevision.isShaLike(ref) || ref.endsWith('^3')) return ref; - } else { + if (pathOrUri != null) { relativePath = this.getRelativePath(pathOrUri, repoPath); + } else if (!GitRevision.isShaLike(ref) || ref.endsWith('^3')) { + // If it doesn't look like a sha at all (e.g. branch name) or is a stash ref (^3) don't try to resolve it + return ref; } const context = await this.ensureRepositoryContext(repoPath); diff --git a/src/views/nodes/branchTrackingStatusNode.ts b/src/views/nodes/branchTrackingStatusNode.ts index bdfcb11..ab82dc7 100644 --- a/src/views/nodes/branchTrackingStatusNode.ts +++ b/src/views/nodes/branchTrackingStatusNode.ts @@ -80,7 +80,8 @@ export class BranchTrackingStatusNode extends ViewNode impleme // Since the last commit when we are looking 'ahead' can have no previous (because of the range given) -- look it up commits = [...log.commits.values()]; const commit = commits[commits.length - 1]; - if (commit.previousSha == null) { + const previousSha = await commit.getPreviousSha(); + if (previousSha == null) { const previousLog = await this.view.container.git.getLog(this.uri.repoPath!, { limit: 2, ref: commit.sha, diff --git a/src/views/viewCommands.ts b/src/views/viewCommands.ts index 6671789..2f3b3b3 100644 --- a/src/views/viewCommands.ts +++ b/src/views/viewCommands.ts @@ -1063,7 +1063,7 @@ export class ViewCommands { } @debug() - private openRevision( + private async openRevision( node: | CommitFileNode | FileRevisionAsCommitNode @@ -1094,7 +1094,7 @@ export class ViewCommands { uri = node.commit.file?.status === 'D' ? Container.instance.git.getRevisionUri( - node.commit.previousSha, + (await node.commit.getPreviousSha()) ?? GitRevision.deletedOrMissing, node.commit.file.path, node.commit.repoPath, )