Make pull diagnostics work for peek editors (#1703)

Author: dbaeumer

client/src/common/client.ts

@@ -11,7 +11,8 @@ import {
 	OnTypeFormattingEditProvider, RenameProvider, DocumentSymbolProvider, DocumentLinkProvider, DeclarationProvider, ImplementationProvider,
 	DocumentColorProvider, SelectionRangeProvider, TypeDefinitionProvider, CallHierarchyProvider, LinkedEditingRangeProvider, TypeHierarchyProvider, WorkspaceSymbolProvider,
 	ProviderResult, TextEdit as VTextEdit, InlineCompletionItemProvider, EventEmitter, type TabChangeEvent, TabInputText, TabInputTextDiff, TabInputCustom,
-	TabInputNotebook, type LogOutputChannel} from 'vscode';
+	TabInputNotebook, type LogOutputChannel, type TextEditor
+} from 'vscode';
 
 import {
 	RAL, Message, MessageSignature, Logger, ResponseError, RequestType0, RequestType, NotificationType0, NotificationType,
@@ -48,8 +49,7 @@ import * as UUID from './utils/uuid';
 import { ProgressPart } from './progressPart';
 import {
 	DynamicFeature, ensure, FeatureClient, LSPCancellationError, TextDocumentSendFeature, RegistrationData, StaticFeature,
-	TextDocumentProviderFeature, WorkspaceProviderFeature,
-	type TabsModel
+	TextDocumentProviderFeature, WorkspaceProviderFeature, type VisibleDocuments
 } from './features';
 
 import { DiagnosticFeature, DiagnosticProviderMiddleware, DiagnosticProviderShape, $DiagnosticPullOptions, DiagnosticFeatureShape } from './diagnostic';
@@ -498,25 +498,23 @@ export enum ShutdownMode {
  * diagnostics we need to de-dupe tabs that show the same resources since
  * we pull on the model not the UI.
  */
-class Tabs implements TabsModel {
+class VisibleDocumentsImpl implements VisibleDocuments {
 
 	private open: Set<string>;
 	private readonly _onOpen: EventEmitter<Set<Uri>>;
 	private readonly _onClose: EventEmitter<Set<Uri>>;
-	private readonly disposable: Disposable;
+	private readonly disposables: Disposable[];
 
 	constructor() {
+		this.disposables = [];
 		this.open = new Set();
 		this._onOpen = new EventEmitter();
 		this._onClose = new EventEmitter();
-		Tabs.fillTabResources(this.open);
-		const openTabsHandler = (event: TabChangeEvent) => {
-			if (event.closed.length === 0 && event.opened.length === 0) {
-				return;
-			}
+		VisibleDocumentsImpl.fillVisibleResources(this.open);
+		const updateVisibleDocuments = () => {
 			const oldTabs = this.open;
 			const currentTabs: Set<string> = new Set();
-			Tabs.fillTabResources(currentTabs);
+			VisibleDocumentsImpl.fillVisibleResources(currentTabs);
 
 			const closed: Set<string> = new Set();
 			const opened: Set<string> = new Set(currentTabs);
@@ -544,11 +542,16 @@ class Tabs implements TabsModel {
 			}
 		};
 
-		if (Window.tabGroups.onDidChangeTabs !== undefined) {
-			this.disposable = Window.tabGroups.onDidChangeTabs(openTabsHandler);
-		} else {
-			this.disposable = { dispose: () => {} };
-		}
+		this.disposables.push(Window.tabGroups.onDidChangeTabs((event: TabChangeEvent) => {
+			if (event.closed.length === 0 && event.opened.length === 0) {
+				return;
+			}
+			updateVisibleDocuments();
+		}));
+
+		this.disposables.push(Window.onDidChangeVisibleTextEditors((_editors: readonly TextEditor[]) => {
+			updateVisibleDocuments();
+		}));
 	}
 
 	public get onClose(): Event<Set<Uri>> {
@@ -560,7 +563,7 @@ class Tabs implements TabsModel {
 	}
 
 	public dispose(): void {
-		this.disposable.dispose();
+		this.disposables.forEach(disposable => disposable.dispose());
 	}
 
 	public isActive(document: TextDocument | Uri): boolean {
@@ -584,13 +587,13 @@ class Tabs implements TabsModel {
 		return this.open.has(uri.toString());
 	}
 
-	public getTabResources(): Set<Uri> {
+	public getResources(): Set<Uri> {
 		const result: Set<Uri> = new Set();
-		Tabs.fillTabResources(new Set(), result);
+		VisibleDocumentsImpl.fillVisibleResources(new Set(), result);
 		return result;
 	}
 
-	private static fillTabResources(strings: Set<string> | undefined, uris?: Set<Uri>): void {
+	private static fillVisibleResources(strings: Set<string> | undefined, uris?: Set<Uri>): void {
 		const seen = strings ?? new Set();
 		for (const group of Window.tabGroups.all) {
 			for (const tab of group.tabs) {
@@ -611,6 +614,14 @@ class Tabs implements TabsModel {
 				}
 			}
 		}
+		// Peek editors are not part of tabs but should be considered visible
+		for (const editor of Window.visibleTextEditors) {
+			const uri = editor.document.uri;
+			if (!seen.has(uri.toString())) {
+				seen.add(uri.toString());
+				uris !== undefined && uris.add(uri);
+			}
+		}
 	}
 }
 
@@ -668,7 +679,7 @@ export abstract class BaseLanguageClient implements FeatureClient<Middleware, La
 
 	private readonly _c2p: c2p.Converter;
 	private readonly _p2c: p2c.Converter;
-	private _tabsModel: TabsModel | undefined;
+	private _visibleDocuments: VisibleDocuments | undefined;
 
 	public constructor(id: string, name: string, clientOptions: LanguageClientOptions) {
 		this._id = id;
@@ -807,11 +818,11 @@ export abstract class BaseLanguageClient implements FeatureClient<Middleware, La
 		return this._c2p;
 	}
 
-	public get tabsModel(): TabsModel {
-		if (this._tabsModel === undefined) {
-			this._tabsModel = new Tabs();
+	public get visibleDocuments(): VisibleDocuments {
+		if (this._visibleDocuments === undefined) {
+			this._visibleDocuments = new VisibleDocumentsImpl();
 		}
-		return this._tabsModel;
+		return this._visibleDocuments;
 	}
 
 	public get onTelemetry(): Event<any> {

client/src/common/diagnostic.ts

@@ -20,7 +20,7 @@ import { generateUuid } from './utils/uuid';
 import { matchGlobPattern } from './utils/globPattern';
 
 import {
-	TextDocumentLanguageFeature, FeatureClient, LSPCancellationError, type TabsModel
+	TextDocumentLanguageFeature, FeatureClient, LSPCancellationError, type VisibleDocuments
 } from './features';
 
 function ensure<T, K extends keyof T>(target: T, key: K): T[K] {
@@ -285,7 +285,7 @@ class DiagnosticRequestor implements Disposable {
 
 	private isDisposed: boolean;
 	private readonly client: FeatureClient<DiagnosticProviderMiddleware, $DiagnosticPullOptions>;
-	private readonly tabs: TabsModel;
+	private readonly visibleDocuments: VisibleDocuments;
 	private readonly options: DiagnosticRegistrationOptions;
 
 	public readonly onDidChangeDiagnosticsEmitter: EventEmitter<void>;
@@ -298,9 +298,9 @@ class DiagnosticRequestor implements Disposable {
 	private workspaceCancellation: CancellationTokenSource | undefined;
 	private workspaceTimeout: Disposable | undefined;
 
-	public constructor(client: FeatureClient<DiagnosticProviderMiddleware, $DiagnosticPullOptions>, tabs: TabsModel, options: DiagnosticRegistrationOptions) {
+	public constructor(client: FeatureClient<DiagnosticProviderMiddleware, $DiagnosticPullOptions>, visibleDocuments: VisibleDocuments, options: DiagnosticRegistrationOptions) {
 		this.client = client;
-		this.tabs = tabs;
+		this.visibleDocuments = visibleDocuments;
 		this.options = options;
 
 		this.isDisposed = false;
@@ -373,7 +373,7 @@ class DiagnosticRequestor implements Disposable {
 				return;
 			}
 			this.openRequests.delete(key);
-			if (!this.tabs.isVisible(document)) {
+			if (!this.visibleDocuments.isVisible(document)) {
 				this.documentStates.unTrack(PullState.document, document);
 				return;
 			}
@@ -767,7 +767,7 @@ class DiagnosticFeatureProviderImpl implements DiagnosticProviderShape {
 	private activeTextDocument: TextDocument | undefined;
 	private readonly backgroundScheduler: BackgroundScheduler;
 
-	constructor(client: FeatureClient<DiagnosticProviderMiddleware, $DiagnosticPullOptions>, tabs: TabsModel, options: DiagnosticRegistrationOptions) {
+	constructor(client: FeatureClient<DiagnosticProviderMiddleware, $DiagnosticPullOptions>, visibleDocuments: VisibleDocuments, options: DiagnosticRegistrationOptions) {
 		const diagnosticPullOptions = Object.assign({ onChange: false, onSave: false, onFocus: false }, client.clientOptions.diagnosticPullOptions);
 		const documentSelector = client.protocol2CodeConverter.asDocumentSelector(options.documentSelector!);
 		const disposables: Disposable[] = [];
@@ -810,12 +810,12 @@ class DiagnosticFeatureProviderImpl implements DiagnosticProviderShape {
 		const matches = (document: TextDocument | Uri): boolean => {
 			return document instanceof Uri
 				? matchResource(document)
-				: Languages.match(documentSelector, document) > 0 && tabs.isVisible(document);
+				: Languages.match(documentSelector, document) > 0 && visibleDocuments.isVisible(document);
 		};
 
 		const matchesCell = (cell: NotebookCell): boolean => {
 			// Cells match if the language is allowed and if the containing notebook is visible.
-			return Languages.match(documentSelector, cell.document) > 0 && tabs.isVisible(cell.notebook.uri);
+			return Languages.match(documentSelector, cell.document) > 0 && visibleDocuments.isVisible(cell.notebook.uri);
 		};
 
 		const isActiveDocument = (document: TextDocument | Uri): boolean => {
@@ -824,7 +824,7 @@ class DiagnosticFeatureProviderImpl implements DiagnosticProviderShape {
 				: this.activeTextDocument === document;
 		};
 
-		this.diagnosticRequestor = new DiagnosticRequestor(client, tabs, options);
+		this.diagnosticRequestor = new DiagnosticRequestor(client, visibleDocuments, options);
 		this.backgroundScheduler = new BackgroundScheduler(client, this.diagnosticRequestor);
 
 		const addToBackgroundIfNeeded = (document: TextDocument | Uri): void => {
@@ -882,7 +882,7 @@ class DiagnosticFeatureProviderImpl implements DiagnosticProviderShape {
 			}
 		}));
 
-		disposables.push(tabs.onOpen((opened) => {
+		disposables.push(visibleDocuments.onOpen((opened) => {
 			for (const resource of opened) {
 				// We already know about this document. This can happen via a document open.
 				if (this.diagnosticRequestor.knows(PullState.document, resource)) {
@@ -902,7 +902,7 @@ class DiagnosticFeatureProviderImpl implements DiagnosticProviderShape {
 				//    workspace.textDocuments and Window.activeTextEditor are updated.
 				//
 				// This means: for newly created tab/editors we don't find the underlying
-				// document yet. So we do nothing an rely on the underlying open document event
+				// document yet. So we do nothing and rely on the underlying open document event
 				// to be fired.
 				if (textDocument !== undefined && matches(textDocument)) {
 					this.diagnosticRequestor.pull(textDocument, () => { addToBackgroundIfNeeded(textDocument!); });
@@ -928,9 +928,9 @@ class DiagnosticFeatureProviderImpl implements DiagnosticProviderShape {
 			}
 		}
 
-		// Pull all tabs if not already pulled as text document
+		// Pull all visible documents if not already pulled as text document
 		if (diagnosticPullOptions.onTabs === true) {
-			for (const resource of tabs.getTabResources()) {
+			for (const resource of visibleDocuments.getResources()) {
 				if (!pulledTextDocuments.has(resource.toString()) && matches(resource)) {
 					this.diagnosticRequestor.pull(resource, () => { addToBackgroundIfNeeded(resource); });
 				}
@@ -1004,8 +1004,8 @@ class DiagnosticFeatureProviderImpl implements DiagnosticProviderShape {
 			}
 		}));
 
-		// Same when a tabs closes.
-		tabs.onClose((closed) => {
+		// Same when a visible document closes.
+		visibleDocuments.onClose((closed) => {
 			for (const document of closed) {
 				this.cleanUpDocument(document);
 			}
@@ -1099,7 +1099,7 @@ export class DiagnosticFeature extends TextDocumentLanguageFeature<DiagnosticOpt
 	}
 
 	protected registerLanguageProvider(options: DiagnosticRegistrationOptions): [Disposable, DiagnosticProviderShape] {
-		const provider = new DiagnosticFeatureProviderImpl(this._client, this._client.tabsModel, options);
+		const provider = new DiagnosticFeatureProviderImpl(this._client, this._client.visibleDocuments, options);
 		return [provider.disposable, provider];
 	}
 }

client/src/common/features.ts

@@ -643,12 +643,12 @@ export interface DedicatedTextSynchronizationFeature {
 	handles(textDocument: TextDocument): boolean;
 }
 
-export interface TabsModel {
+export interface VisibleDocuments {
 	onClose: Event<Set<Uri>>;
 	onOpen: Event<Set<Uri>>;
 	isActive(document: TextDocument | Uri): boolean;
 	isVisible(document: TextDocument | Uri): boolean;
-	getTabResources(): Set<Uri>;
+	getResources(): Set<Uri>;
 }
 
 // Features can refer to other feature when implementing themselves.
@@ -671,7 +671,7 @@ export interface FeatureClient<M, CO = object> {
 	clientOptions: CO;
 	middleware: M;
 
-	tabsModel: TabsModel;
+	visibleDocuments: VisibleDocuments;
 
 	start(): Promise<void>;
 	isRunning(): boolean;

client/src/common/textSynchronization.ts

@@ -73,8 +73,8 @@ export class DidOpenTextDocumentFeature extends TextDocumentEventFeature<DidOpen
 			if (!this.matches(document)) {
 				return;
 			}
-			const tabsModel = this._client.tabsModel;
-			if (tabsModel.isVisible(document)) {
+			const visibleDocuments = this._client.visibleDocuments;
+			if (visibleDocuments.isVisible(document)) {
 				return super.callback(document);
 			} else {
 				this._pendingOpenNotifications.set(document.uri.toString(), document);
@@ -113,8 +113,8 @@ export class DidOpenTextDocumentFeature extends TextDocumentEventFeature<DidOpen
 				return;
 			}
 			if (Languages.match(documentSelector, textDocument) > 0 && !this._client.hasDedicatedTextSynchronizationFeature(textDocument)) {
-				const tabsModel = this._client.tabsModel;
-				if (tabsModel.isVisible(textDocument)) {
+				const visibleDocuments = this._client.visibleDocuments;
+				if (visibleDocuments.isVisible(textDocument)) {
 					const middleware = this._client.middleware;
 					const didOpen = (textDocument: TextDocument): Promise<void> => {
 						return this._client.sendNotification(this._type, this._createParams(textDocument));
@@ -130,13 +130,13 @@ export class DidOpenTextDocumentFeature extends TextDocumentEventFeature<DidOpen
 		});
 		if (this._delayOpen && this._pendingOpenListeners === undefined) {
 			this._pendingOpenListeners = [];
-			const tabsModel = this._client.tabsModel;
-			this._pendingOpenListeners.push(tabsModel.onClose((closed) => {
+			const visibleDocuments = this._client.visibleDocuments;
+			this._pendingOpenListeners.push(visibleDocuments.onClose((closed) => {
 				for (const uri of closed) {
 					this._pendingOpenNotifications.delete(uri.toString());
 				}
 			}));
-			this._pendingOpenListeners.push(tabsModel.onOpen((opened) => {
+			this._pendingOpenListeners.push(visibleDocuments.onOpen((opened) => {
 				for (const uri of opened) {
 					const document = this._pendingOpenNotifications.get(uri.toString());
 					if (document !== undefined) {

testbed/server/src/server.ts

@@ -438,8 +438,10 @@ connection.onSignatureHelp((item): SignatureHelp => {
 });
 
 connection.onDefinition((params): DefinitionLink[] => {
+	const uri = URI.parse(params.textDocument.uri);
+	const target = path.join(path.dirname(uri.fsPath), 'peek.bat');
 	return [{
-		targetUri: params.textDocument.uri,
+		targetUri: URI.file(target).toString(),
 		targetRange: { start: { line: 0, character: 2}, end: {line: 5, character: 45 } },
 		targetSelectionRange: { start: { line: 1, character: 5}, end: {line: 1, character: 10 } },
 		originSelectionRange: {
@@ -450,8 +452,10 @@ connection.onDefinition((params): DefinitionLink[] => {
 });
 
 connection.onDeclaration((params): DeclarationLink[] => {
+	const uri = URI.parse(params.textDocument.uri);
+	const target = path.join(path.dirname(uri.fsPath), 'peek.bat');
 	return [{
-		targetUri: params.textDocument.uri,
+		targetUri: URI.file(target).toString(),
 		targetRange: { start: { line: 3, character: 0}, end: {line: 3, character: 10 } },
 		targetSelectionRange: { start: { line: 3, character: 0}, end: {line: 3, character: 10 } },
 		originSelectionRange: {

testbed/workspace/peek.bat

@@ -0,0 +1,3 @@
+FOR %%f IN (*.jpg *.png *.bmp) DO XCOPY C:\source\"%%f" c:\images /m /y
+REM This moves any files with a .jpg, .png,
+REM or .bmp extension from c:\source to c:\images;;