From e04a58db9c2b877e5cc94c37305bcb0f35042303 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Sat, 5 Feb 2022 17:47:22 -0500 Subject: [PATCH] Combines blame file vs contents Providers can now choose how to handle editor contents blaming Simplies "commit" line model to just have line & originalLine - Not really accurate for log parsing, but close enough Adds forces line blame to improve performance --- src/annotations/blameAnnotationProvider.ts | 8 +- .../gutterBlameAnnotationProvider.ts | 6 +- .../gutterChangesAnnotationProvider.ts | 7 +- .../gutterHeatmapBlameAnnotationProvider.ts | 2 +- src/codelens/codeLensProvider.ts | 10 +- src/commands/copyMessageToClipboard.ts | 8 +- src/commands/copyShaToClipboard.ts | 4 +- src/commands/diffLineWithWorking.ts | 6 +- src/commands/openCommitOnRemote.ts | 4 +- src/env/node/git/localGitProvider.ts | 34 ++-- src/git/gitProvider.ts | 9 +- src/git/gitProviderService.ts | 16 +- src/git/models/commit.ts | 10 +- src/git/parsers/blameParser.ts | 6 +- src/git/parsers/logParser.ts | 12 +- src/hovers/hovers.ts | 4 +- src/hovers/lineHoverController.ts | 4 +- src/premium/github/githubGitProvider.ts | 146 +++++++++++++----- src/statusbar/statusBarController.ts | 2 +- src/trackers/gitLineTracker.ts | 22 ++- src/views/nodes/commitFileNode.ts | 2 +- src/views/nodes/fileRevisionAsCommitNode.ts | 2 +- src/views/nodes/lineHistoryNode.ts | 6 +- 23 files changed, 184 insertions(+), 146 deletions(-) diff --git a/src/annotations/blameAnnotationProvider.ts b/src/annotations/blameAnnotationProvider.ts index 025c32b42a367..f28cf80d1293f 100644 --- a/src/annotations/blameAnnotationProvider.ts +++ b/src/annotations/blameAnnotationProvider.ts @@ -21,9 +21,7 @@ export abstract class BlameAnnotationProviderBase extends AnnotationProviderBase ) { super(annotationType, editor, trackedDocument); - this.blame = editor.document.isDirty - ? this.container.git.getBlameForFileContents(this.trackedDocument.uri, editor.document.getText()) - : this.container.git.getBlameForFile(this.trackedDocument.uri); + this.blame = this.container.git.getBlame(this.trackedDocument.uri, editor.document); if (editor.document.isDirty) { trackedDocument.setForceDirtyStateChangeOnNextDocumentChange(); @@ -174,8 +172,8 @@ export abstract class BlameAnnotationProviderBase extends AnnotationProviderBase private async getDetailsHoverMessage(commit: GitCommit, document: TextDocument) { let editorLine = this.editor.selection.active.line; const line = editorLine + 1; - const commitLine = commit.lines.find(l => l.to.line === line) ?? commit.lines[0]; - editorLine = commitLine.from.line - 1; + const commitLine = commit.lines.find(l => l.line === line) ?? commit.lines[0]; + editorLine = commitLine.originalLine - 1; return Hovers.detailsMessage( commit, diff --git a/src/annotations/gutterBlameAnnotationProvider.ts b/src/annotations/gutterBlameAnnotationProvider.ts index 094aec7a3b137..fdcdbf6d22dfa 100644 --- a/src/annotations/gutterBlameAnnotationProvider.ts +++ b/src/annotations/gutterBlameAnnotationProvider.ts @@ -89,7 +89,7 @@ export class GutterBlameAnnotationProvider extends BlameAnnotationProviderBase { for (const l of blame.lines) { // editor lines are 0-based - const editorLine = l.to.line - 1; + const editorLine = l.line - 1; if (previousSha === l.sha) { if (gutter == null) continue; @@ -202,9 +202,7 @@ export class GutterBlameAnnotationProvider extends BlameAnnotationProviderBase { const highlightDecorationRanges = Arrays.filterMap(blame.lines, l => l.sha === sha ? // editor lines are 0-based - this.editor.document.validateRange( - new Range(l.to.line - 1, 0, l.to.line - 1, Number.MAX_SAFE_INTEGER), - ) + this.editor.document.validateRange(new Range(l.line - 1, 0, l.line - 1, Number.MAX_SAFE_INTEGER)) : undefined, ); diff --git a/src/annotations/gutterChangesAnnotationProvider.ts b/src/annotations/gutterChangesAnnotationProvider.ts index af5de366d618b..6ff829d4d48ff 100644 --- a/src/annotations/gutterChangesAnnotationProvider.ts +++ b/src/annotations/gutterChangesAnnotationProvider.ts @@ -166,12 +166,7 @@ export class GutterChangesAnnotationProvider extends AnnotationProviderBase( BuiltInCommands.ExecuteDocumentSymbolProvider, document.uri, @@ -169,7 +165,7 @@ export class GitCodeLensProvider implements CodeLensProvider { ]); } - if (blame === undefined || blame.lines.length === 0) return lenses; + if (blame == null || blame?.lines.length === 0) return lenses; } else if (languageScope.scopes.length !== 1 || !languageScope.scopes.includes(CodeLensScopes.Document)) { symbols = await commands.executeCommand( BuiltInCommands.ExecuteDocumentSymbolProvider, diff --git a/src/commands/copyMessageToClipboard.ts b/src/commands/copyMessageToClipboard.ts index 5901b4a4cd2b4..4ea26ebddbf26 100644 --- a/src/commands/copyMessageToClipboard.ts +++ b/src/commands/copyMessageToClipboard.ts @@ -73,13 +73,7 @@ export class CopyMessageToClipboardCommand extends ActiveEditorCommand { if (blameline < 0) return; try { - const blame = editor?.document.isDirty - ? await this.container.git.getBlameForLineContents( - gitUri, - blameline, - editor.document.getText(), - ) - : await this.container.git.getBlameForLine(gitUri, blameline); + const blame = await this.container.git.getBlameForLine(gitUri, blameline, editor?.document); if (blame == null || blame.commit.isUncommitted) return; void (await GitActions.Commit.copyMessageToClipboard(blame.commit)); diff --git a/src/commands/copyShaToClipboard.ts b/src/commands/copyShaToClipboard.ts index 5f24ca233ad0a..121d85be47469 100644 --- a/src/commands/copyShaToClipboard.ts +++ b/src/commands/copyShaToClipboard.ts @@ -65,9 +65,7 @@ export class CopyShaToClipboardCommand extends ActiveEditorCommand { try { const gitUri = await GitUri.fromUri(uri); - const blame = editor?.document.isDirty - ? await this.container.git.getBlameForLineContents(gitUri, blameline, editor.document.getText()) - : await this.container.git.getBlameForLine(gitUri, blameline); + const blame = await this.container.git.getBlameForLine(gitUri, blameline, editor?.document); if (blame == null) return; args.sha = blame.commit.sha; diff --git a/src/commands/diffLineWithWorking.ts b/src/commands/diffLineWithWorking.ts index 2d29a2df1ba7a..553f87c3ad7b2 100644 --- a/src/commands/diffLineWithWorking.ts +++ b/src/commands/diffLineWithWorking.ts @@ -39,9 +39,7 @@ export class DiffLineWithWorkingCommand extends ActiveEditorCommand { if (blameline < 0) return; try { - const blame = editor?.document.isDirty - ? await this.container.git.getBlameForLineContents(gitUri, blameline, editor.document.getText()) - : await this.container.git.getBlameForLine(gitUri, blameline); + const blame = await this.container.git.getBlameForLine(gitUri, blameline, editor?.document); if (blame == null) { void Messages.showFileNotUnderSourceControlWarningMessage('Unable to open compare'); @@ -68,7 +66,7 @@ export class DiffLineWithWorkingCommand extends ActiveEditorCommand { lhsUri = args.commit.file!.uri; } // editor lines are 0-based - args.line = blame.line.to.line - 1; + args.line = blame.line.line - 1; } catch (ex) { Logger.error(ex, 'DiffLineWithWorkingCommand', `getBlameForLine(${blameline})`); void Messages.showGenericErrorMessage('Unable to open compare'); diff --git a/src/commands/openCommitOnRemote.ts b/src/commands/openCommitOnRemote.ts index 7f495370f1683..c6ae4818cb4e2 100644 --- a/src/commands/openCommitOnRemote.ts +++ b/src/commands/openCommitOnRemote.ts @@ -71,9 +71,7 @@ export class OpenCommitOnRemoteCommand extends ActiveEditorCommand { const blameline = editor == null ? 0 : editor.selection.active.line; if (blameline < 0) return; - const blame = editor?.document.isDirty - ? await this.container.git.getBlameForLineContents(gitUri, blameline, editor.document.getText()) - : await this.container.git.getBlameForLine(gitUri, blameline); + const blame = await this.container.git.getBlameForLine(gitUri, blameline, editor?.document); if (blame == null) { void Messages.showFileNotUnderSourceControlWarningMessage( 'Unable to open commit on remote provider', diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 772acbcba89b2..9c9281d6d53d4 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -9,6 +9,7 @@ import { extensions, FileType, Range, + TextDocument, Uri, window, workspace, @@ -905,9 +906,11 @@ export class LocalGitProvider implements GitProvider, Disposable { @gate() @log() - async getBlameForFile(uri: GitUri): Promise { + async getBlame(uri: GitUri, document?: TextDocument | undefined): Promise { const cc = Logger.getCorrelationContext(); + if (document?.isDirty) return this.getBlameContents(uri, document.getText()); + let key = 'blame'; if (uri.sha != null) { key += `:${uri.sha}`; @@ -930,7 +933,7 @@ export class LocalGitProvider implements GitProvider, Disposable { } } - const promise = this.getBlameForFileCore(uri, doc, key, cc); + const promise = this.getBlameCore(uri, doc, key, cc); if (doc.state != null) { Logger.debug(cc, `Cache add: '${key}'`); @@ -944,7 +947,7 @@ export class LocalGitProvider implements GitProvider, Disposable { return promise; } - private async getBlameForFileCore( + private async getBlameCore( uri: GitUri, document: TrackedDocument, key: string, @@ -986,8 +989,8 @@ export class LocalGitProvider implements GitProvider, Disposable { } } - @log({ args: { 1: '' } }) - async getBlameForFileContents(uri: GitUri, contents: string): Promise { + @log({ args: { 1: '' } }) + async getBlameContents(uri: GitUri, contents: string): Promise { const cc = Logger.getCorrelationContext(); const key = `blame:${md5(contents)}`; @@ -1009,7 +1012,7 @@ export class LocalGitProvider implements GitProvider, Disposable { } } - const promise = this.getBlameForFileContentsCore(uri, contents, doc, key, cc); + const promise = this.getBlameContentsCore(uri, contents, doc, key, cc); if (doc.state != null) { Logger.debug(cc, `Cache add: '${key}'`); @@ -1023,7 +1026,7 @@ export class LocalGitProvider implements GitProvider, Disposable { return promise; } - private async getBlameForFileContentsCore( + private async getBlameContentsCore( uri: GitUri, contents: string, document: TrackedDocument, @@ -1071,10 +1074,13 @@ export class LocalGitProvider implements GitProvider, Disposable { async getBlameForLine( uri: GitUri, editorLine: number, + document?: TextDocument | undefined, options?: { forceSingleLine?: boolean }, ): Promise { + if (document?.isDirty) return this.getBlameForLineContents(uri, editorLine, document.getText(), options); + if (!options?.forceSingleLine && this.useCaching) { - const blame = await this.getBlameForFile(uri); + const blame = await this.getBlame(uri); if (blame == null) return undefined; let blameLine = blame.lines[editorLine]; @@ -1125,7 +1131,7 @@ export class LocalGitProvider implements GitProvider, Disposable { options?: { forceSingleLine?: boolean }, ): Promise { if (!options?.forceSingleLine && this.useCaching) { - const blame = await this.getBlameForFileContents(uri, contents); + const blame = await this.getBlameContents(uri, contents); if (blame == null) return undefined; let blameLine = blame.lines[editorLine]; @@ -1170,7 +1176,7 @@ export class LocalGitProvider implements GitProvider, Disposable { @log() async getBlameForRange(uri: GitUri, range: Range): Promise { - const blame = await this.getBlameForFile(uri); + const blame = await this.getBlame(uri); if (blame == null) return undefined; return this.getBlameRange(blame, uri, range); @@ -1178,7 +1184,7 @@ export class LocalGitProvider implements GitProvider, Disposable { @log({ args: { 2: '' } }) async getBlameForRangeContents(uri: GitUri, range: Range, contents: string): Promise { - const blame = await this.getBlameForFileContents(uri, contents); + const blame = await this.getBlameContents(uri, contents); if (blame == null) return undefined; return this.getBlameRange(blame, uri, range); @@ -1205,7 +1211,7 @@ export class LocalGitProvider implements GitProvider, Disposable { if (!shas.has(c.sha)) continue; const commit = c.with({ - lines: c.lines.filter(l => l.to.line >= startLine && l.to.line <= endLine), + lines: c.lines.filter(l => l.line >= startLine && l.line <= endLine), }); commits.set(c.sha, commit); @@ -2915,7 +2921,7 @@ export class LocalGitProvider implements GitProvider, Disposable { ref = blameLine.commit.sha; relativePath = blameLine.commit.file?.path ?? blameLine.commit.file?.originalPath ?? relativePath; uri = this.getAbsoluteUri(relativePath, repoPath); - editorLine = blameLine.line.from.line - 1; + editorLine = blameLine.line.originalLine - 1; if (skip === 0 && blameLine.commit.file?.previousSha) { previous = GitUri.fromFile(relativePath, repoPath, blameLine.commit.file.previousSha); @@ -2944,7 +2950,7 @@ export class LocalGitProvider implements GitProvider, Disposable { ref = blameLine.commit.sha; relativePath = blameLine.commit.file?.path ?? blameLine.commit.file?.originalPath ?? relativePath; uri = this.getAbsoluteUri(relativePath, repoPath); - editorLine = blameLine.line.from.line - 1; + editorLine = blameLine.line.originalLine - 1; if (skip === 0 && blameLine.commit.file?.previousSha) { previous = GitUri.fromFile(relativePath, repoPath, blameLine.commit.file.previousSha); diff --git a/src/git/gitProvider.ts b/src/git/gitProvider.ts index 2bbb221fefcf9..db80e7ba2158d 100644 --- a/src/git/gitProvider.ts +++ b/src/git/gitProvider.ts @@ -1,4 +1,4 @@ -import { Disposable, Event, Range, Uri, WorkspaceFolder } from 'vscode'; +import { Disposable, Event, Range, TextDocument, Uri, WorkspaceFolder } from 'vscode'; import { Commit, InputBox } from '../@types/vscode.git'; import { GitUri } from './gitUri'; import { @@ -129,23 +129,26 @@ export interface GitProvider extends Disposable { /** * Returns the blame of a file * @param uri Uri of the file to blame + * @param document Optional TextDocument to blame the contents of if dirty */ - getBlameForFile(uri: GitUri): Promise; + getBlame(uri: GitUri, document?: TextDocument | undefined): Promise; /** * Returns the blame of a file, using the editor contents (for dirty editors) * @param uri Uri of the file to blame * @param contents Contents from the editor to use */ - getBlameForFileContents(uri: GitUri, contents: string): Promise; + getBlameContents(uri: GitUri, contents: string): Promise; /** * Returns the blame of a single line * @param uri Uri of the file to blame * @param editorLine Editor line number (0-based) to blame (Git is 1-based) + * @param document Optional TextDocument to blame the contents of if dirty * @param options.forceSingleLine Forces blame to be for the single line (rather than the whole file) */ getBlameForLine( uri: GitUri, editorLine: number, + document?: TextDocument | undefined, options?: { forceSingleLine?: boolean }, ): Promise; /** diff --git a/src/git/gitProviderService.ts b/src/git/gitProviderService.ts index b798146661e26..194d5cb3a48a5 100644 --- a/src/git/gitProviderService.ts +++ b/src/git/gitProviderService.ts @@ -6,6 +6,7 @@ import { EventEmitter, ProgressLocation, Range, + TextDocument, TextEditor, Uri, window, @@ -849,21 +850,22 @@ export class GitProviderService implements Disposable { /** * Returns the blame of a file * @param uri Uri of the file to blame + * @param document Optional TextDocument to blame the contents of if dirty */ - async getBlameForFile(uri: GitUri): Promise { + async getBlame(uri: GitUri, document?: TextDocument | undefined): Promise { const { provider } = this.getProvider(uri); - return provider.getBlameForFile(uri); + return provider.getBlame(uri, document); } - @log({ args: { 1: '' } }) + @log({ args: { 1: '' } }) /** * Returns the blame of a file, using the editor contents (for dirty editors) * @param uri Uri of the file to blame * @param contents Contents from the editor to use */ - async getBlameForFileContents(uri: GitUri, contents: string): Promise { + async getBlameContents(uri: GitUri, contents: string): Promise { const { provider } = this.getProvider(uri); - return provider.getBlameForFileContents(uri, contents); + return provider.getBlameContents(uri, contents); } @log() @@ -871,15 +873,17 @@ export class GitProviderService implements Disposable { * Returns the blame of a single line * @param uri Uri of the file to blame * @param editorLine Editor line number (0-based) to blame (Git is 1-based) + * @param document Optional TextDocument to blame the contents of if dirty * @param options.forceSingleLine Forces blame to be for the single line (rather than the whole file) */ async getBlameForLine( uri: GitUri, editorLine: number, + document?: TextDocument | undefined, options?: { forceSingleLine?: boolean }, ): Promise { const { provider } = this.getProvider(uri); - return provider.getBlameForLine(uri, editorLine, options); + return provider.getBlameForLine(uri, editorLine, document, options); } @log({ args: { 2: '' } }) diff --git a/src/git/models/commit.ts b/src/git/models/commit.ts index 3dae7a2835cde..0516d18fa4b63 100644 --- a/src/git/models/commit.ts +++ b/src/git/models/commit.ts @@ -494,14 +494,8 @@ export class GitCommitIdentity { export interface GitCommitLine { sha: string; previousSha?: string | undefined; - from: { - line: number; - count: number; - }; - to: { - line: number; - count: number; - }; + originalLine: number; + line: number; } export interface GitCommitStats { diff --git a/src/git/parsers/blameParser.ts b/src/git/parsers/blameParser.ts index 26344d44518b2..19d57061d7a37 100644 --- a/src/git/parsers/blameParser.ts +++ b/src/git/parsers/blameParser.ts @@ -254,12 +254,12 @@ export class GitBlameParser { const line: GitCommitLine = { sha: entry.sha, previousSha: commit.file!.previousSha, - from: { line: entry.originalLine + i, count: 1 }, - to: { line: entry.line + i, count: 1 }, + originalLine: entry.originalLine + i, + line: entry.line + i, }; commit.lines.push(line); - lines[line.to.line - 1] = line; + lines[line.line - 1] = line; } } } diff --git a/src/git/parsers/logParser.ts b/src/git/parsers/logParser.ts index 11d6ea888e585..10a6f3d5a46cd 100644 --- a/src/git/parsers/logParser.ts +++ b/src/git/parsers/logParser.ts @@ -451,14 +451,10 @@ export class GitLogParser { if (match !== null) { entry.line = { sha: entry.sha!, - from: { - line: parseInt(match[1], 10), - count: parseInt(match[2], 10), - }, - to: { - line: parseInt(match[3], 10), - count: parseInt(match[4], 10), - }, + originalLine: parseInt(match[1], 10), + // count: parseInt(match[2], 10), + line: parseInt(match[3], 10), + // count: parseInt(match[4], 10), }; } diff --git a/src/hovers/hovers.ts b/src/hovers/hovers.ts index a7ceb0d9ac566..ffe4e5b1e2c30 100644 --- a/src/hovers/hovers.ts +++ b/src/hovers/hovers.ts @@ -43,7 +43,7 @@ export namespace Hovers { } const line = editorLine + 1; - const commitLine = commit.lines.find(l => l.to.line === line) ?? commit.lines[0]; + const commitLine = commit.lines.find(l => l.line === line) ?? commit.lines[0]; let originalPath = commit.file.originalPath; if (originalPath == null) { @@ -52,7 +52,7 @@ export namespace Hovers { } } - editorLine = commitLine.to.line - 1; + editorLine = commitLine.line - 1; // TODO: Doesn't work with dirty files -- pass in editor? or contents? let hunkLine = await Container.instance.git.getDiffForLine(uri, editorLine, ref, ref2); diff --git a/src/hovers/lineHoverController.ts b/src/hovers/lineHoverController.ts index 702a235ae9fb6..808d2b925266f 100644 --- a/src/hovers/lineHoverController.ts +++ b/src/hovers/lineHoverController.ts @@ -135,8 +135,8 @@ export class LineHoverController implements Disposable { let editorLine = position.line; const line = editorLine + 1; - const commitLine = commit.lines.find(l => l.to.line === line) ?? commit.lines[0]; - editorLine = commitLine.from.line - 1; + const commitLine = commit.lines.find(l => l.line === line) ?? commit.lines[0]; + editorLine = commitLine.originalLine - 1; const trackedDocument = await this.container.tracker.get(document); if (trackedDocument == null) return undefined; diff --git a/src/premium/github/githubGitProvider.ts b/src/premium/github/githubGitProvider.ts index cca5373a5ce59..4deb313a208f0 100644 --- a/src/premium/github/githubGitProvider.ts +++ b/src/premium/github/githubGitProvider.ts @@ -7,6 +7,7 @@ import { EventEmitter, FileType, Range, + TextDocument, Uri, window, workspace, @@ -344,9 +345,12 @@ export class GitHubGitProvider implements GitProvider, Disposable { @gate() @log() - async getBlameForFile(uri: GitUri): Promise { + async getBlame(uri: GitUri, document?: TextDocument | undefined): Promise { const cc = Logger.getCorrelationContext(); + // TODO@eamodio we need to figure out when to do this, since dirty isn't enough, we need to know if there are any uncommitted changes + if (document?.isDirty) return this.getBlameContents(uri, document.getText()); + let key = 'blame'; if (uri.sha != null) { key += `:${uri.sha}`; @@ -367,7 +371,7 @@ export class GitHubGitProvider implements GitProvider, Disposable { doc.state = new GitDocumentState(doc.key); } - const promise = this.getBlameForFileCore(uri, doc, key, cc); + const promise = this.getBlameCore(uri, doc, key, cc); if (doc.state != null) { Logger.debug(cc, `Cache add: '${key}'`); @@ -381,7 +385,7 @@ export class GitHubGitProvider implements GitProvider, Disposable { return promise; } - private async getBlameForFileCore( + private async getBlameCore( uri: GitUri, document: TrackedDocument, key: string, @@ -393,7 +397,7 @@ export class GitHubGitProvider implements GitProvider, Disposable { const { metadata, github, remotehub, session } = context; const root = remotehub.getVirtualUri(remotehub.getProviderRootUri(uri)); - const file = this.getRelativePath(uri, root); + const relativePath = this.getRelativePath(uri, root); // const sha = await this.resolveReferenceCore(uri.repoPath!, metadata, uri.sha); // if (sha == null) return undefined; @@ -404,7 +408,7 @@ export class GitHubGitProvider implements GitProvider, Disposable { metadata.repo.owner, metadata.repo.name, ref, - file, + relativePath, ); const authors = new Map(); @@ -440,7 +444,7 @@ export class GitHubGitProvider implements GitProvider, Disposable { c.message.split('\n', 1)[0], c.parents.nodes[0]?.oid ? [c.parents.nodes[0]?.oid] : [], c.message, - new GitFileChange(root.toString(), file, GitFileIndexStatus.Modified), + new GitFileChange(root.toString(), relativePath, GitFileIndexStatus.Modified), { changedFiles: c.changedFiles ?? 0, additions: c.additions ?? 0, deletions: c.deletions ?? 0 }, [], ); @@ -449,17 +453,7 @@ export class GitHubGitProvider implements GitProvider, Disposable { } for (let i = range.startingLine; i <= range.endingLine; i++) { - const line: GitCommitLine = { - sha: c.oid, - from: { - line: i, - count: 1, - }, - to: { - line: i, - count: 1, - }, - }; + const line: GitCommitLine = { sha: c.oid, originalLine: i, line: i }; commit.lines.push(line); lines[i - 1] = line; @@ -496,9 +490,10 @@ export class GitHubGitProvider implements GitProvider, Disposable { } } - @log({ args: { 1: '' } }) - async getBlameForFileContents(uri: GitUri, _contents: string): Promise { - return this.getBlameForFile(uri); + @log({ args: { 1: '' } }) + async getBlameContents(_uri: GitUri, _contents: string): Promise { + // TODO@eamodio figure out how to actually generate a blame given the contents (need to generate a diff) + return undefined; //this.getBlame(uri); } @gate() @@ -506,41 +501,110 @@ export class GitHubGitProvider implements GitProvider, Disposable { async getBlameForLine( uri: GitUri, editorLine: number, - _options?: { forceSingleLine?: boolean }, + document?: TextDocument | undefined, + options?: { forceSingleLine?: boolean }, ): Promise { - const blame = await this.getBlameForFile(uri); - if (blame == null) return undefined; + const cc = Logger.getCorrelationContext(); + + // TODO@eamodio we need to figure out when to do this, since dirty isn't enough, we need to know if there are any uncommitted changes + if (document?.isDirty) return this.getBlameForLineContents(uri, editorLine, document.getText(), options); + + if (!options?.forceSingleLine) { + const blame = await this.getBlame(uri); + if (blame == null) return undefined; + + let blameLine = blame.lines[editorLine]; + if (blameLine == null) { + if (blame.lines.length !== editorLine) return undefined; + blameLine = blame.lines[editorLine - 1]; + } + + const commit = blame.commits.get(blameLine.sha); + if (commit == null) return undefined; - let blameLine = blame.lines[editorLine]; - if (blameLine == null) { - if (blame.lines.length !== editorLine) return undefined; - blameLine = blame.lines[editorLine - 1]; + const author = blame.authors.get(commit.author.name)!; + return { + author: { ...author, lineCount: commit.lines.length }, + commit: commit, + line: blameLine, + }; } - const commit = blame.commits.get(blameLine.sha); - if (commit == null) return undefined; + try { + const context = await this.ensureRepositoryContext(uri.repoPath!); + if (context == null) return undefined; + const { metadata, github, remotehub, session } = context; - const author = blame.authors.get(commit.author.name)!; - return { - author: { ...author, lineCount: commit.lines.length }, - commit: commit, - line: blameLine, - }; + const root = remotehub.getVirtualUri(remotehub.getProviderRootUri(uri)); + const relativePath = this.getRelativePath(uri, root); + + const ref = uri.sha ?? (await metadata.getRevision()).revision; + const blame = await github.getBlame( + session?.accessToken, + metadata.repo.owner, + metadata.repo.name, + ref, + relativePath, + ); + + const range = blame.ranges.find(r => r.startingLine === editorLine); + if (range == null) return undefined; + + const c = range.commit; + + const { viewer = session.account.label } = blame; + const authorName = viewer != null && c.author.name === viewer ? 'You' : c.author.name; + const committerName = viewer != null && c.committer.name === viewer ? 'You' : c.committer.name; + + const commit = new GitCommit( + this.container, + uri.repoPath!, + c.oid, + new GitCommitIdentity(authorName, c.author.email, new Date(c.author.date), c.author.avatarUrl), + new GitCommitIdentity(committerName, c.committer.email, new Date(c.author.date)), + c.message.split('\n', 1)[0], + c.parents.nodes[0]?.oid ? [c.parents.nodes[0]?.oid] : [], + c.message, + new GitFileChange(root.toString(), relativePath, GitFileIndexStatus.Modified), + { changedFiles: c.changedFiles ?? 0, additions: c.additions ?? 0, deletions: c.deletions ?? 0 }, + [], + ); + + for (let i = range.startingLine; i <= range.endingLine; i++) { + const line: GitCommitLine = { sha: c.oid, originalLine: i, line: i }; + + commit.lines.push(line); + } + + return { + author: { + name: authorName, + lineCount: range.endingLine - range.startingLine + 1, + }, + commit: commit, + line: { sha: c.oid, originalLine: range.startingLine, line: editorLine }, + }; + } catch (ex) { + debugger; + Logger.error(cc, ex); + return undefined; + } } @log({ args: { 2: '' } }) async getBlameForLineContents( - uri: GitUri, - editorLine: number, + _uri: GitUri, + _editorLine: number, _contents: string, _options?: { forceSingleLine?: boolean }, ): Promise { - return this.getBlameForLine(uri, editorLine); + // TODO@eamodio figure out how to actually generate a blame given the contents (need to generate a diff) + return undefined; //this.getBlameForLine(uri, editorLine); } @log() async getBlameForRange(uri: GitUri, range: Range): Promise { - const blame = await this.getBlameForFile(uri); + const blame = await this.getBlame(uri); if (blame == null) return undefined; return this.getBlameRange(blame, uri, range); @@ -548,7 +612,7 @@ export class GitHubGitProvider implements GitProvider, Disposable { @log({ args: { 2: '' } }) async getBlameForRangeContents(uri: GitUri, range: Range, contents: string): Promise { - const blame = await this.getBlameForFileContents(uri, contents); + const blame = await this.getBlameContents(uri, contents); if (blame == null) return undefined; return this.getBlameRange(blame, uri, range); @@ -575,7 +639,7 @@ export class GitHubGitProvider implements GitProvider, Disposable { if (!shas.has(c.sha)) continue; const commit = c.with({ - lines: c.lines.filter(l => l.to.line >= startLine && l.to.line <= endLine), + lines: c.lines.filter(l => l.line >= startLine && l.line <= endLine), }); commits.set(c.sha, commit); diff --git a/src/statusbar/statusBarController.ts b/src/statusbar/statusBarController.ts index 79fa775fb7f94..b69aec67267f2 100644 --- a/src/statusbar/statusBarController.ts +++ b/src/statusbar/statusBarController.ts @@ -371,7 +371,7 @@ export class StatusBarController implements Disposable { const tooltip = await Hovers.detailsMessage( commit, commit.getGitUri(), - commit.lines[0].to.line, + commit.lines[0].line, this.container.config.statusBar.tooltipFormat, this.container.config.defaultDateFormat, { diff --git a/src/trackers/gitLineTracker.ts b/src/trackers/gitLineTracker.ts index 334e826aa90cc..525d76de7cf9d 100644 --- a/src/trackers/gitLineTracker.ts +++ b/src/trackers/gitLineTracker.ts @@ -148,14 +148,12 @@ export class GitLineTracker extends LineTracker { } if (selections.length === 1) { - const blameLine = editor.document.isDirty - ? await this.container.git.getBlameForLineContents( - trackedDocument.uri, - selections[0].active, - editor.document.getText(), - ) - : await this.container.git.getBlameForLine(trackedDocument.uri, selections[0].active); - if (blameLine === undefined) { + const blameLine = await this.container.git.getBlameForLine( + trackedDocument.uri, + selections[0].active, + editor?.document, + ); + if (blameLine == null) { if (cc != null) { cc.exitDetails = ` ${GlyphChars.Dot} blame failed`; } @@ -163,12 +161,10 @@ export class GitLineTracker extends LineTracker { return false; } - this.setState(blameLine.line.to.line - 1, new GitLineState(blameLine.commit)); + this.setState(blameLine.line.line - 1, new GitLineState(blameLine.commit)); } else { - const blame = editor.document.isDirty - ? await this.container.git.getBlameForFileContents(trackedDocument.uri, editor.document.getText()) - : await this.container.git.getBlameForFile(trackedDocument.uri); - if (blame === undefined) { + const blame = await this.container.git.getBlame(trackedDocument.uri, editor.document); + if (blame == null) { if (cc != null) { cc.exitDetails = ` ${GlyphChars.Dot} blame failed`; } diff --git a/src/views/nodes/commitFileNode.ts b/src/views/nodes/commitFileNode.ts index 401b700e9aa34..62374b2d8c81b 100644 --- a/src/views/nodes/commitFileNode.ts +++ b/src/views/nodes/commitFileNode.ts @@ -139,7 +139,7 @@ export class CommitFileNode