Skip to content

Commit

Permalink
extension: do not show error messages in startCueLsp
Browse files Browse the repository at this point in the history
startCueLsp is called in response to configuration changes or when
manually attempting to start the LSP via a user command.

We cannot make assumptions about the calling context in startCueLsp as
to whether to show error messages or not. Instead, we should return a
rejected promise and let the caller decide what to do.

This CL implements that change.

Similarly, when handling a configuration state change, per the doc
comment for the handler, we need to show an error message to the user in
case of an error, rather than returning a rejected promise (because
the caller is not handling rejected promises, now confirmed via
experience).

This CL also implements that change.

Signed-off-by: Paul Jolly <[email protected]>
Change-Id: Ib9cd8523ed30d8701a694e41c7edf207f3ef376f
Reviewed-on: https://review.gerrithub.io/c/cue-lang/vscode-cue/+/1206557
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>
  • Loading branch information
myitcv committed Jan 2, 2025
1 parent 809a2db commit 2a94d71
Showing 1 changed file with 18 additions and 10 deletions.
28 changes: 18 additions & 10 deletions extension/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export class Extension {
// Not visible to users via the command palette
this.registerCommand(copyStatusVersionToClipboardCmd, this.copyStatusVersionToClipboard);

this.statusBarItem = vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Right, 100);
this.statusBarItem = vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Right);

// TODO(myitcv): in the early days of 'cue lsp', it might be worthwhile
// adding a command that toggles the enabled-ness of the LSP in the active
Expand Down Expand Up @@ -282,7 +282,7 @@ export class Extension {
// for the updated version string.
let [cueCommand, err] = await ve(this.absCueCommand(this.config!.cueCommand));
if (err !== null) {
return Promise.reject(err);
return this.showErrorMessage(`${err}`);
}
let cueVersion: Cmd = {
Args: [cueCommand!, 'version']
Expand All @@ -295,14 +295,14 @@ export class Extension {
} else {
msgSuffix = cueVersion.Stderr!;
}
return Promise.reject(new Error(`failed to run ${JSON.stringify(cueVersion)}: ${msgSuffix}`));
return this.showErrorMessage(`failed to run ${JSON.stringify(cueVersion)}: ${msgSuffix}`);
}
let versionOutput = cueVersion.Stdout!.trim();
const versionRegex = /^cue version (.*)/m;
let match = versionOutput.match(versionRegex);
if (!match) {
return Promise.reject(
new Error(`failed to parse version output from ${JSON.stringify(cueVersion)}: ${JSON.stringify(versionOutput)}`)
return this.showErrorMessage(
`failed to parse version output from ${JSON.stringify(cueVersion)}: ${JSON.stringify(versionOutput)}`
);
}
this.cueVersion = match[1];
Expand Down Expand Up @@ -385,7 +385,10 @@ export class Extension {
return this.showErrorMessage(`useLanguageServer is configured to false`);
}
this.manualLspStop = false;
return this.startCueLsp('manually');
let [, err] = await ve(this.startCueLsp('manually'));
if (err !== null) {
return this.showErrorMessage(`${err}`);
}
};

// cmdStopLSP is used to explicitly stop the LSP server.
Expand All @@ -395,7 +398,10 @@ export class Extension {
}

this.manualLspStop = true;
return this.stopCueLsp('manually');
let [, err] = await ve(this.stopCueLsp('manually'));
if (err !== null) {
return this.showErrorMessage(`${err}`);
}
};

// startCueLsp is responsible for starting 'cue lsp'. It stops an existing client
Expand Down Expand Up @@ -451,11 +457,13 @@ export class Extension {
[, err] = await ve(osexecRun(cueHelpLsp));
if (err !== null) {
if (isErrnoException(err)) {
return this.showErrorMessage(`failed to run ${JSON.stringify(cueHelpLsp)}: ${err}`);
return Promise.reject(new Error(`failed to run ${JSON.stringify(cueHelpLsp)}: ${err}`));
}
// Probably running an early version of CUE with no LSP support.
return this.showErrorMessage(
`the version of cmd/cue at ${JSON.stringify(cueCommand)} does not support 'cue lsp'. Please upgrade to at least v0.11.0`
return Promise.reject(
new Error(
`the version of cmd/cue at ${JSON.stringify(cueCommand)} does not support 'cue lsp'. Please upgrade to at least v0.11.0`
)
);
}

Expand Down

0 comments on commit 2a94d71

Please sign in to comment.