Reworks line tracking to avoid sync issues

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)
 				);
 			}
 

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;
 

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 = !(

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<GitLineState> {
-	private _count = 0;
-	private _subscriptions: Map<any, Disposable> = 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<GitDocumentState>) =>
+				`editor=${e.editor.document.uri.toString(true)}, doc=${e.document.uri.toString(true)}, blameable=${
+					e.blameable
+				}`
+		}
+	})
 	private onBlameStateChanged(e: DocumentBlameStateChangeEvent<GitDocumentState>) {
 		this.trigger('editor');
 	}
 
+	@debug({
+		args: {
+			0: (e: DocumentDirtyIdleTriggerEvent<GitDocumentState>) =>
+				`editor=${e.editor.document.uri.toString(true)}, doc=${e.document.uri.toString(true)}`
+		}
+	})
 	private onDirtyIdleTriggered(e: DocumentDirtyIdleTriggerEvent<GitDocumentState>) {
 		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<GitDocumentState>) =>
+				`editor=${e.editor.document.uri.toString(true)}, doc=${e.document.uri.toString(true)}, dirty=${e.dirty}`
+		}
+	})
 	private onDirtyStateChanged(e: DocumentDirtyStateChangeEvent<GitDocumentState>) {
 		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<boolean> {
 		const trackedDocument = await Container.tracker.getOrAdd(editor.document);
 		if (!trackedDocument.isBlameable || !this.includesAll(lines)) return false;

src/trackers/lineTracker.ts

@@ -23,7 +23,9 @@ export class LineTracker implements Disposable {
 	private readonly _state: Map<number, T | undefined> = 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<any, Disposable[]> = 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;
 					}
 

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
 
 		return Container.lineTracker.start(
 			this,
-			Disposable.from(
-				Container.lineTracker.onDidChangeActiveLines((e: LinesChangeEvent) => {
-					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();
 	}