-
Notifications
You must be signed in to change notification settings - Fork 146
Fix/Use active R session package for editor language completion #1816
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
33b7f4c
acb6b89
b33b255
0a5cf6f
6e361b6
b2fbee2
cc26c50
d53815d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,16 @@ import { CommonOptions } from 'child_process'; | |
| import { boundSessionForDocument, onDidBindSessionDocument, Session } from './session'; | ||
| import { SessionSignatureHelpProvider } from './signatureHelp'; | ||
|
|
||
| interface SessionState { | ||
| attachedPackages: string[]; | ||
| loadedNamespaces: string[]; | ||
| } | ||
|
|
||
| interface SessionWorkspaceData { | ||
| search: string[]; | ||
| loaded_namespaces: string[]; | ||
| } | ||
|
|
||
| export class LanguageService implements Disposable { | ||
| private readonly clients: Map<string, LanguageClient> = new Map(); | ||
| private readonly initSet: Set<string> = new Set(); | ||
|
|
@@ -22,6 +32,8 @@ export class LanguageService implements Disposable { | |
| private readonly clientUpdates = new Map<string, Promise<void>>(); | ||
| private readonly listeners: Disposable[] = []; | ||
| private disposed = false; | ||
| private readonly clientScopes: Map<string, string> = new Map(); | ||
| private readonly sessionStates: Map<string, { state: SessionState; stateKey: string; sessionId: string }> = new Map(); | ||
|
|
||
| constructor() { | ||
| this.outputChannel = window.createOutputChannel('R Language Server'); | ||
|
|
@@ -35,6 +47,68 @@ export class LanguageService implements Disposable { | |
| return this.stopLanguageService(); | ||
| } | ||
|
|
||
| syncSessionState(data?: SessionWorkspaceData, resource?: Uri, sessionId = '', active = true): void { | ||
| // Bound virtual documents keep their owner's packages when focus moves. | ||
| if (sessionId) { | ||
| this.syncSessionScope(`session:${sessionId}`, data, sessionId); | ||
| } | ||
| if (active || !data) { | ||
| this.syncSessionScope(this.getSessionScope(resource), data, sessionId); | ||
| } | ||
| } | ||
|
|
||
| private syncSessionScope(scope: string, data: SessionWorkspaceData | undefined, sessionId: string): void { | ||
| const current = this.sessionStates.get(scope); | ||
| if (!data) { | ||
| if (!current || (sessionId && current.sessionId !== sessionId)) { | ||
| return; | ||
| } | ||
| this.sessionStates.delete(scope); | ||
| this.applySessionStateToScope(scope, { | ||
| attachedPackages: [], | ||
| loadedNamespaces: [], | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const state: SessionState = { | ||
| attachedPackages: data.search | ||
| .filter(value => value.startsWith('package:')) | ||
| .map(value => value.substring(8)), | ||
| loadedNamespaces: data.loaded_namespaces, | ||
| }; | ||
| const stateKey = this.getSessionStateKey(state); | ||
| if (current?.stateKey === stateKey && current.sessionId === sessionId) { | ||
| return; | ||
| } | ||
|
|
||
| this.sessionStates.set(scope, { state, stateKey, sessionId }); | ||
| this.applySessionStateToScope(scope, state); | ||
| } | ||
|
|
||
| private applySessionStateToScope(scope: string, state: SessionState): void { | ||
| for (const [key, client] of this.clients) { | ||
| if (this.clientScopes.get(key) === scope) { | ||
| void this.applySessionState(client, state); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private async applySessionState(client: LanguageClient, state: SessionState): Promise<void> { | ||
| try { | ||
| await client.sendRequest('r/syncSessionState', state); | ||
| } catch { | ||
| // Keep language-service features available if session synchronization fails. | ||
| } | ||
| } | ||
|
|
||
| private getSessionStateKey(state: SessionState): string { | ||
| return [ | ||
| state.attachedPackages.join('\u0000'), | ||
| state.loadedNamespaces.join('\u0000'), | ||
| ].join('\u0001'); | ||
| } | ||
|
|
||
| private spawnServer(client: LanguageClient, rPath: string, args: readonly string[], options: CommonOptions & { cwd: string }): DisposableProcess { | ||
| const childProcess = spawn(rPath, args, options); | ||
| const pid = childProcess.pid || -1; | ||
|
|
@@ -64,9 +138,9 @@ export class LanguageService implements Disposable { | |
| return childProcess; | ||
| } | ||
|
|
||
| private async createClient(selector: DocumentFilter[], | ||
| private async createClient(key: string, selector: DocumentFilter[], | ||
| cwd: string, workspaceFolder: WorkspaceFolder | undefined, outputChannel: OutputChannel, | ||
| resource?: Uri, target?: Session): Promise<LanguageClient> { | ||
| resource?: Uri, sessionScope: string = 'global', target?: Session): Promise<LanguageClient> { | ||
|
|
||
| let client: LanguageClient; | ||
| const virtualOnly = selector.every(filter => 'scheme' in filter | ||
|
|
@@ -227,11 +301,24 @@ export class LanguageService implements Disposable { | |
| } | ||
|
|
||
| extensionContext.subscriptions.push(client); | ||
| if (target) { | ||
| this.syncSessionScope(sessionScope, target.workspaceData, target.sessionId); | ||
| } | ||
| await client.start(); | ||
| await this.registerClient(key, client, sessionScope); | ||
| return client; | ||
| } | ||
|
|
||
|
|
||
| private async registerClient(key: string, client: LanguageClient, sessionScope: string): Promise<void> { | ||
| this.clients.set(key, client); | ||
| this.clientScopes.set(key, sessionScope); | ||
| const sessionState = this.sessionStates.get(sessionScope)?.state; | ||
| if (sessionState) { | ||
| await this.applySessionState(client, sessionState); | ||
|
Comment on lines
+316
to
+318
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Register the client before awaiting its initial state synchronization The caller adds this client to |
||
| } | ||
| } | ||
|
|
||
| private checkClient(name: string): boolean { | ||
| if (this.initSet.has(name)) { | ||
| return true; | ||
|
|
@@ -244,6 +331,14 @@ export class LanguageService implements Disposable { | |
| return false; | ||
| } | ||
|
|
||
| private getSessionScope(resource?: Uri): string { | ||
| if (this.config.get<boolean>('lsp.multiServer') !== true) { | ||
| return 'global'; | ||
| } | ||
| const folder = resource ? workspace.getWorkspaceFolder(resource) : undefined; | ||
| return folder ? this.getKey(folder.uri) : 'unscoped'; | ||
| } | ||
|
|
||
| private getKey(uri: Uri): string { | ||
| switch (uri.scheme) { | ||
| case 'untitled': | ||
|
|
@@ -272,25 +367,26 @@ export class LanguageService implements Disposable { | |
|
|
||
| if (document.uri.scheme === 'vscode-interactive-input') { | ||
| const key = document.uri.toString(); | ||
| const scope = target ? `session:${target.sessionId}` : this.getSessionScope(folder?.uri); | ||
| if (!this.checkClient(key)) { | ||
| const selector = [{ scheme: 'vscode-interactive-input', language: 'r', pattern: document.uri.fsPath }]; | ||
| const client = await this.createClient(selector, target?.workingDir ?? folder?.uri.fsPath ?? os.homedir(), folder, this.outputChannel, folder?.uri, target); | ||
| this.clients.set(key, client); this.initSet.delete(key); | ||
| await this.createClient(key, selector, target?.workingDir ?? folder?.uri.fsPath ?? os.homedir(), folder, this.outputChannel, folder?.uri, scope, target); | ||
| this.initSet.delete(key); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| // Each notebook uses a server started from parent folder | ||
| if (document.uri.scheme === 'vscode-notebook-cell') { | ||
| const key = this.getKey(document.uri); | ||
| const scope = target ? `session:${target.sessionId}` : this.getSessionScope(folder?.uri); | ||
| if (!this.checkClient(key)) { | ||
| console.log(`Start language server for ${document.uri.toString(true)}`); | ||
| const documentSelector: DocumentFilter[] = [ | ||
| { scheme: 'vscode-notebook-cell', language: 'r', pattern: `${document.uri.fsPath}` }, | ||
| ]; | ||
| const client = await this.createClient(documentSelector, | ||
| target?.workingDir ?? folder?.uri.fsPath ?? dirname(document.uri.fsPath), folder, this.outputChannel, folder?.uri ?? document.uri, target); | ||
| this.clients.set(key, client); | ||
| await this.createClient(key, documentSelector, | ||
| target?.workingDir ?? folder?.uri.fsPath ?? dirname(document.uri.fsPath), folder, this.outputChannel, folder?.uri ?? document.uri, scope, target); | ||
| this.initSet.delete(key); | ||
| } | ||
| return; | ||
|
|
@@ -307,8 +403,7 @@ export class LanguageService implements Disposable { | |
| { scheme: 'file', language: 'r', pattern: pattern }, | ||
| { scheme: 'file', language: 'rmd', pattern: pattern }, | ||
| ]; | ||
| const client = await this.createClient(documentSelector, folder.uri.fsPath, folder, this.outputChannel, folder.uri); | ||
| this.clients.set(key, client); | ||
| await this.createClient(key, documentSelector, folder.uri.fsPath, folder, this.outputChannel, folder.uri, key); | ||
| this.initSet.delete(key); | ||
| } | ||
|
|
||
|
|
@@ -323,8 +418,7 @@ export class LanguageService implements Disposable { | |
| { scheme: 'untitled', language: 'r' }, | ||
| { scheme: 'untitled', language: 'rmd' }, | ||
| ]; | ||
| const client = await this.createClient(documentSelector, os.homedir(), undefined, this.outputChannel, document.uri); | ||
| this.clients.set(key, client); | ||
| await this.createClient(key, documentSelector, os.homedir(), undefined, this.outputChannel, document.uri, 'unscoped'); | ||
| this.initSet.delete(key); | ||
| } | ||
| return; | ||
|
|
@@ -338,9 +432,8 @@ export class LanguageService implements Disposable { | |
| const documentSelector: DocumentFilter[] = [ | ||
| { scheme: 'file', pattern: document.uri.fsPath }, | ||
| ]; | ||
| const client = await this.createClient(documentSelector, | ||
| dirname(document.uri.fsPath), undefined, this.outputChannel, document.uri); | ||
| this.clients.set(key, client); | ||
| await this.createClient(key, documentSelector, | ||
| dirname(document.uri.fsPath), undefined, this.outputChannel, document.uri, 'unscoped'); | ||
| this.initSet.delete(key); | ||
| } | ||
| return; | ||
|
|
@@ -372,6 +465,7 @@ export class LanguageService implements Disposable { | |
| const client = this.clients.get(key); | ||
| if (client) { | ||
| this.clients.delete(key); | ||
| this.clientScopes.delete(key); | ||
| this.initSet.delete(key); | ||
| void client.stop(); | ||
| } | ||
|
|
@@ -389,7 +483,7 @@ export class LanguageService implements Disposable { | |
| // repeatedly. Restart its shared server only once per owner. | ||
| if (restart && clientTargets.get(key) !== target) { | ||
| const client = this.clients.get(key); | ||
| this.clients.delete(key); this.initSet.delete(key); | ||
| this.clients.delete(key); this.clientScopes.delete(key); this.initSet.delete(key); | ||
| await client?.stop(); | ||
| } | ||
| await didOpenTextDocument(document); | ||
|
|
@@ -417,6 +511,7 @@ export class LanguageService implements Disposable { | |
| const client = this.clients.get(key); | ||
| if (client) { | ||
| this.clients.delete(key); | ||
| this.clientScopes.delete(key); | ||
| this.initSet.delete(key); | ||
| void client.stop(); | ||
| } | ||
|
|
@@ -444,8 +539,7 @@ export class LanguageService implements Disposable { | |
|
|
||
| const workspaceFolder = workspace.workspaceFolders?.[0]; | ||
| const cwd = workspaceFolder ? workspaceFolder.uri.fsPath : os.homedir(); | ||
| const client = await this.createClient(documentSelector, cwd, undefined, this.outputChannel, workspaceFolder?.uri); | ||
| this.clients.set('global', client); | ||
| await this.createClient('global', documentSelector, cwd, undefined, this.outputChannel, workspaceFolder?.uri, 'global'); | ||
| this.startMultiLanguageService(true); | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Apply the current session state to workspaces created after this request
This updates only the workspaces that already exist when
r/syncSessionStateruns. In single-server mode, adding another folder to an existing multi-root workspace creates a newWorkspacethroughworkspace/didChangeWorkspaceFolders, and that workspace starts with the default packages. Since the session/package state is unchanged, the TypeScript deduplication does not send another sync. I reproduced this by syncing dplyr, then invoking the real workspace-folder notification handler: the existing workspace offeredmutate, while the newly added workspace did not. Please retain the active state on the server and apply it when adding a workspace, or explicitly replay it after workspace-folder additions so completion remains available across all documents handled by the single server.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rechecked at d53815d: this case is still unresolved. The R handler still updates only self$workspaces$values() at synchronization time, and no replay is triggered for added folders. Using this revision's handler with languageserver 0.3.20, I synced dplyr, invoked workspace_did_change_workspace_folders to add a folder, and requested workspace completion: mutate was present for the existing workspace and absent for the added workspace. Please retain/apply the active state when a workspace is created, or replay synchronization after folder additions, and add coverage for that path.