diff --git a/CHANGELOG.md b/CHANGELOG.md index d3979e4e5..4cbcfa36c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,11 @@ - Fix Incorrect semantic highlighting of `external` declarations https://github.com/rescript-lang/rescript-vscode/pull/517 +#### :nail_care: Polish + +- Don't restart server on change configuration `rescript.settings`. https://github.com/rescript-lang/rescript-vscode/pull/528 +- Simplifies the `initializationOptions`. https://github.com/rescript-lang/rescript-vscode/pull/528 + ## v1.4.2 #### :bug: Bug Fix diff --git a/README.md b/README.md index 7b6c9eaf2..be6584753 100644 --- a/README.md +++ b/README.md @@ -107,7 +107,7 @@ You'll find all ReScript specific settings under the scope `rescript.settings`. | Prompt to Start Build | If there's no ReScript build running already in the opened project, the extension will prompt you and ask if you want to start a build automatically. You can turn off this automatic prompt via the setting `rescript.settings.askToStartBuild`. | | ReScript Binary Path | The extension will look for the existence of a `/node_modules/.bin/rescript` file and use its directory as the `binaryPath`. If it does not find it at the project root (which is where the nearest `bsconfig.json` resides), it goes up folders in the filesystem recursively until it either finds it (often the case in monorepos) or hits the top level. To override this lookup process, the path can be configured explicitly using the setting `rescript.settings.binaryPath` | | Inlay Hints (experimental) | This allows an editor to place annotations inline with text to display type hints. Enable using `rescript.settings.inlayHints.enable: true` | -| Code Lens (experimental) | This tells the editor to add code lenses to function definitions, showing its full type above the definition. Enable using `rescript.settings.codeLens: true` | +| Code Lens (experimental) | This tells the editor to add code lenses to function definitions, showing its full type above the definition. Enable using `rescript.settings.codeLens.enable: true` | | Autostarting the Code Analyzer | The Code Analyzer needs to be started manually by default. However, you can configure the extension to start the Code Analyzer automatically via the setting `rescript.settings.autoRunCodeAnalysis`. | **Default settings:** @@ -129,7 +129,7 @@ You'll find all ReScript specific settings under the scope `rescript.settings`. "rescript.settings.inlayHints.maxLength": 25 // Enable (experimental) code lens for function definitions. -"rescript.settings.codeLens": true +"rescript.settings.codeLens.enable": true ``` ## 🚀 Code Analyzer diff --git a/client/src/extension.ts b/client/src/extension.ts index 7bd85b918..d9869098c 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -96,9 +96,7 @@ export function activate(context: ExtensionContext) { // We'll send the initial configuration in here, but this might be // problematic because every consumer of the LS will need to mimic this. // We'll leave it like this for now, but might be worth revisiting later on. - initializationOptions: { - extensionConfiguration: workspace.getConfiguration("rescript.settings"), - }, + initializationOptions: workspace.getConfiguration("rescript.settings"), }; const client = new LanguageClient( @@ -235,21 +233,14 @@ export function activate(context: ExtensionContext) { // Start the client. This will also launch the server client.start(); - // Restart the language client automatically when certain configuration - // changes. These are typically settings that affect the capabilities of the - // language client, and because of that requires a full restart. context.subscriptions.push( workspace.onDidChangeConfiguration(({ affectsConfiguration }) => { - // Put any configuration that, when changed, requires a full restart of - // the server here. That will typically be any configuration that affects - // the capabilities declared by the server, since those cannot be updated - // on the fly, and require a full restart with new capabilities set when - // initializing. if ( - affectsConfiguration("rescript.settings.inlayHints") || - affectsConfiguration("rescript.settings.codeLens") + affectsConfiguration("rescript.settings") ) { - commands.executeCommand("rescript-vscode.restart_language_server"); + client.sendNotification( + "workspace/didChangeConfiguration", + { settings: workspace.getConfiguration("rescript.settings") }).catch((err) => { window.showErrorMessage(String(err)) }) } }) ); diff --git a/server/src/constants.ts b/server/src/constants.ts index ddce1cf98..ed3def181 100644 --- a/server/src/constants.ts +++ b/server/src/constants.ts @@ -52,4 +52,3 @@ export let bsconfigModuleDefault = "commonjs"; export let bsconfigSuffixDefault = ".js"; export let configurationRequestId = "rescript_configuration_request"; -export let pullConfigurationInterval = 10_000; diff --git a/server/src/server.ts b/server/src/server.ts index 76c78e0b7..67a90477c 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -13,6 +13,7 @@ import { InitializeParams, InlayHintParams, CodeLensParams, + DidChangeConfigurationParams, } from "vscode-languageserver-protocol"; import * as utils from "./utils"; import * as codeActions from "./codeActions"; @@ -24,28 +25,31 @@ import { ChildProcess } from "child_process"; import { WorkspaceEdit } from "vscode-languageserver"; import { filesDiagnostics } from "./utils"; -interface extensionConfiguration { +interface initializationOptions { askToStartBuild: boolean; inlayHints: { enable: boolean; maxLength: number | null; }; - codeLens: boolean; + codeLens: { + enable: boolean; + }; binaryPath: string | null; } // All values here are temporary, and will be overridden as the server is // initialized, and the current config is received from the client. -let extensionConfiguration: extensionConfiguration = { +let initializationOptions: initializationOptions = { askToStartBuild: true, inlayHints: { enable: false, maxLength: 25, }, - codeLens: false, + codeLens: { + enable: false + }, binaryPath: null, }; -let pullConfigurationPeriodically: NodeJS.Timeout | null = null; // https://microsoft.github.io/language-server-protocol/specification#initialize // According to the spec, there could be requests before the 'initialize' request. Link in comment tells how to handle them. @@ -99,9 +103,9 @@ let findBinaryDirPathFromProjectRoot = ( }; let getBinaryDirPath = (projectRootPath: p.DocumentUri) => - extensionConfiguration.binaryPath == null + initializationOptions.binaryPath == null ? findBinaryDirPathFromProjectRoot(projectRootPath) - : extensionConfiguration.binaryPath; + : initializationOptions.binaryPath; let findRescriptBinary = (projectRootPath: p.DocumentUri) => utils.findRescriptBinary(getBinaryDirPath(projectRootPath)); @@ -231,10 +235,10 @@ let compilerLogsWatcher = chokidar .on("all", (_e, changedPath) => { sendUpdatedDiagnostics(); sendCompilationFinishedMessage(); - if (extensionConfiguration.inlayHints.enable === true) { + if (initializationOptions.inlayHints?.enable === true) { sendInlayHintsRefresh(); } - if (extensionConfiguration.codeLens === true) { + if (initializationOptions.codeLens?.enable === true) { sendCodeLensRefresh(); } }); @@ -279,7 +283,7 @@ let openedFile = (fileUri: string, fileContent: string) => { let bsbLockPath = path.join(projectRootPath, c.bsbLock); if ( projectRootState.hasPromptedToStartBuild === false && - extensionConfiguration.askToStartBuild === true && + initializationOptions.askToStartBuild === true && !fs.existsSync(bsbLockPath) ) { // TODO: sometime stale .bsb.lock dangling. bsb -w knows .bsb.lock is @@ -399,6 +403,15 @@ function hover(msg: p.RequestMessage) { } function inlayHint(msg: p.RequestMessage) { + + if (!initializationOptions.inlayHints?.enable) { + return { + jsonrpc: c.jsonrpcVersion, + id: msg.id, + result: null, + }; + } + const params = msg.params as p.InlayHintParams; const filePath = fileURLToPath(params.textDocument.uri); @@ -409,7 +422,7 @@ function inlayHint(msg: p.RequestMessage) { filePath, params.range.start.line, params.range.end.line, - extensionConfiguration.inlayHints.maxLength, + initializationOptions.inlayHints.maxLength, ], msg ); @@ -426,6 +439,15 @@ function sendInlayHintsRefresh() { } function codeLens(msg: p.RequestMessage) { + + if (!initializationOptions.codeLens?.enable) { + return { + jsonrpc: c.jsonrpcVersion, + id: msg.id, + result: null, + }; + } + const params = msg.params as p.CodeLensParams; const filePath = fileURLToPath(params.textDocument.uri); @@ -569,24 +591,6 @@ function documentSymbol(msg: p.RequestMessage) { return response; } -function askForAllCurrentConfiguration() { - // https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workspace_configuration - let params: p.ConfigurationParams = { - items: [ - { - section: "rescript.settings", - }, - ], - }; - let req: p.RequestMessage = { - jsonrpc: c.jsonrpcVersion, - id: c.configurationRequestId, - method: p.ConfigurationRequest.type.method, - params, - }; - send(req); -} - function semanticTokens(msg: p.RequestMessage) { // https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_semanticTokens let params = msg.params as p.SemanticTokensParams; @@ -975,8 +979,8 @@ function onMessage(msg: p.Message) { let params = msg.params as p.DidCloseTextDocumentParams; closedFile(params.textDocument.uri); } else if (msg.method === DidChangeConfigurationNotification.type.method) { - // Can't seem to get this notification to trigger, but if it does this will be here and ensure we're synced up at the server. - askForAllCurrentConfiguration(); + let params = msg.params as DidChangeConfigurationParams + initializationOptions = params.settings } } else if (p.Message.isRequest(msg)) { // request message, aka client sent request and waits for our mandatory reply @@ -993,11 +997,10 @@ function onMessage(msg: p.Message) { } else if (msg.method === "initialize") { // Save initial configuration, if present let initParams = msg.params as InitializeParams; - let initialConfiguration = initParams.initializationOptions - ?.extensionConfiguration as extensionConfiguration | undefined; + let initialConfiguration = initParams.initializationOptions as initializationOptions | undefined; if (initialConfiguration != null) { - extensionConfiguration = initialConfiguration; + initializationOptions = initialConfiguration; } // send the list of features we support @@ -1035,12 +1038,10 @@ function onMessage(msg: p.Message) { // TODO: Support range for full, and add delta support full: true, }, - inlayHintProvider: extensionConfiguration.inlayHints.enable, - codeLensProvider: extensionConfiguration.codeLens - ? { - workDoneProgress: false, - } - : undefined, + inlayHintProvider: true, + codeLensProvider: { + workDoneProgress: false + } }, }; let response: p.ResponseMessage = { @@ -1050,11 +1051,6 @@ function onMessage(msg: p.Message) { }; initialized = true; - // Periodically pull configuration from the client. - pullConfigurationPeriodically = setInterval(() => { - askForAllCurrentConfiguration(); - }, c.pullConfigurationInterval); - send(response); } else if (msg.method === "initialized") { // sent from client after initialize. Nothing to do for now @@ -1082,10 +1078,6 @@ function onMessage(msg: p.Message) { stopWatchingCompilerLog(); // TODO: delete bsb watchers - if (pullConfigurationPeriodically != null) { - clearInterval(pullConfigurationPeriodically); - } - let response: p.ResponseMessage = { jsonrpc: c.jsonrpcVersion, id: msg.id, @@ -1154,10 +1146,10 @@ function onMessage(msg: p.Message) { // without their settings overriding eachother. Not a problem now though // as we'll likely only have "global" settings starting out. let [configuration] = msg.result as [ - extensionConfiguration | null | undefined + initializationOptions | null | undefined ]; if (configuration != null) { - extensionConfiguration = configuration; + initializationOptions = configuration; } } } else if (