From e9db04f0c7d7df3b823c8351b3911ee6680f2fed Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Sat, 12 Nov 2016 16:30:07 -0500 Subject: [PATCH] Removes unnecessary subscriptions on invalid blame Removes duplicate lint rule Switches on-demand CodeLens to be a global toggle --- CHANGELOG.md | 1 + src/blameAnnotationController.ts | 27 +++++++++++++++------------ src/blameAnnotationProvider.ts | 26 +++++++++++++++++++------- src/blameStatusBarController.ts | 3 ++- src/git/git.ts | 11 +++++------ src/gitCodeLensProvider.ts | 7 +++---- src/gitProvider.ts | 38 +++++++++++--------------------------- tslint.json | 3 --- 8 files changed, 56 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f92dd73..89d3a32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Adds new `gitlens.diffLineWithWorking` command for line sensitive diffs - Adds `gitlens.diffWithPrevious` command to the explorer context menu - Adds output channel logging, controlled by the `gitlens.advanced.output.level` setting + - Switches on-demand CodeLens to be a global toggle (rather than per file) - Complete rewrite of the blame annotation provider to reduce overhead and provide better performance - Improves performance (significantly) when only showing CodeLens at the document level - Improves performance of status bar blame support diff --git a/src/blameAnnotationController.ts b/src/blameAnnotationController.ts index 436bb84..2a5d396 100644 --- a/src/blameAnnotationController.ts +++ b/src/blameAnnotationController.ts @@ -44,7 +44,20 @@ export default class BlameAnnotationController extends Disposable { } async showBlameAnnotation(editor: TextEditor, shaOrLine?: string | number): Promise { - if (!editor || !editor.document) return false; + if (!editor || !editor.document || editor.viewColumn === undefined) return false; + + const currentProvider = this._annotationProviders.get(editor.viewColumn); + if (currentProvider && TextEditorComparer.equals(currentProvider.editor, editor)) { + await currentProvider.setSelection(shaOrLine); + return true; + } + + const provider = new BlameAnnotationProvider(this.context, this.git, editor); + if (!await provider.supportsBlame()) return false; + + if (currentProvider) { + await this.clear(currentProvider.editor.viewColumn, false); + } if (!this._blameAnnotationsDisposable && this._annotationProviders.size === 0) { Logger.log(`Add listener registrations for blame annotations`); @@ -60,22 +73,12 @@ export default class BlameAnnotationController extends Disposable { this._visibleColumns = this._getVisibleColumns(window.visibleTextEditors); } - let provider = this._annotationProviders.get(editor.viewColumn); - if (provider) { - if (TextEditorComparer.equals(provider.editor, editor)) { - await provider.setSelection(shaOrLine); - return true; - } - await this.clear(provider.editor.viewColumn, false); - } - - provider = new BlameAnnotationProvider(this.context, this.git, editor); this._annotationProviders.set(editor.viewColumn, provider); return provider.provideBlameAnnotation(shaOrLine); } async toggleBlameAnnotation(editor: TextEditor, shaOrLine?: string | number): Promise { - if (!editor || !editor.document) return false; + if (!editor || !editor.document || editor.viewColumn === undefined) return false; let provider = this._annotationProviders.get(editor.viewColumn); if (!provider) return this.showBlameAnnotation(editor, shaOrLine); diff --git a/src/blameAnnotationProvider.ts b/src/blameAnnotationProvider.ts index 35fe59c..1bf3a43 100644 --- a/src/blameAnnotationProvider.ts +++ b/src/blameAnnotationProvider.ts @@ -88,6 +88,11 @@ export class BlameAnnotationProvider extends Disposable { return this.setSelection(e.selections[0].active.line); } + async supportsBlame(): Promise { + const blame = await this._blame; + return !!(blame && blame.lines.length); + } + async provideBlameAnnotation(shaOrLine?: string | number): Promise { const blame = await this._blame; if (!blame || !blame.lines.length) return false; @@ -168,7 +173,8 @@ export class BlameAnnotationProvider extends Disposable { let previous = blame.commits.get(commit.previousSha); if (previous) { hoverMessage = ['Uncommitted changes', `_${previous.sha}_ - ${previous.message}`, `${previous.author}, ${moment(previous.date).format('MMMM Do, YYYY h:MMa')}`]; - } else { + } + else { hoverMessage = ['Uncommitted changes', `_${l.previousSha}_`]; } } @@ -216,16 +222,19 @@ export class BlameAnnotationProvider extends Disposable { if (this._config.annotation.date) { if (width > 0) { width += 7; - } else { + } + else { width += 6; } } if (this._config.annotation.author) { if (width > 5 + 6) { width += 12; - } else if (width > 0) { + } + else if (width > 0) { width += 11; - } else { + } + else { width += 10; } } @@ -241,7 +250,8 @@ export class BlameAnnotationProvider extends Disposable { let previous = blame.commits.get(commit.previousSha); if (previous) { hoverMessage = ['Uncommitted changes', `_${previous.sha}_ - ${previous.message}`, `${previous.author}, ${moment(previous.date).format('MMMM Do, YYYY h:MMa')}`]; - } else { + } + else { hoverMessage = ['Uncommitted changes', `_${l.previousSha}_`]; } } @@ -274,9 +284,11 @@ export class BlameAnnotationProvider extends Disposable { const date = this._getDate(commit); if (this._config.annotation.sha) { return `${commit.sha.substring(0, 8)}${(date ? `\\00a0\\2022\\00a0 ${date}` : '')}${(author ? `\\00a0\\2022\\00a0 ${author}` : '')}`; - } else if (this._config.annotation.date) { + } + else if (this._config.annotation.date) { return `${date}${(author ? `\\00a0\\2022\\00a0 ${author}` : '')}`; - } else { + } + else { return author; } } diff --git a/src/blameStatusBarController.ts b/src/blameStatusBarController.ts index fdcbfe7..c6bcda8 100644 --- a/src/blameStatusBarController.ts +++ b/src/blameStatusBarController.ts @@ -64,7 +64,8 @@ export default class BlameStatusBarController extends Disposable { subscriptions.push(window.onDidChangeTextEditorSelection(this._onActiveSelectionChanged, this)); this._statusBarDisposable = Disposable.from(...subscriptions); - } else { + } + else { this._statusBarDisposable = undefined; this._statusBarItem = undefined; } diff --git a/src/git/git.ts b/src/git/git.ts index f4e60a6..700e518 100644 --- a/src/git/git.ts +++ b/src/git/git.ts @@ -23,7 +23,8 @@ async function gitCommand(cwd: string, ...args: any[]) { const msg = ex && ex.toString(); if (msg && (msg.includes('Not a git repository') || msg.includes('is outside repository') || msg.includes('no such path'))) { Logger.warn('git', ...args, ` cwd='${cwd}'`, msg && `\n ${msg.replace(/\r?\n|\r/g, ' ')}`); - } else { + } + else { Logger.error('git', ...args, ` cwd='${cwd}'`, msg && `\n ${msg.replace(/\r?\n|\r/g, ' ')}`); } throw ex; @@ -47,11 +48,9 @@ export default class Git { // Logger.error(`Git.splitPath(${fileName}) is not an absolute path!`); // debugger; // } - if (repoPath) { - return [fileName.replace(`${repoPath}/`, ''), repoPath]; - } else { - return [path.basename(fileName).replace(/\\/g, '/'), path.dirname(fileName).replace(/\\/g, '/')]; - } + if (repoPath) return [fileName.replace(`${repoPath}/`, ''), repoPath]; + + return [path.basename(fileName).replace(/\\/g, '/'), path.dirname(fileName).replace(/\\/g, '/')]; } static async repoPath(cwd: string, gitPath?: string) { diff --git a/src/gitCodeLensProvider.ts b/src/gitCodeLensProvider.ts index 715eff2..4655a5a 100644 --- a/src/gitCodeLensProvider.ts +++ b/src/gitCodeLensProvider.ts @@ -127,9 +127,7 @@ export default class GitCodeLensProvider implements CodeLensProvider { const line = document.lineAt(symbol.location.range.start); // Make sure there is only 1 lense per line - if (lenses.length && lenses[lenses.length - 1].range.start.line === line.lineNumber) { - return; - } + if (lenses.length && lenses[lenses.length - 1].range.start.line === line.lineNumber) return; let startChar = -1; try { @@ -138,7 +136,8 @@ export default class GitCodeLensProvider implements CodeLensProvider { catch (ex) { } if (startChar === -1) { startChar = line.firstNonWhitespaceCharacterIndex; - } else { + } + else { startChar += Math.floor(symbol.name.length / 2); } diff --git a/src/gitProvider.ts b/src/gitProvider.ts index 9fa85c3..07e4595 100644 --- a/src/gitProvider.ts +++ b/src/gitProvider.ts @@ -1,6 +1,6 @@ 'use strict'; import { Functions, Iterables, Objects } from './system'; -import { Disposable, DocumentFilter, ExtensionContext, languages, Location, Position, Range, TextDocument, TextEditor, Uri, window, workspace } from 'vscode'; +import { Disposable, ExtensionContext, languages, Location, Position, Range, TextDocument, TextEditor, Uri, workspace } from 'vscode'; import { CodeLensVisibility, IConfig } from './configuration'; import { DocumentSchemes, WorkspaceState } from './constants'; import Git, { GitBlameParserEnricher, GitBlameFormat, GitCommit, GitLogParserEnricher, IGitAuthor, IGitBlame, IGitBlameLine, IGitBlameLines, IGitLog } from './git/git'; @@ -45,7 +45,6 @@ export default class GitProvider extends Disposable { private _config: IConfig; private _disposable: Disposable; private _codeLensProviderDisposable: Disposable | undefined; - private _codeLensProviderSelector: DocumentFilter; private _gitignore: Promise; static EmptyPromise: Promise = Promise.resolve(undefined); @@ -103,9 +102,9 @@ export default class GitProvider extends Disposable { Logger.log('CodeLens config changed; resetting CodeLens provider'); this._codeLensProviderDisposable && this._codeLensProviderDisposable.dispose(); if (config.codeLens.visibility === CodeLensVisibility.Auto && (config.codeLens.recentChange.enabled || config.codeLens.authors.enabled)) { - this._codeLensProviderSelector = GitCodeLensProvider.selector; - this._codeLensProviderDisposable = languages.registerCodeLensProvider(this._codeLensProviderSelector, new GitCodeLensProvider(this.context, this)); - } else { + this._codeLensProviderDisposable = languages.registerCodeLensProvider(GitCodeLensProvider.selector, new GitCodeLensProvider(this.context, this)); + } + else { this._codeLensProviderDisposable = undefined; } } @@ -125,7 +124,8 @@ export default class GitProvider extends Disposable { disposables.push(workspace.onDidChangeTextDocument(e => removeCachedEntryFn(e.document, RemoveCacheReason.DocumentChanged))); this._cacheDisposable = Disposable.from(...disposables); - } else { + } + else { this._cacheDisposable && this._cacheDisposable.dispose(); this._cacheDisposable = undefined; this._cache && this._cache.clear(); @@ -437,34 +437,18 @@ export default class GitProvider extends Disposable { } toggleCodeLens(editor: TextEditor) { - Logger.log(`toggleCodeLens(${editor})`); - if (this._config.codeLens.visibility !== CodeLensVisibility.OnDemand || (!this._config.codeLens.recentChange.enabled && !this._config.codeLens.authors.enabled)) return; + Logger.log(`toggleCodeLens(${editor})`); + if (this._codeLensProviderDisposable) { this._codeLensProviderDisposable.dispose(); - - if (editor.document.fileName === (this._codeLensProviderSelector && this._codeLensProviderSelector.pattern)) { - this._codeLensProviderDisposable = undefined; - return; - } + this._codeLensProviderDisposable = undefined; + return; } - const disposables: Disposable[] = []; - - this._codeLensProviderSelector = { scheme: DocumentSchemes.File, pattern: editor.document.fileName }; - - disposables.push(languages.registerCodeLensProvider(this._codeLensProviderSelector, new GitCodeLensProvider(this.context, this))); - - disposables.push(window.onDidChangeActiveTextEditor(e => { - if (e.viewColumn && e.document !== editor.document) { - this._codeLensProviderDisposable && this._codeLensProviderDisposable.dispose(); - this._codeLensProviderDisposable = undefined; - } - })); - - this._codeLensProviderDisposable = Disposable.from(...disposables); + this._codeLensProviderDisposable = languages.registerCodeLensProvider(GitCodeLensProvider.selector, new GitCodeLensProvider(this.context, this)); } static isUncommitted(sha: string) { diff --git a/tslint.json b/tslint.json index 5871484..b460771 100644 --- a/tslint.json +++ b/tslint.json @@ -21,9 +21,6 @@ "no-unsafe-finally": true, "no-unused-expression": false, "no-unused-new": true, - "no-unused-variable": [ - true - ], "no-var-keyword": true, "no-var-requires": false, "object-literal-key-quotes": [