Adds get next comparison uris for GitHub

Adds get previous line comparison uris for GitHub
Improves perf of get previous comparison uris for GitHub
Replaces HEAD usage with proper revision for GitHub
Renames comparison uri methods for clarity

src/commands/diffLineWithPrevious.ts

@@ -32,7 +32,7 @@ export class DiffLineWithPreviousCommand extends ActiveEditorCommand {
 		const gitUri = args.commit?.getGitUri() ?? (await GitUri.fromUri(uri));
 
 		try {
-			const diffUris = await this.container.git.getPreviousLineDiffUris(
+			const diffUris = await this.container.git.getPreviousComparisonUrisForLine(
 				gitUri.repoPath!,
 				gitUri,
 				args.line,

src/commands/diffWithNext.ts

@@ -41,7 +41,7 @@ export class DiffWithNextCommand extends ActiveEditorCommand {
 
 		const gitUri = args.commit?.getGitUri() ?? (await GitUri.fromUri(uri));
 		try {
-			const diffUris = await this.container.git.getNextDiffUris(
+			const diffUris = await this.container.git.getNextComparisonUris(
 				gitUri.repoPath!,
 				gitUri,
 				gitUri.sha,

src/commands/diffWithPrevious.ts

@@ -82,7 +82,7 @@ export class DiffWithPreviousCommand extends ActiveEditorCommand {
 		// }
 
 		try {
-			const diffUris = await this.container.git.getPreviousDiffUris(
+			const diffUris = await this.container.git.getPreviousComparisonUris(
 				gitUri.repoPath!,
 				gitUri,
 				gitUri.sha,

src/commands/diffWithWorking.ts

@@ -37,7 +37,11 @@ export class DiffWithWorkingCommand extends ActiveEditorCommand {
 
 		if (args.inDiffRightEditor) {
 			try {
-				const diffUris = await this.container.git.getPreviousDiffUris(gitUri.repoPath!, gitUri, gitUri.sha, 0);
+				const diffUris = await this.container.git.getPreviousComparisonUris(
+					gitUri.repoPath!,
+					gitUri,
+					gitUri.sha,
+				);
 				gitUri = diffUris?.previous ?? gitUri;
 			} catch (ex) {
 				Logger.error(

src/commands/openFileAtRevision.ts

@@ -60,7 +60,7 @@ 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.getPreviousLineDiffUris(
+								const diffUris = await blame.commit.getPreviousComparisonUrisForLine(
 									gitUri,
 									editorLine,
 									gitUri.sha,

src/env/node/git/localGitProvider.ts

@@ -1073,7 +1073,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 	@log()
 	async getBlameForLine(
 		uri: GitUri,
-		editorLine: number,
+		editorLine: number, // 0-based, Git is 1-based
 		document?: TextDocument | undefined,
 		options?: { forceSingleLine?: boolean },
 	): Promise<GitBlameLine | undefined> {
@@ -1126,7 +1126,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 	@log<LocalGitProvider['getBlameForLineContents']>({ args: { 2: '<contents>' } })
 	async getBlameForLineContents(
 		uri: GitUri,
-		editorLine: number,
+		editorLine: number, // 0-based, Git is 1-based
 		contents: string,
 		options?: { forceSingleLine?: boolean },
 	): Promise<GitBlameLine | undefined> {
@@ -1831,7 +1831,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 	@log()
 	async getDiffForLine(
 		uri: GitUri,
-		editorLine: number,
+		editorLine: number, // 0-based, Git is 1-based
 		ref1: string | undefined,
 		ref2?: string,
 	): Promise<GitDiffHunkLine | undefined> {
@@ -2672,7 +2672,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 	}
 
 	@log()
-	async getNextDiffUris(
+	async getNextComparisonUris(
 		repoPath: string,
 		uri: Uri,
 		ref: string | undefined,
@@ -2719,7 +2719,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 	}
 
 	@log()
-	async getNextUri(
+	private async getNextUri(
 		repoPath: string,
 		uri: Uri,
 		ref?: string,
@@ -2775,7 +2775,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 	}
 
 	@log()
-	async getPreviousDiffUris(
+	async getPreviousComparisonUris(
 		repoPath: string,
 		uri: Uri,
 		ref: string | undefined,
@@ -2852,10 +2852,10 @@ export class LocalGitProvider implements GitProvider, Disposable {
 	}
 
 	@log()
-	async getPreviousLineDiffUris(
+	async getPreviousComparisonUrisForLine(
 		repoPath: string,
 		uri: Uri,
-		editorLine: number,
+		editorLine: number, // 0-based, Git is 1-based
 		ref: string | undefined,
 		skip: number = 0,
 	): Promise<{ current: GitUri; previous: GitUri | undefined; line: number } | undefined> {
@@ -2971,7 +2971,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 	}
 
 	@log()
-	async getPreviousUri(
+	private async getPreviousUri(
 		repoPath: string,
 		uri: Uri,
 		ref?: string,

src/git/gitProvider.ts

@@ -295,35 +295,26 @@ export interface GitProvider extends Disposable {
 	): Promise<string | undefined>;
 	getMergeStatus(repoPath: string): Promise<GitMergeStatus | undefined>;
 	getRebaseStatus(repoPath: string): Promise<GitRebaseStatus | undefined>;
-	getNextDiffUris(
+	getNextComparisonUris(
 		repoPath: string,
 		uri: Uri,
 		ref: string | undefined,
 		skip?: number,
 	): Promise<{ current: GitUri; next: GitUri | undefined; deleted?: boolean | undefined } | undefined>;
-	getNextUri(repoPath: string, uri: Uri, ref?: string, skip?: number): Promise<GitUri | undefined>;
-	getPreviousDiffUris(
+	getPreviousComparisonUris(
 		repoPath: string,
 		uri: Uri,
 		ref: string | undefined,
 		skip?: number,
 		firstParent?: boolean,
 	): Promise<{ current: GitUri; previous: GitUri | undefined } | undefined>;
-	getPreviousLineDiffUris(
+	getPreviousComparisonUrisForLine(
 		repoPath: string,
 		uri: Uri,
 		editorLine: number,
 		ref: string | undefined,
 		skip?: number,
 	): Promise<{ current: GitUri; previous: GitUri | undefined; line: number } | undefined>;
-	getPreviousUri(
-		repoPath: string,
-		uri: Uri,
-		ref?: string,
-		skip?: number,
-		editorLine?: number,
-		firstParent?: boolean,
-	): Promise<GitUri | undefined>;
 	getIncomingActivity(
 		repoPath: string,
 		options?: {

src/git/gitProviderService.ts

@@ -1264,75 +1264,44 @@ export class GitProviderService implements Disposable {
 	}
 
 	@log()
-	async getNextDiffUris(
+	getNextComparisonUris(
 		repoPath: string | Uri,
 		uri: Uri,
 		ref: string | undefined,
 		skip: number = 0,
 	): Promise<{ current: GitUri; next: GitUri | undefined; deleted?: boolean } | undefined> {
-		// If we have no ref (or staged ref) there is no next commit
-		if (ref == null || ref.length === 0) return undefined;
+		if (!ref) return Promise.resolve(undefined);
 
 		const { provider, path } = this.getProvider(repoPath);
-		return provider.getNextDiffUris(path, uri, ref, skip);
+		return provider.getNextComparisonUris(path, uri, ref, skip);
 	}
 
 	@log()
-	async getNextUri(
-		repoPath: string | Uri,
-		uri: Uri,
-		ref?: string,
-		skip: number = 0,
-		// editorLine?: number
-	): Promise<GitUri | undefined> {
-		// If we have no ref (or staged ref) there is no next commit
-		if (ref == null || ref.length === 0 || GitRevision.isUncommittedStaged(ref)) return undefined;
-
-		const { provider, path } = this.getProvider(repoPath);
-		return provider.getNextUri(path, uri, ref, skip);
-	}
-
-	@log()
-	async getPreviousDiffUris(
+	getPreviousComparisonUris(
 		repoPath: string | Uri,
 		uri: Uri,
 		ref: string | undefined,
 		skip: number = 0,
 		firstParent: boolean = false,
 	): Promise<{ current: GitUri; previous: GitUri | undefined } | undefined> {
-		if (ref === GitRevision.deletedOrMissing) return undefined;
+		if (ref === GitRevision.deletedOrMissing) return Promise.resolve(undefined);
 
 		const { provider, path } = this.getProvider(repoPath);
-		return provider.getPreviousDiffUris(path, uri, ref, skip, firstParent);
+		return provider.getPreviousComparisonUris(path, uri, ref, skip, firstParent);
 	}
 
 	@log()
-	async getPreviousLineDiffUris(
+	getPreviousComparisonUrisForLine(
 		repoPath: string | Uri,
 		uri: Uri,
 		editorLine: number,
 		ref: string | undefined,
 		skip: number = 0,
 	): Promise<{ current: GitUri; previous: GitUri | undefined; line: number } | undefined> {
-		if (ref === GitRevision.deletedOrMissing) return undefined;
-
-		const { provider, path } = this.getProvider(repoPath);
-		return provider.getPreviousLineDiffUris(path, uri, editorLine, ref, skip);
-	}
-
-	@log()
-	async getPreviousUri(
-		repoPath: string | Uri,
-		uri: Uri,
-		ref?: string,
-		skip: number = 0,
-		editorLine?: number,
-		firstParent: boolean = false,
-	): Promise<GitUri | undefined> {
-		if (ref === GitRevision.deletedOrMissing) return undefined;
+		if (ref === GitRevision.deletedOrMissing) return Promise.resolve(undefined);
 
 		const { provider, path } = this.getProvider(repoPath);
-		return provider.getPreviousUri(path, uri, ref, skip, editorLine, firstParent);
+		return provider.getPreviousComparisonUrisForLine(path, uri, editorLine, ref, skip);
 	}
 
 	async getPullRequestForBranch(

src/git/models/commit.ts

@@ -406,10 +406,10 @@ export class GitCommit implements GitRevisionReference {
 		});
 	}
 
-	@memoize<GitCommit['getPreviousLineDiffUris']>((u, e, r) => `${u.toString()}|${e}|${r ?? ''}`)
-	getPreviousLineDiffUris(uri: Uri, editorLine: number, ref: string | undefined) {
+	@memoize<GitCommit['getPreviousComparisonUrisForLine']>((u, e, r) => `${u.toString()}|${e}|${r ?? ''}`)
+	getPreviousComparisonUrisForLine(uri: Uri, editorLine: number, ref: string | undefined) {
 		return this.file?.path
-			? this.container.git.getPreviousLineDiffUris(this.repoPath, uri, editorLine, ref)
+			? this.container.git.getPreviousComparisonUrisForLine(this.repoPath, uri, editorLine, ref)
 			: Promise.resolve(undefined);
 	}
 
@@ -494,7 +494,9 @@ export class GitCommitIdentity {
 export interface GitCommitLine {
 	sha: string;
 	previousSha?: string | undefined;
+	/** The original (previous) line number prior to this commit; 1-based */
 	originalLine: number;
+	/** The current line number in this commit; 1-based */
 	line: number;
 }
 

src/hovers/hovers.ts

@@ -15,7 +15,7 @@ export namespace Hovers {
 	export async function changesMessage(
 		commit: GitCommit,
 		uri: GitUri,
-		editorLine: number,
+		editorLine: number, // 0-based, Git is 1-based
 		document: TextDocument,
 	): Promise<MarkdownString | undefined> {
 		const documentRef = uri.sha;
@@ -76,7 +76,7 @@ export namespace Hovers {
 		let previous;
 		let current;
 		if (commit.isUncommitted) {
-			const diffUris = await commit.getPreviousLineDiffUris(uri, editorLine, documentRef);
+			const diffUris = await commit.getPreviousComparisonUrisForLine(uri, editorLine, documentRef);
 			if (diffUris?.previous == null) return undefined;
 
 			message = `[$(compare-changes)](${DiffWithCommand.getMarkdownCommandArgs({
@@ -144,7 +144,7 @@ export namespace Hovers {
 	export async function localChangesMessage(
 		fromCommit: GitCommit | undefined,
 		uri: GitUri,
-		editorLine: number,
+		editorLine: number, // 0-based, Git is 1-based
 		hunk: GitDiffHunk,
 	): Promise<MarkdownString | undefined> {
 		const diff = getDiffFromHunk(hunk);
@@ -191,7 +191,7 @@ export namespace Hovers {
 	export async function detailsMessage(
 		commit: GitCommit,
 		uri: GitUri,
-		editorLine: number,
+		editorLine: number, // 0-based, Git is 1-based
 		format: string,
 		dateFormat: string | null,
 		options?: {
@@ -224,7 +224,7 @@ export namespace Hovers {
 		if (options?.cancellationToken?.isCancellationRequested) return new MarkdownString();
 
 		const [previousLineDiffUris, autolinkedIssuesOrPullRequests, pr, presence] = await Promise.all([
-			commit.isUncommitted ? commit.getPreviousLineDiffUris(uri, editorLine, uri.sha) : undefined,
+			commit.isUncommitted ? commit.getPreviousComparisonUrisForLine(uri, editorLine, uri.sha) : undefined,
 			getAutoLinkedIssuesOrPullRequests(message, remotes),
 			options?.pullRequests?.pr ??
 				getPullRequestForCommit(commit.ref, remotes, {

src/hovers/lineHoverController.ts

@@ -116,23 +116,6 @@ export class LineHoverController implements Disposable {
 		);
 		if (!wholeLine && range.start.character !== position.character) return undefined;
 
-		// // Get the full commit message -- since blame only returns the summary
-		// let logCommit = lineState?.logCommit;
-		// if (logCommit == null && !commit.isUncommitted) {
-		// 	logCommit = await this.container.git.getCommitForFile(commit.repoPath, commit.uri, {
-		// 		ref: commit.sha,
-		// 	});
-		// 	if (logCommit != null) {
-		// 		// Preserve the previous commit from the blame commit
-		// 		logCommit.previousSha = commit.previousSha;
-		// 		logCommit.previousFileName = commit.previousFileName;
-
-		// 		if (lineState != null) {
-		// 			lineState.logCommit = logCommit;
-		// 		}
-		// 	}
-		// }
-
 		let editorLine = position.line;
 		const line = editorLine + 1;
 		const commitLine = commit.lines.find(l => l.line === line) ?? commit.lines[0];