From 57055c1aeeaa42703c7846343d4077bd2e8cbbdd Mon Sep 17 00:00:00 2001 From: krassowski Date: Thu, 29 Apr 2021 22:17:02 +0100 Subject: [PATCH 1/4] Add priority setting for servers handling the same language --- atest/03_Notebook.robot | 2 +- docs/Configuring.ipynb | 3 +- docs/Language Servers.ipynb | 5 +- packages/jupyterlab-lsp/schema/plugin.json | 6 ++ .../src/components/statusbar.tsx | 55 ++++++++++------- .../jupyterlab-lsp/src/connection_manager.ts | 45 ++++++++++---- .../src/editor_integration/testutils.ts | 6 +- packages/jupyterlab-lsp/src/index.ts | 6 +- packages/jupyterlab-lsp/src/manager.ts | 60 +++++++++++++++---- packages/jupyterlab-lsp/src/tokens.ts | 41 ++++++------- .../jupyterlab-lsp/src/virtual/editor.spec.ts | 4 +- .../jupyter_lsp/schema/schema.json | 2 +- 12 files changed, 159 insertions(+), 76 deletions(-) diff --git a/atest/03_Notebook.robot b/atest/03_Notebook.robot index 7b2e5a0db..89c39a4d4 100644 --- a/atest/03_Notebook.robot +++ b/atest/03_Notebook.robot @@ -47,7 +47,7 @@ Moving Cells Around Foreign Extractors ${file} = Set Variable Foreign extractors.ipynb Configure JupyterLab Plugin - ... {"language_servers": {"texlab": {"serverSettings": {"latex.lint.onChange": true}}, "bash-langauge-server": {"bashIde.highlightParsingErrors": true}}} + ... {"language_servers": {"texlab": {"serverSettings": {"latex.lint.onChange": true}}, "bash-langauge-server": {"bashIde.highlightParsingErrors": true}}, "pylsp": {"priority": 1000}} Capture Page Screenshot 10-configured.png Reset Application State Setup Notebook Python ${file} diff --git a/docs/Configuring.ipynb b/docs/Configuring.ipynb index 6917786f4..f66064853 100644 --- a/docs/Configuring.ipynb +++ b/docs/Configuring.ipynb @@ -66,7 +66,8 @@ " `mkdir \"new directory\"` should be split into `[\"mkdir\", \"new directory\"]`;\n", " If you have Python installed, you can use `shlex.split(\"your command\")` to\n", " get such an array.\n", - "- the `languages` which the language server will respond to, and\n", + "- the `languages` which the language server will respond to (language names\n", + " should be given in lowercase), and\n", "- the schema `version` of the spec (currently `2`)\n", "- `mime_types` by which the notebooks and files will be matched to the language\n", " server:\n", diff --git a/docs/Language Servers.ipynb b/docs/Language Servers.ipynb index c3cc8835d..328985b3b 100644 --- a/docs/Language Servers.ipynb +++ b/docs/Language Servers.ipynb @@ -137,8 +137,9 @@ "\n", "These servers have support for notebooks and file editors. The `pyls` and\n", "`r-languageserver` are well-tested, while `jedi` and `Julia` servers are\n", - "experimental. Please only install one language server per language (`jedi` or\n", - "`pyls` for Python - not both)." + "experimental. If you choose to install multiple language servers for the same\n", + "language, the one with the highest `priority` (which can be set in the _Advanced\n", + "Settings Editor_) will be used." ] }, { diff --git a/packages/jupyterlab-lsp/schema/plugin.json b/packages/jupyterlab-lsp/schema/plugin.json index 8a2fa3f3b..3eb772151 100644 --- a/packages/jupyterlab-lsp/schema/plugin.json +++ b/packages/jupyterlab-lsp/schema/plugin.json @@ -16,6 +16,12 @@ "description": "Configuration to be sent to language server over LSP when initialized: see the specific language server's documentation for more", "type": "object", "default": {} + }, + "priority": { + "title": "Priority of the server", + "description": "When multiple servers match specific document/language, the server with higher priority will be used", + "type": "number", + "default": 50 } } } diff --git a/packages/jupyterlab-lsp/src/components/statusbar.tsx b/packages/jupyterlab-lsp/src/components/statusbar.tsx index 65649196a..3349d4176 100644 --- a/packages/jupyterlab-lsp/src/components/statusbar.tsx +++ b/packages/jupyterlab-lsp/src/components/statusbar.tsx @@ -35,7 +35,12 @@ import { LSPConnection } from '../connection'; import { DocumentConnectionManager } from '../connection_manager'; import { SERVER_EXTENSION_404 } from '../errors'; import { LanguageServerManager } from '../manager'; -import { ILSPAdapterManager, ILanguageServerManager } from '../tokens'; +import { + ILSPAdapterManager, + ILanguageServerManager, + TSessionMap, + TLanguageServerId +} from '../tokens'; import { VirtualDocument, collect_documents } from '../virtual/document'; import { codeCheckIcon, codeClockIcon, codeWarningIcon } from './icons'; @@ -464,13 +469,13 @@ export namespace LSPStatus { }, this); } - get available_servers(): Array { - return Array.from(this.language_server_manager.sessions.values()); + get available_servers(): TSessionMap { + return this.language_server_manager.sessions; } get supported_languages(): Set { const languages = new Set(); - for (let server of this.available_servers) { + for (let server of this.available_servers.values()) { for (let language of server.spec.languages) { languages.add(language.toLocaleLowerCase()); } @@ -478,20 +483,30 @@ export namespace LSPStatus { return languages; } - private is_server_running(server: SCHEMA.LanguageServerSession): boolean { - for (let language of server.spec.languages) { - if (this.detected_languages.has(language.toLocaleLowerCase())) { + private is_server_running( + id: TLanguageServerId, + server: SCHEMA.LanguageServerSession + ): boolean { + for (const language of this.detected_languages) { + const matchedServers = this.language_server_manager.getMatchingServers({ + language + }); + // TODO server.status === "started" ? + // TODO update once multiple servers are allowed + if (matchedServers[0] === id) { return true; } } - return false; } get documents_by_server(): Map< SCHEMA.LanguageServerSession, Map > { - let data = new Map(); + let data = new Map< + SCHEMA.LanguageServerSession, + Map + >(); if (!this.adapter?.virtual_editor) { return data; } @@ -501,19 +516,17 @@ export namespace LSPStatus { for (let document of documents.values()) { let language = document.language.toLocaleLowerCase(); - let servers = this.available_servers.filter( - server => server.spec.languages.indexOf(language) !== -1 + let server_ids = this._connection_manager.language_server_manager.getMatchingServers( + { language: document.language } ); - if (servers.length > 1) { - console.warn('More than one server per language for', language); - } - if (servers.length === 0) { + if (server_ids.length === 0) { continue; } - let server = servers[0]; + // For now only use the server with the highest priority + let server = this.language_server_manager.sessions.get(server_ids[0]); if (!data.has(server)) { - data.set(server, new Map()); + data.set(server, new Map()); } let documents_map = data.get(server); @@ -529,9 +542,9 @@ export namespace LSPStatus { } get servers_available_not_in_use(): Array { - return this.available_servers.filter( - server => !this.is_server_running(server) - ); + return [...this.available_servers.entries()] + .filter(([id, server]) => !this.is_server_running(id, server)) + .map(([id, server]) => server); } get detected_languages(): Set { @@ -577,7 +590,7 @@ export namespace LSPStatus { detected_documents.forEach((document, uri) => { let connection = this._connection_manager.connections.get(uri); - let server_id = this._connection_manager.language_server_manager.getServerId( + let server_id = this._connection_manager.language_server_manager.getMatchingServers( { language: document.language } ); if (server_id !== null) { diff --git a/packages/jupyterlab-lsp/src/connection_manager.ts b/packages/jupyterlab-lsp/src/connection_manager.ts index 5b0df1ebf..ba5f73029 100644 --- a/packages/jupyterlab-lsp/src/connection_manager.ts +++ b/packages/jupyterlab-lsp/src/connection_manager.ts @@ -1,13 +1,14 @@ import { PageConfig, URLExt } from '@jupyterlab/coreutils'; import { Signal } from '@lumino/signaling'; +import type * as protocol from 'vscode-languageserver-protocol'; import type * as ConnectionModuleType from './connection'; import { ILSPLogConsole, - ILanguageServerConfiguration, ILanguageServerManager, TLanguageServerConfigurations, - TLanguageServerId + TLanguageServerId, + TServerKeys } from './tokens'; import { expandDottedPaths, sleep, until_ready } from './utils'; import { IForeignContext, VirtualDocument } from './virtual/document'; @@ -134,9 +135,14 @@ export class DocumentConnectionManager { language ); - const language_server_id = this.language_server_manager.getServerId({ + const matchingServers = this.language_server_manager.getMatchingServers({ language }); + this.console.debug('Matching servers: ', matchingServers); + + // for now use only the server with the highest priority. + const language_server_id = + matchingServers.length === 0 ? null : matchingServers[0]; // lazily load 1) the underlying library (1.5mb) and/or 2) a live WebSocket- // like connection: either already connected or potentially in the process @@ -161,13 +167,21 @@ export class DocumentConnectionManager { * "serverSettings" keyword in the setting registry. New keywords can * be added and extra functionality implemented here when needed. */ - public updateServerConfigurations(allServerSettings: any) { - for (let language_server_id in allServerSettings) { - const parsedSettings = expandDottedPaths( - allServerSettings[language_server_id].serverSettings - ); + public updateServerConfigurations( + allServerSettings: TLanguageServerConfigurations + ) { + let language_server_id: TServerKeys; + this.language_server_manager.setConfiguration(allServerSettings); + + for (language_server_id in allServerSettings) { + if (!allServerSettings.hasOwnProperty(language_server_id)) { + continue; + } + const rawSettings = allServerSettings[language_server_id]; - const serverSettings: ILanguageServerConfiguration = { + const parsedSettings = expandDottedPaths(rawSettings.serverSettings); + + const serverSettings: protocol.DidChangeConfigurationParams = { settings: parsedSettings }; @@ -351,9 +365,14 @@ export namespace DocumentConnectionManager { ? rootUri : virtualDocumentsUri; - const language_server_id = Private.getLanguageServerManager().getServerId({ - language - }); + // for now take the best match only + const matchingServers = Private.getLanguageServerManager().getMatchingServers( + { + language + } + ); + const language_server_id = + matchingServers.length === 0 ? null : matchingServers[0]; if (language_server_id === null) { throw `No language server installed for language ${language}`; @@ -441,7 +460,7 @@ namespace Private { export function updateServerConfiguration( language_server_id: TLanguageServerId, - settings: ILanguageServerConfiguration + settings: protocol.DidChangeConfigurationParams ): void { const connection = _connections.get(language_server_id); if (connection) { diff --git a/packages/jupyterlab-lsp/src/editor_integration/testutils.ts b/packages/jupyterlab-lsp/src/editor_integration/testutils.ts index c8709da66..2667dc079 100644 --- a/packages/jupyterlab-lsp/src/editor_integration/testutils.ts +++ b/packages/jupyterlab-lsp/src/editor_integration/testutils.ts @@ -71,7 +71,7 @@ export interface ITestEnvironment { export class MockLanguageServerManager extends LanguageServerManager { async fetchSessions() { this._sessions = new Map(); - this._sessions.set('pyls', { + this._sessions.set('pylsp', { spec: { languages: ['python'] } @@ -111,7 +111,9 @@ export class MockExtension implements ILSPExtension { this.app = null; this.feature_manager = new FeatureManager(); this.editor_type_manager = new VirtualEditorManager(); - this.language_server_manager = new MockLanguageServerManager({}); + this.language_server_manager = new MockLanguageServerManager({ + console: new BrowserConsole() + }); this.connection_manager = new DocumentConnectionManager({ language_server_manager: this.language_server_manager, console: new BrowserConsole() diff --git a/packages/jupyterlab-lsp/src/index.ts b/packages/jupyterlab-lsp/src/index.ts index 722b5b778..80de10f02 100644 --- a/packages/jupyterlab-lsp/src/index.ts +++ b/packages/jupyterlab-lsp/src/index.ts @@ -146,10 +146,12 @@ export class LSPExtension implements ILSPExtension { status_bar: IStatusBar | null ) { const trans = (translator || nullTranslator).load('jupyterlab-lsp'); - this.language_server_manager = new LanguageServerManager({}); + this.language_server_manager = new LanguageServerManager({ + console: this.console.scope('LanguageServerManager') + }); this.connection_manager = new DocumentConnectionManager({ language_server_manager: this.language_server_manager, - console: this.console + console: this.console.scope('DocumentConnectionManager') }); const statusButtonExtension = new StatusButtonExtension({ diff --git a/packages/jupyterlab-lsp/src/manager.ts b/packages/jupyterlab-lsp/src/manager.ts index 4ef9f1d60..0fe38ccee 100644 --- a/packages/jupyterlab-lsp/src/manager.ts +++ b/packages/jupyterlab-lsp/src/manager.ts @@ -3,7 +3,13 @@ import { ServerConnection } from '@jupyterlab/services'; import { Signal } from '@lumino/signaling'; import * as SCHEMA from './_schema'; -import { ILanguageServerManager, TSessionMap } from './tokens'; +import { + ILanguageServerManager, + ILSPLogConsole, + TLanguageServerConfigurations, + TLanguageServerId, + TSessionMap +} from './tokens'; export class LanguageServerManager implements ILanguageServerManager { protected _sessionsChanged: Signal = new Signal< @@ -16,6 +22,8 @@ export class LanguageServerManager implements ILanguageServerManager { private _statusCode: number; private _retries: number; private _retriesInterval: number; + private _configuration: TLanguageServerConfigurations; + private console: ILSPLogConsole; constructor(options: ILanguageServerManager.IOptions) { this._settings = options.settings || ServerConnection.makeSettings(); @@ -23,6 +31,8 @@ export class LanguageServerManager implements ILanguageServerManager { this._retries = options.retries || 2; this._retriesInterval = options.retriesInterval || 10000; this._statusCode = null; + this._configuration = {}; + this.console = options.console; this.fetchSessions().catch(console.warn); } @@ -38,16 +48,44 @@ export class LanguageServerManager implements ILanguageServerManager { return this._sessions; } - getServerId(options: ILanguageServerManager.IGetServerIdOptions) { + setConfiguration(configuration: TLanguageServerConfigurations): void { + this._configuration = configuration; + } + + getMatchingServers(options: ILanguageServerManager.IGetServerIdOptions) { + const matchingSessionsKeys: TLanguageServerId[] = []; + const config = this._configuration; + // most things speak language + // if language is not known, it is guessed based on MIME type earlier + // so some language should be available by now (which can be not obvious, e.g. "plain" for txt documents) for (const [key, session] of this._sessions.entries()) { if (options.language) { - if (session.spec.languages.indexOf(options.language) !== -1) { - return key; + if ( + session.spec.languages.indexOf( + options.language.toLocaleLowerCase() + ) !== -1 + ) { + matchingSessionsKeys.push(key); } + } else { + this.console.error( + 'Cannot match server by language: language not available; ensure that kernel and specs provide language and MIME type' + ); } } - return null; + + return matchingSessionsKeys.sort((a, b) => { + const a_priority = config[a]?.priority ?? 50; + const b_priority = config[b]?.priority ?? 50; + if (a_priority == b_priority) { + this.console.warn( + `Two matching servers: ${a} and ${b} have the same priority; choose which one to use by changing the priority in Advanced Settings Editor` + ); + } + // higher priority = higher in the list (descending order) + return b_priority - a_priority; + }); } get statusCode(): number { @@ -81,11 +119,12 @@ export class LanguageServerManager implements ILanguageServerManager { return; } - for (const key of Object.keys(sessions)) { - if (this._sessions.has(key)) { - Object.assign(this._sessions.get(key), sessions[key]); + for (let key of Object.keys(sessions)) { + let id: TLanguageServerId = key as TLanguageServerId; + if (this._sessions.has(id)) { + Object.assign(this._sessions.get(id), sessions[key]); } else { - this._sessions.set(key, sessions[key]); + this._sessions.set(id, sessions[key]); } } @@ -93,7 +132,8 @@ export class LanguageServerManager implements ILanguageServerManager { for (const oldKey in oldKeys) { if (!sessions[oldKey]) { - this._sessions.delete(oldKey); + let oldId = oldKey as TLanguageServerId; + this._sessions.delete(oldId); } } diff --git a/packages/jupyterlab-lsp/src/tokens.ts b/packages/jupyterlab-lsp/src/tokens.ts index af12ae37c..3c798838d 100644 --- a/packages/jupyterlab-lsp/src/tokens.ts +++ b/packages/jupyterlab-lsp/src/tokens.ts @@ -5,6 +5,7 @@ import { ServerConnection } from '@jupyterlab/services'; import { Token } from '@lumino/coreutils'; import { ISignal, Signal } from '@lumino/signaling'; +import { LanguageServer2 as LSPLanguageServerSettings } from './_plugin'; import * as SCHEMA from './_schema'; import { WidgetAdapter } from './adapters/adapter'; import { @@ -24,16 +25,15 @@ import { IFeatureOptions, ILSPExtension, LSPExtension } from './index'; import IEditor = CodeEditor.IEditor; -export type TLanguageServerId = string; export type TLanguageId = string; -export type TSessionMap = Map; - /** - * TODO: Should this support custom server keys? + * Example server keys==ids that are expected. The list is not exhaustive. + * Custom server keys are allowed. Constraining the values helps avoid errors, + * but at runtime any value is allowed. */ -export type TServerKeys = - | 'pyls' +export type TLanguageServerId = + | 'pylsp' | 'bash-language-server' | 'dockerfile-language-server-nodejs' | 'javascript-typescript-langserver' @@ -43,33 +43,29 @@ export type TServerKeys = | 'vscode-json-languageserver-bin' | 'yaml-language-server' | 'r-languageserver'; +export type TServerKeys = TLanguageServerId; -export type TLanguageServerConfigurations = { - [k in TServerKeys]: { - serverSettings: any; - }; -}; +export type TSessionMap = Map; + +export type TLanguageServerConfigurations = Partial< + Record +>; export interface ILanguageServerManager { sessionsChanged: ISignal; sessions: TSessionMap; - getServerId( + /** + * An ordered list of matching servers, with servers of higher priority higher in the list + */ + getMatchingServers( options: ILanguageServerManager.IGetServerIdOptions - ): TLanguageServerId; + ): TLanguageServerId[]; + setConfiguration(configuration: TLanguageServerConfigurations): void; fetchSessions(): Promise; statusUrl: string; statusCode: number; } -export interface ILanguageServerConfiguration { - /** - * The config params must be nested inside the settings keyword - */ - settings: { - [k: string]: any; - }; -} - export namespace ILanguageServerManager { export const URL_NS = 'lsp'; export interface IOptions { @@ -84,6 +80,7 @@ export namespace ILanguageServerManager { * The interval for retries, default 10 seconds. */ retriesInterval?: number; + console: ILSPLogConsole; } export interface IGetServerIdOptions { language?: TLanguageId; diff --git a/packages/jupyterlab-lsp/src/virtual/editor.spec.ts b/packages/jupyterlab-lsp/src/virtual/editor.spec.ts index 415f9d7bb..ed7f2baa1 100644 --- a/packages/jupyterlab-lsp/src/virtual/editor.spec.ts +++ b/packages/jupyterlab-lsp/src/virtual/editor.spec.ts @@ -30,7 +30,9 @@ describe('VirtualEditor', () => { '/home/username/project/.virtual_documents' ); - const LANGSERVER_MANAGER = new MockLanguageServerManager({}); + const LANGSERVER_MANAGER = new MockLanguageServerManager({ + console: new BrowserConsole() + }); const CONNECTION_MANAGER = new DocumentConnectionManager({ language_server_manager: LANGSERVER_MANAGER, console: new BrowserConsole() diff --git a/python_packages/jupyter_lsp/jupyter_lsp/schema/schema.json b/python_packages/jupyter_lsp/jupyter_lsp/schema/schema.json index 67fd35b75..047e98a2a 100644 --- a/python_packages/jupyter_lsp/jupyter_lsp/schema/schema.json +++ b/python_packages/jupyter_lsp/jupyter_lsp/schema/schema.json @@ -34,7 +34,7 @@ "type": "string" }, "language-list": { - "description": "languages supported by this Language Server", + "description": "languages supported by this Language Server; lowercase names", "items": { "type": "string" }, From b34e9601784ac3541bb9b81fd6c6ae454aa00944 Mon Sep 17 00:00:00 2001 From: krassowski Date: Thu, 29 Apr 2021 23:34:49 +0100 Subject: [PATCH 2/4] Fix failing test, update changelog, add serverIdentifier --- CHANGELOG.md | 10 ++++++++-- packages/jupyterlab-lsp/src/components/statusbar.tsx | 7 ++++--- packages/jupyterlab-lsp/src/connection.ts | 6 +++++- packages/jupyterlab-lsp/src/connection_manager.ts | 3 ++- .../jupyterlab-lsp/src/editor_integration/testutils.ts | 3 ++- 5 files changed, 21 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f1af6cee..d9a8ad562 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,14 @@ ## Changelog -### (unreleased) +### `@krassowski/jupyterlab-lsp 3.7.0` (unreleased) -- Add ability to deactivate Kernel completions or LSP completion through the settings. +- features: + + - add ability to deactivate Kernel completions or LSP completion through the settings ([#586], thanks @Carreau) + - allow to set a priority for LSP server, allowing to choose which server to use when multiple servers are installed ([#588]) + +[#586]: https://github.com/krassowski/jupyterlab-lsp/pull/586 +[#588]: https://github.com/krassowski/jupyterlab-lsp/pull/588 ### `jupyter-lsp 1.2.0` (2021-04-26) diff --git a/packages/jupyterlab-lsp/src/components/statusbar.tsx b/packages/jupyterlab-lsp/src/components/statusbar.tsx index 3349d4176..bcdaa4ae9 100644 --- a/packages/jupyterlab-lsp/src/components/statusbar.tsx +++ b/packages/jupyterlab-lsp/src/components/statusbar.tsx @@ -493,7 +493,7 @@ export namespace LSPStatus { }); // TODO server.status === "started" ? // TODO update once multiple servers are allowed - if (matchedServers[0] === id) { + if (matchedServers.length && matchedServers[0] === id) { return true; } } @@ -590,10 +590,11 @@ export namespace LSPStatus { detected_documents.forEach((document, uri) => { let connection = this._connection_manager.connections.get(uri); - let server_id = this._connection_manager.language_server_manager.getMatchingServers( + let server_ids = this._connection_manager.language_server_manager.getMatchingServers( { language: document.language } ); - if (server_id !== null) { + + if (server_ids.length !== 0) { documents_with_known_servers.add(document); } if (!connection) { diff --git a/packages/jupyterlab-lsp/src/connection.ts b/packages/jupyterlab-lsp/src/connection.ts index bc35c6f87..1d28cd2db 100644 --- a/packages/jupyterlab-lsp/src/connection.ts +++ b/packages/jupyterlab-lsp/src/connection.ts @@ -12,13 +12,17 @@ import * as lsProtocol from 'vscode-languageserver-protocol'; import { until_ready } from './utils'; -interface ILSPOptions extends ILspOptions {} +interface ILSPOptions extends ILspOptions { + serverIdentifier?: string; +} export class LSPConnection extends LspWsConnection { protected documentsToOpen: IDocumentInfo[]; + public serverIdentifier: string; constructor(options: ILSPOptions) { super(options); + this.serverIdentifier = options?.serverIdentifier; this.documentsToOpen = []; } diff --git a/packages/jupyterlab-lsp/src/connection_manager.ts b/packages/jupyterlab-lsp/src/connection_manager.ts index ba5f73029..66bc06b28 100644 --- a/packages/jupyterlab-lsp/src/connection_manager.ts +++ b/packages/jupyterlab-lsp/src/connection_manager.ts @@ -444,7 +444,8 @@ namespace Private { const connection = new LSPConnection({ languageId: language, serverUri: uris.server, - rootUri: uris.base + rootUri: uris.base, + serverIdentifier: language_server_id }); // TODO: remove remaining unbounded users of connection.on connection.setMaxListeners(999); diff --git a/packages/jupyterlab-lsp/src/editor_integration/testutils.ts b/packages/jupyterlab-lsp/src/editor_integration/testutils.ts index 2667dc079..6b0bbb80f 100644 --- a/packages/jupyterlab-lsp/src/editor_integration/testutils.ts +++ b/packages/jupyterlab-lsp/src/editor_integration/testutils.ts @@ -242,7 +242,8 @@ function FeatureSupport(Base: TBase) { return new LSPConnection({ languageId: this.document_options.language, serverUri: 'ws://localhost:8080', - rootUri: 'file:///unit-test' + rootUri: 'file:///unit-test', + serverIdentifier: 'pylsp' }); } From 10aa2bb740f5d788f368eea7a7c7e9ff1fb5a6c2 Mon Sep 17 00:00:00 2001 From: krassowski Date: Tue, 11 May 2021 18:16:32 +0100 Subject: [PATCH 3/4] Address review comments, back down on lowercase --- docs/Configuring.ipynb | 3 +- packages/jupyterlab-lsp/schema/plugin.json | 2 +- .../src/editor_integration/testutils.ts | 6 ++- packages/jupyterlab-lsp/src/manager.ts | 53 ++++++++++--------- .../jupyter_lsp/schema/schema.json | 2 +- 5 files changed, 36 insertions(+), 30 deletions(-) diff --git a/docs/Configuring.ipynb b/docs/Configuring.ipynb index f66064853..6917786f4 100644 --- a/docs/Configuring.ipynb +++ b/docs/Configuring.ipynb @@ -66,8 +66,7 @@ " `mkdir \"new directory\"` should be split into `[\"mkdir\", \"new directory\"]`;\n", " If you have Python installed, you can use `shlex.split(\"your command\")` to\n", " get such an array.\n", - "- the `languages` which the language server will respond to (language names\n", - " should be given in lowercase), and\n", + "- the `languages` which the language server will respond to, and\n", "- the schema `version` of the spec (currently `2`)\n", "- `mime_types` by which the notebooks and files will be matched to the language\n", " server:\n", diff --git a/packages/jupyterlab-lsp/schema/plugin.json b/packages/jupyterlab-lsp/schema/plugin.json index 3eb772151..0263e3649 100644 --- a/packages/jupyterlab-lsp/schema/plugin.json +++ b/packages/jupyterlab-lsp/schema/plugin.json @@ -19,7 +19,7 @@ }, "priority": { "title": "Priority of the server", - "description": "When multiple servers match specific document/language, the server with higher priority will be used", + "description": "When multiple servers match specific document/language, the server with the highest priority will be used", "type": "number", "default": 50 } diff --git a/packages/jupyterlab-lsp/src/editor_integration/testutils.ts b/packages/jupyterlab-lsp/src/editor_integration/testutils.ts index 6b0bbb80f..30ef6f9fb 100644 --- a/packages/jupyterlab-lsp/src/editor_integration/testutils.ts +++ b/packages/jupyterlab-lsp/src/editor_integration/testutils.ts @@ -54,6 +54,8 @@ import { EditorAdapter } from './editor_adapter'; import createNotebookPanel = NBTestUtils.createNotebookPanel; import IEditor = CodeEditor.IEditor; +const DEFAULT_SERVER_ID = 'pylsp'; + export interface ITestEnvironment { document_options: VirtualDocument.IOptions; @@ -71,7 +73,7 @@ export interface ITestEnvironment { export class MockLanguageServerManager extends LanguageServerManager { async fetchSessions() { this._sessions = new Map(); - this._sessions.set('pylsp', { + this._sessions.set(DEFAULT_SERVER_ID, { spec: { languages: ['python'] } @@ -243,7 +245,7 @@ function FeatureSupport(Base: TBase) { languageId: this.document_options.language, serverUri: 'ws://localhost:8080', rootUri: 'file:///unit-test', - serverIdentifier: 'pylsp' + serverIdentifier: DEFAULT_SERVER_ID }); } diff --git a/packages/jupyterlab-lsp/src/manager.ts b/packages/jupyterlab-lsp/src/manager.ts index 0fe38ccee..2a4e490c9 100644 --- a/packages/jupyterlab-lsp/src/manager.ts +++ b/packages/jupyterlab-lsp/src/manager.ts @@ -52,40 +52,45 @@ export class LanguageServerManager implements ILanguageServerManager { this._configuration = configuration; } + protected _comparePriorities(a: TLanguageServerId, b: TLanguageServerId) { + const DEFAULT_PRIORITY = 50; + const a_priority = this._configuration[a]?.priority ?? DEFAULT_PRIORITY; + const b_priority = this._configuration[b]?.priority ?? DEFAULT_PRIORITY; + if (a_priority == b_priority) { + this.console.warn( + `Two matching servers: ${a} and ${b} have the same priority; choose which one to use by changing the priority in Advanced Settings Editor` + ); + return a.localeCompare(b); + } + // higher priority = higher in the list (descending order) + return b_priority - a_priority; + } + getMatchingServers(options: ILanguageServerManager.IGetServerIdOptions) { + if (!options.language) { + this.console.error( + 'Cannot match server by language: language not available; ensure that kernel and specs provide language and MIME type' + ); + return []; + } + const matchingSessionsKeys: TLanguageServerId[] = []; - const config = this._configuration; + const lowerCaseLanguage = options.language.toLocaleLowerCase(); // most things speak language // if language is not known, it is guessed based on MIME type earlier // so some language should be available by now (which can be not obvious, e.g. "plain" for txt documents) for (const [key, session] of this._sessions.entries()) { - if (options.language) { - if ( - session.spec.languages.indexOf( - options.language.toLocaleLowerCase() - ) !== -1 - ) { - matchingSessionsKeys.push(key); - } - } else { - this.console.error( - 'Cannot match server by language: language not available; ensure that kernel and specs provide language and MIME type' - ); + if ( + session.spec.languages.some( + language => language.toLocaleLowerCase() == lowerCaseLanguage + ) + ) { + matchingSessionsKeys.push(key); } } - return matchingSessionsKeys.sort((a, b) => { - const a_priority = config[a]?.priority ?? 50; - const b_priority = config[b]?.priority ?? 50; - if (a_priority == b_priority) { - this.console.warn( - `Two matching servers: ${a} and ${b} have the same priority; choose which one to use by changing the priority in Advanced Settings Editor` - ); - } - // higher priority = higher in the list (descending order) - return b_priority - a_priority; - }); + return matchingSessionsKeys.sort(this._comparePriorities); } get statusCode(): number { diff --git a/python_packages/jupyter_lsp/jupyter_lsp/schema/schema.json b/python_packages/jupyter_lsp/jupyter_lsp/schema/schema.json index 047e98a2a..67fd35b75 100644 --- a/python_packages/jupyter_lsp/jupyter_lsp/schema/schema.json +++ b/python_packages/jupyter_lsp/jupyter_lsp/schema/schema.json @@ -34,7 +34,7 @@ "type": "string" }, "language-list": { - "description": "languages supported by this Language Server; lowercase names", + "description": "languages supported by this Language Server", "items": { "type": "string" }, From 6d7647d02369db88de41b4e55a7b6804c3bc7886 Mon Sep 17 00:00:00 2001 From: krassowski Date: Tue, 11 May 2021 18:30:34 +0100 Subject: [PATCH 4/4] Add minimum --- packages/jupyterlab-lsp/schema/plugin.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/jupyterlab-lsp/schema/plugin.json b/packages/jupyterlab-lsp/schema/plugin.json index 0263e3649..8893348fa 100644 --- a/packages/jupyterlab-lsp/schema/plugin.json +++ b/packages/jupyterlab-lsp/schema/plugin.json @@ -21,7 +21,8 @@ "title": "Priority of the server", "description": "When multiple servers match specific document/language, the server with the highest priority will be used", "type": "number", - "default": 50 + "default": 50, + "minimum": 1 } } }