Skip to content

Commit

Permalink
Combines blame file vs contents
Browse files Browse the repository at this point in the history
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
  • Loading branch information
eamodio committed Feb 5, 2022
1 parent aca5a81 commit e04a58d
Show file tree
Hide file tree
Showing 23 changed files with 184 additions and 146 deletions.
8 changes: 3 additions & 5 deletions src/annotations/blameAnnotationProvider.ts
Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 2 additions & 4 deletions src/annotations/gutterBlameAnnotationProvider.ts
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
);

Expand Down
7 changes: 1 addition & 6 deletions src/annotations/gutterChangesAnnotationProvider.ts
Expand Up @@ -166,12 +166,7 @@ export class GutterChangesAnnotationProvider extends AnnotationProviderBase<Chan
// If we want to only show changes from the specified sha, get the blame so we can compare with "visible" shas
const blame =
context?.sha != null && context?.only
? this.editor?.document.isDirty
? await this.container.git.getBlameForFileContents(
this.trackedDocument.uri,
this.editor.document.getText(),
)
: await this.container.git.getBlameForFile(this.trackedDocument.uri)
? await this.container.git.getBlame(this.trackedDocument.uri, this.editor?.document)
: undefined;

let selection: Selection | undefined;
Expand Down
2 changes: 1 addition & 1 deletion src/annotations/gutterHeatmapBlameAnnotationProvider.ts
Expand Up @@ -36,7 +36,7 @@ export class GutterHeatmapBlameAnnotationProvider extends BlameAnnotationProvide
let commit: GitCommit | undefined;
for (const l of blame.lines) {
// editor lines are 0-based
const editorLine = l.to.line - 1;
const editorLine = l.line - 1;

commit = blame.commits.get(l.sha);
if (commit == null) continue;
Expand Down
10 changes: 3 additions & 7 deletions src/codelens/codeLensProvider.ts
Expand Up @@ -154,22 +154,18 @@ export class GitCodeLensProvider implements CodeLensProvider {
if (token.isCancellationRequested) return lenses;

if (languageScope.scopes.length === 1 && languageScope.scopes.includes(CodeLensScopes.Document)) {
blame = document.isDirty
? await this.container.git.getBlameForFileContents(gitUri, document.getText())
: await this.container.git.getBlameForFile(gitUri);
blame = await this.container.git.getBlame(gitUri, document);
} else {
[blame, symbols] = await Promise.all([
document.isDirty
? this.container.git.getBlameForFileContents(gitUri, document.getText())
: this.container.git.getBlameForFile(gitUri),
this.container.git.getBlame(gitUri, document),
commands.executeCommand<SymbolInformation[]>(
BuiltInCommands.ExecuteDocumentSymbolProvider,
document.uri,
),
]);
}

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<SymbolInformation[]>(
BuiltInCommands.ExecuteDocumentSymbolProvider,
Expand Down
8 changes: 1 addition & 7 deletions src/commands/copyMessageToClipboard.ts
Expand Up @@ -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));
Expand Down
4 changes: 1 addition & 3 deletions src/commands/copyShaToClipboard.ts
Expand Up @@ -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;
Expand Down
6 changes: 2 additions & 4 deletions src/commands/diffLineWithWorking.ts
Expand Up @@ -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');

Expand All @@ -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');
Expand Down
4 changes: 1 addition & 3 deletions src/commands/openCommitOnRemote.ts
Expand Up @@ -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',
Expand Down
34 changes: 20 additions & 14 deletions src/env/node/git/localGitProvider.ts
Expand Up @@ -9,6 +9,7 @@ import {
extensions,
FileType,
Range,
TextDocument,
Uri,
window,
workspace,
Expand Down Expand Up @@ -905,9 +906,11 @@ export class LocalGitProvider implements GitProvider, Disposable {

@gate()
@log()
async getBlameForFile(uri: GitUri): Promise<GitBlame | undefined> {
async getBlame(uri: GitUri, document?: TextDocument | undefined): Promise<GitBlame | undefined> {
const cc = Logger.getCorrelationContext();

if (document?.isDirty) return this.getBlameContents(uri, document.getText());

let key = 'blame';
if (uri.sha != null) {
key += `:${uri.sha}`;
Expand All @@ -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}'`);
Expand All @@ -944,7 +947,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
return promise;
}

private async getBlameForFileCore(
private async getBlameCore(
uri: GitUri,
document: TrackedDocument<GitDocumentState>,
key: string,
Expand Down Expand Up @@ -986,8 +989,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
}
}

@log<LocalGitProvider['getBlameForFileContents']>({ args: { 1: '<contents>' } })
async getBlameForFileContents(uri: GitUri, contents: string): Promise<GitBlame | undefined> {
@log<LocalGitProvider['getBlameContents']>({ args: { 1: '<contents>' } })
async getBlameContents(uri: GitUri, contents: string): Promise<GitBlame | undefined> {
const cc = Logger.getCorrelationContext();

const key = `blame:${md5(contents)}`;
Expand All @@ -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}'`);
Expand All @@ -1023,7 +1026,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
return promise;
}

private async getBlameForFileContentsCore(
private async getBlameContentsCore(
uri: GitUri,
contents: string,
document: TrackedDocument<GitDocumentState>,
Expand Down Expand Up @@ -1071,10 +1074,13 @@ export class LocalGitProvider implements GitProvider, Disposable {
async getBlameForLine(
uri: GitUri,
editorLine: number,
document?: TextDocument | undefined,
options?: { forceSingleLine?: boolean },
): Promise<GitBlameLine | undefined> {
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];
Expand Down Expand Up @@ -1125,7 +1131,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
options?: { forceSingleLine?: boolean },
): Promise<GitBlameLine | undefined> {
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];
Expand Down Expand Up @@ -1170,15 +1176,15 @@ export class LocalGitProvider implements GitProvider, Disposable {

@log()
async getBlameForRange(uri: GitUri, range: Range): Promise<GitBlameLines | undefined> {
const blame = await this.getBlameForFile(uri);
const blame = await this.getBlame(uri);
if (blame == null) return undefined;

return this.getBlameRange(blame, uri, range);
}

@log<LocalGitProvider['getBlameForRangeContents']>({ args: { 2: '<contents>' } })
async getBlameForRangeContents(uri: GitUri, range: Range, contents: string): Promise<GitBlameLines | undefined> {
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);
Expand All @@ -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);

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 6 additions & 3 deletions 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 {
Expand Down Expand Up @@ -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<GitBlame | undefined>;
getBlame(uri: GitUri, document?: TextDocument | undefined): Promise<GitBlame | undefined>;
/**
* 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<GitBlame | undefined>;
getBlameContents(uri: GitUri, contents: string): Promise<GitBlame | undefined>;
/**
* 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<GitBlameLine | undefined>;
/**
Expand Down
16 changes: 10 additions & 6 deletions src/git/gitProviderService.ts
Expand Up @@ -6,6 +6,7 @@ import {
EventEmitter,
ProgressLocation,
Range,
TextDocument,
TextEditor,
Uri,
window,
Expand Down Expand Up @@ -849,37 +850,40 @@ 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<GitBlame | undefined> {
async getBlame(uri: GitUri, document?: TextDocument | undefined): Promise<GitBlame | undefined> {
const { provider } = this.getProvider(uri);
return provider.getBlameForFile(uri);
return provider.getBlame(uri, document);
}

@log<GitProviderService['getBlameForFileContents']>({ args: { 1: '<contents>' } })
@log<GitProviderService['getBlameContents']>({ args: { 1: '<contents>' } })
/**
* 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<GitBlame | undefined> {
async getBlameContents(uri: GitUri, contents: string): Promise<GitBlame | undefined> {
const { provider } = this.getProvider(uri);
return provider.getBlameForFileContents(uri, contents);
return provider.getBlameContents(uri, contents);
}

@log()
/**
* 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<GitBlameLine | undefined> {
const { provider } = this.getProvider(uri);
return provider.getBlameForLine(uri, editorLine, options);
return provider.getBlameForLine(uri, editorLine, document, options);
}

@log<GitProviderService['getBlameForLineContents']>({ args: { 2: '<contents>' } })
Expand Down
10 changes: 2 additions & 8 deletions src/git/models/commit.ts
Expand Up @@ -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 {
Expand Down

0 comments on commit e04a58d

Please sign in to comment.