From e64c8bc4069d5c94223071228247fe16aad36324 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Mon, 11 Nov 2019 01:35:14 -0500 Subject: [PATCH] Reworks line tracking to avoid sync issues --- src/annotations/lineAnnotationController.ts | 10 ++-- src/hovers/lineHoverController.ts | 11 +++- src/statusbar/statusBarController.ts | 11 +++- src/trackers/gitLineTracker.ts | 80 +++++++++++------------------ src/trackers/lineTracker.ts | 77 +++++++++++++++++++++------ src/views/nodes/lineHistoryTrackerNode.ts | 21 +++++--- 6 files changed, 129 insertions(+), 81 deletions(-) diff --git a/src/annotations/lineAnnotationController.ts b/src/annotations/lineAnnotationController.ts index 85cbbc1..20eca9e 100644 --- a/src/annotations/lineAnnotationController.ts +++ b/src/annotations/lineAnnotationController.ts @@ -94,9 +94,9 @@ export class LineAnnotationController implements Disposable { @debug({ args: { 0: (e: LinesChangeEvent) => - `editor=${e.editor ? e.editor.document.uri.toString(true) : undefined}, lines=${ - e.lines ? e.lines.join(',') : undefined - }, pending=${Boolean(e.pending)}, reason=${e.reason}` + `editor=${e.editor?.document.uri.toString(true)}, lines=${e.lines?.join(',')}, pending=${Boolean( + e.pending + )}, reason=${e.reason}` } }) private onActiveLinesChanged(e: LinesChangeEvent) { @@ -113,7 +113,7 @@ export class LineAnnotationController implements Disposable { void this.refresh(window.activeTextEditor); } - @debug({ args: false }) + @debug({ args: false, singleLine: true }) clear(editor: TextEditor | undefined) { if (this._editor !== editor && this._editor !== undefined) { this.clearAnnotations(this._editor); @@ -235,7 +235,7 @@ export class LineAnnotationController implements Disposable { if (!Container.lineTracker.isSubscribed(this)) { Container.lineTracker.start( this, - Disposable.from(Container.lineTracker.onDidChangeActiveLines(this.onActiveLinesChanged, this)) + Container.lineTracker.onDidChangeActiveLines(this.onActiveLinesChanged, this) ); } diff --git a/src/hovers/lineHoverController.ts b/src/hovers/lineHoverController.ts index a68a0cf..ef87de8 100644 --- a/src/hovers/lineHoverController.ts +++ b/src/hovers/lineHoverController.ts @@ -15,6 +15,7 @@ import { Annotations } from '../annotations/annotations'; import { configuration } from '../configuration'; import { Container } from '../container'; import { LinesChangeEvent } from '../trackers/gitLineTracker'; +import { debug } from '../system'; export class LineHoverController implements Disposable { private _disposable: Disposable; @@ -43,7 +44,7 @@ export class LineHoverController implements Disposable { if (Container.config.hovers.enabled && Container.config.hovers.currentLine.enabled) { Container.lineTracker.start( this, - Disposable.from(Container.lineTracker.onDidChangeActiveLines(this.onActiveLinesChanged, this)) + Container.lineTracker.onDidChangeActiveLines(this.onActiveLinesChanged, this) ); this.register(window.activeTextEditor); @@ -53,6 +54,14 @@ export class LineHoverController implements Disposable { } } + @debug({ + args: { + 0: (e: LinesChangeEvent) => + `editor=${e.editor?.document.uri.toString(true)}, lines=${e.lines?.join(',')}, pending=${Boolean( + e.pending + )}, reason=${e.reason}` + } + }) private onActiveLinesChanged(e: LinesChangeEvent) { if (e.pending) return; diff --git a/src/statusbar/statusBarController.ts b/src/statusbar/statusBarController.ts index 5a14a6f..46b2e2d 100644 --- a/src/statusbar/statusBarController.ts +++ b/src/statusbar/statusBarController.ts @@ -6,6 +6,7 @@ import { isTextEditor } from '../constants'; import { Container } from '../container'; import { CommitFormatter, GitCommit } from '../git/gitService'; import { LinesChangeEvent } from '../trackers/gitLineTracker'; +import { debug } from '../system'; export class StatusBarController implements Disposable { private _blameStatusBarItem: StatusBarItem | undefined; @@ -80,7 +81,7 @@ export class StatusBarController implements Disposable { if (configuration.changed(e, 'statusBar', 'enabled')) { Container.lineTracker.start( this, - Disposable.from(Container.lineTracker.onDidChangeActiveLines(this.onActiveLinesChanged, this)) + Container.lineTracker.onDidChangeActiveLines(this.onActiveLinesChanged, this) ); } } else if (configuration.changed(e, 'statusBar', 'enabled')) { @@ -93,6 +94,14 @@ export class StatusBarController implements Disposable { } } + @debug({ + args: { + 0: (e: LinesChangeEvent) => + `editor=${e.editor?.document.uri.toString(true)}, lines=${e.lines?.join(',')}, pending=${Boolean( + e.pending + )}, reason=${e.reason}` + } + }) private onActiveLinesChanged(e: LinesChangeEvent) { // If we need to reduceFlicker, don't clear if only the selected lines changed let clear = !( diff --git a/src/trackers/gitLineTracker.ts b/src/trackers/gitLineTracker.ts index 320a5f5..6037d2d 100644 --- a/src/trackers/gitLineTracker.ts +++ b/src/trackers/gitLineTracker.ts @@ -9,6 +9,7 @@ import { GitDocumentState } from './gitDocumentTracker'; import { LinesChangeEvent, LineTracker } from './lineTracker'; +import { debug } from '../system'; export * from './lineTracker'; @@ -17,9 +18,6 @@ export class GitLineState { } export class GitLineTracker extends LineTracker { - private _count = 0; - private _subscriptions: Map = new Map(); - protected async fireLinesChanged(e: LinesChangeEvent) { this.reset(); @@ -31,10 +29,32 @@ export class GitLineTracker extends LineTracker { return super.fireLinesChanged(updated ? e : { ...e, lines: undefined }); } + protected onStart(): Disposable | undefined { + return Disposable.from( + Container.tracker.onDidChangeBlameState(this.onBlameStateChanged, this), + Container.tracker.onDidChangeDirtyState(this.onDirtyStateChanged, this), + Container.tracker.onDidTriggerDirtyIdle(this.onDirtyIdleTriggered, this) + ); + } + + @debug({ + args: { + 0: (e: DocumentBlameStateChangeEvent) => + `editor=${e.editor.document.uri.toString(true)}, doc=${e.document.uri.toString(true)}, blameable=${ + e.blameable + }` + } + }) private onBlameStateChanged(e: DocumentBlameStateChangeEvent) { this.trigger('editor'); } + @debug({ + args: { + 0: (e: DocumentDirtyIdleTriggerEvent) => + `editor=${e.editor.document.uri.toString(true)}, doc=${e.document.uri.toString(true)}` + } + }) private onDirtyIdleTriggered(e: DocumentDirtyIdleTriggerEvent) { const maxLines = Container.config.advanced.blame.sizeThresholdAfterEdit; if (maxLines > 0 && e.document.lineCount > maxLines) return; @@ -42,6 +62,12 @@ export class GitLineTracker extends LineTracker { this.resume(); } + @debug({ + args: { + 0: (e: DocumentDirtyStateChangeEvent) => + `editor=${e.editor.document.uri.toString(true)}, doc=${e.document.uri.toString(true)}, dirty=${e.dirty}` + } + }) private onDirtyStateChanged(e: DocumentDirtyStateChangeEvent) { if (e.dirty) { this.suspend(); @@ -52,6 +78,7 @@ export class GitLineTracker extends LineTracker { private _suspended = false; + @debug() private resume(options: { force?: boolean } = {}) { if (!options.force && !this._suspended) return; @@ -59,6 +86,7 @@ export class GitLineTracker extends LineTracker { this.trigger('editor'); } + @debug() private suspend(options: { force?: boolean } = {}) { if (!options.force && this._suspended) return; @@ -66,52 +94,6 @@ export class GitLineTracker extends LineTracker { this.trigger('editor'); } - isSubscribed(subscriber: any) { - return this._subscriptions.has(subscriber); - } - - start(subscriber: any, subscription: Disposable): Disposable { - const disposable = { - dispose: () => this.stop(subscriber) - }; - - if (this.isSubscribed(subscriber)) return disposable; - - this._subscriptions.set(subscriber, subscription); - - this._count++; - if (this._count === 1) { - super.start(); - - this._disposable = Disposable.from( - this._disposable!, - Container.tracker.onDidChangeBlameState(this.onBlameStateChanged, this), - Container.tracker.onDidChangeDirtyState(this.onDirtyStateChanged, this), - Container.tracker.onDidTriggerDirtyIdle(this.onDirtyIdleTriggered, this) - ); - } - - return disposable; - } - - stop(subscriber: any) { - const subscription = this._subscriptions.get(subscriber); - if (subscription === undefined) return; - - this._subscriptions.delete(subscriber); - subscription.dispose(); - - if (this._disposable === undefined) { - this._count = 0; - return; - } - - this._count--; - if (this._count === 0) { - super.stop(); - } - } - private async updateState(lines: number[], editor: TextEditor): Promise { const trackedDocument = await Container.tracker.getOrAdd(editor.document); if (!trackedDocument.isBlameable || !this.includesAll(lines)) return false; diff --git a/src/trackers/lineTracker.ts b/src/trackers/lineTracker.ts index b1d0c65..b0236f0 100644 --- a/src/trackers/lineTracker.ts +++ b/src/trackers/lineTracker.ts @@ -23,7 +23,9 @@ export class LineTracker implements Disposable { private readonly _state: Map = new Map(); dispose() { - this.stop(); + for (const subscriber of this._subscriptions.keys()) { + this.stop(subscriber); + } } private onActiveTextEditorChanged(editor: TextEditor | undefined) { @@ -32,7 +34,7 @@ export class LineTracker implements Disposable { this.reset(); this._editor = editor; - this._lines = editor !== undefined ? editor.selections.map(s => s.active.line) : undefined; + this._lines = editor?.selections.map(s => s.active.line); this.trigger('editor'); } @@ -41,15 +43,14 @@ export class LineTracker implements Disposable { // If this isn't for our cached editor and its not a real editor -- kick out if (this._editor !== e.textEditor && !isTextEditor(e.textEditor)) return; - const reason = this._editor === e.textEditor ? 'selection' : 'editor'; - const lines = e.selections.map(s => s.active.line); if (this._editor === e.textEditor && this.includesAll(lines)) return; this.reset(); this._editor = e.textEditor; this._lines = lines; - this.trigger(reason); + + this.trigger(this._editor === e.textEditor ? 'selection' : 'editor'); } getState(line: number): T | undefined { @@ -81,26 +82,63 @@ export class LineTracker implements Disposable { this._state.clear(); } - start(subscriber?: any, subscription?: Disposable): void { - if (this._disposable !== undefined) return; + private _subscriptions: Map = new Map(); + + isSubscribed(subscriber: any) { + return this._subscriptions.has(subscriber); + } + + protected onStart(): Disposable | undefined { + return undefined; + } + + start(subscriber: any, subscription: Disposable): Disposable { + const disposable = { + dispose: () => this.stop(subscriber) + }; + + const first = this._subscriptions.size === 0; - this._disposable = Disposable.from( - window.onDidChangeActiveTextEditor(Functions.debounce(this.onActiveTextEditorChanged, 0), this), - window.onDidChangeTextEditorSelection(this.onTextEditorSelectionChanged, this) - ); + let subs = this._subscriptions.get(subscriber); + if (subs === undefined) { + subs = [subscription]; + this._subscriptions.set(subscriber, subs); + } else { + subs.push(subscription); + } + + if (first) { + this._disposable = Disposable.from( + window.onDidChangeActiveTextEditor(Functions.debounce(this.onActiveTextEditorChanged, 0), this), + window.onDidChangeTextEditorSelection(this.onTextEditorSelectionChanged, this), + this.onStart() ?? { dispose: () => {} } + ); + + setImmediate(() => this.onActiveTextEditorChanged(window.activeTextEditor)); + } - setImmediate(() => this.onActiveTextEditorChanged(window.activeTextEditor)); + return disposable; } - stop(subscriber?: any) { - if (this._disposable === undefined) return; + stop(subscriber: any) { + const subs = this._subscriptions.get(subscriber); + if (subs === undefined) return; + + this._subscriptions.delete(subscriber); + for (const sub of subs) { + sub.dispose(); + } + + if (this._subscriptions.size !== 0) return; if (this._linesChangedDebounced !== undefined) { this._linesChangedDebounced.cancel(); } - this._disposable.dispose(); - this._disposable = undefined; + if (this._disposable !== undefined) { + this._disposable.dispose(); + this._disposable = undefined; + } } protected fireLinesChanged(e: LinesChangeEvent) { @@ -133,7 +171,12 @@ export class LineTracker implements Disposable { (e: LinesChangeEvent) => { if (window.activeTextEditor !== e.editor) return; // Make sure we are still on the same lines - if (!LineTracker.includesAll(e.lines, e.editor && e.editor.selections.map(s => s.active.line))) { + if ( + !LineTracker.includesAll( + e.lines, + e.editor?.selections.map(s => s.active.line) + ) + ) { return; } diff --git a/src/views/nodes/lineHistoryTrackerNode.ts b/src/views/nodes/lineHistoryTrackerNode.ts index 7b5fc4b..3859eb9 100644 --- a/src/views/nodes/lineHistoryTrackerNode.ts +++ b/src/views/nodes/lineHistoryTrackerNode.ts @@ -1,5 +1,5 @@ 'use strict'; -import { Disposable, Selection, TreeItem, TreeItemCollapsibleState, window } from 'vscode'; +import { Selection, TreeItem, TreeItemCollapsibleState, window } from 'vscode'; import { UriComparer } from '../../comparers'; import { GlyphChars } from '../../constants'; import { Container } from '../../container'; @@ -171,17 +171,22 @@ export class LineHistoryTrackerNode extends SubscribeableViewNode { - if (e.pending) return; + Container.lineTracker.onDidChangeActiveLines((e: LinesChangeEvent) => { + if (e.pending) return; - onActiveLinesChanged(e); - }) - ) + onActiveLinesChanged(e); + }) ); } - @debug({ args: false }) + @debug({ + args: { + 0: (e: LinesChangeEvent) => + `editor=${e.editor?.document.uri.toString(true)}, lines=${e.lines?.join(',')}, pending=${Boolean( + e.pending + )}, reason=${e.reason}` + } + }) private onActiveLinesChanged(e: LinesChangeEvent) { void this.triggerChange(); }