Skip to content

Commit

Permalink
Avoid awaiting VS Code popup notifications (#557)
Browse files Browse the repository at this point in the history
## Summary

This PR fixes a bug to avoid awaiting the VS Code popup notifications.

### How to reproduce this?

For posterity, this is how to reproduce this bug:
1. Keep the extension setting to only contain:
```json
{
	"ruff.nativeServer": true
}
```
2. Add the following additional setting which is incompatible with the
native server:
```json
{
	"ruff.nativeServer": true,
	"ruff.format.args": ["--line-length=10"]
}
```
3. Saving the new settings will trigger a restart and a notification
will popup which warns you about this incompatible setting (**Do NOT
dismiss the notification window**)
4. Remove the `ruff.format.args` and save the settings which should then
trigger a restart again

But, the restart triggered by (3) never completed because the
notification was awaited and the user never dismissed it. This is where
the extension hangs.

### Solution

I've updated all `vscode.window.show(Error|Warning)Message` calls to not
be awaited and instead use the callback mechanism if there's a need to
respond to user's selection.

## Test Plan

Follow the same steps as above and make sure that the restart happens.


https://github.com/user-attachments/assets/28781673-9472-4ad7-9da8-d15302877307
  • Loading branch information
dhruvmanila authored Jul 24, 2024
1 parent 51f5a57 commit 4060b01
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 19 deletions.
31 changes: 16 additions & 15 deletions src/common/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ async function findRuffBinaryPath(
const stdout = await executeFile(settings.interpreter[0], [FIND_RUFF_BINARY_SCRIPT_PATH]);
ruffBinaryPath = stdout.trim();
} catch (err) {
await vscode.window
vscode.window
.showErrorMessage(
"Unexpected error while trying to find the Ruff binary. See the logs for more details.",
"Show Logs",
Expand Down Expand Up @@ -168,7 +168,7 @@ async function createNativeServer(
MINIMUM_NATIVE_SERVER_VERSION,
)}, but found ${versionToString(ruffVersion)} at ${ruffBinaryPath} instead`;
traceError(message);
await vscode.window.showErrorMessage(message);
vscode.window.showErrorMessage(message);
return Promise.reject();
}

Expand Down Expand Up @@ -246,15 +246,16 @@ async function createLegacyServer(
return new LanguageClient(serverId, serverName, serverOptions, clientOptions);
}

async function showWarningMessageWithLogs(message: string, outputChannel: LogOutputChannel) {
const selection = await vscode.window.showWarningMessage(message, "Show Logs");
if (selection) {
outputChannel.show();
}
function showWarningMessageWithLogs(message: string, outputChannel: LogOutputChannel) {
vscode.window.showWarningMessage(message, "Show Logs").then((selection) => {
if (selection) {
outputChannel.show();
}
});
}

async function legacyServerSettingsWarning(settings: string[], outputChannel: LogOutputChannel) {
await showWarningMessageWithLogs(
function legacyServerSettingsWarning(settings: string[], outputChannel: LogOutputChannel) {
showWarningMessageWithLogs(
"Unsupported settings used with the native server. Refer to the logs for more details.",
outputChannel,
);
Expand All @@ -263,12 +264,12 @@ async function legacyServerSettingsWarning(settings: string[], outputChannel: Lo
);
}

async function nativeServerSettingsWarning(
function nativeServerSettingsWarning(
settings: string[],
outputChannel: LogOutputChannel,
suggestion?: string,
) {
await showWarningMessageWithLogs(
showWarningMessageWithLogs(
"Unsupported settings used with the legacy server (ruff-lsp). Refer to the logs for more details.",
outputChannel,
);
Expand Down Expand Up @@ -301,22 +302,22 @@ async function resolveNativeServerSetting(
case true:
const legacyServerSettings = getUserSetLegacyServerSettings(serverId, workspace);
if (legacyServerSettings.length > 0) {
await legacyServerSettingsWarning(legacyServerSettings, outputChannel);
legacyServerSettingsWarning(legacyServerSettings, outputChannel);
}
return { useNativeServer: true, executable };
case "off":
case false:
if (!vscode.workspace.isTrusted) {
const message =
"Cannot use the legacy server (ruff-lsp) in an untrusted workspace; switching to the native server using the bundled executable.";
await vscode.window.showWarningMessage(message);
vscode.window.showWarningMessage(message);
traceWarn(message);
return { useNativeServer: true, executable };
}

let nativeServerSettings = getUserSetNativeServerSettings(serverId, workspace);
if (nativeServerSettings.length > 0) {
await nativeServerSettingsWarning(nativeServerSettings, outputChannel);
nativeServerSettingsWarning(nativeServerSettings, outputChannel);
}
return { useNativeServer: false, executable };
case "auto":
Expand Down Expand Up @@ -346,7 +347,7 @@ async function resolveNativeServerSetting(
);
let nativeServerSettings = getUserSetNativeServerSettings(serverId, workspace);
if (nativeServerSettings.length > 0) {
await nativeServerSettingsWarning(
nativeServerSettingsWarning(
nativeServerSettings,
outputChannel,
"Please remove these settings or set 'nativeServer' to 'on' to use the native server",
Expand Down
8 changes: 4 additions & 4 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>
};

await lsClient.sendRequest(ExecuteCommandRequest.type, params).then(undefined, async () => {
await vscode.window.showErrorMessage(
vscode.window.showErrorMessage(
"Failed to apply Ruff fixes to the document. Please consider opening an issue with steps to reproduce.",
);
});
Expand All @@ -206,7 +206,7 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>
};

await lsClient.sendRequest(ExecuteCommandRequest.type, params).then(undefined, async () => {
await vscode.window.showErrorMessage(
vscode.window.showErrorMessage(
"Failed to apply Ruff formatting to the document. Please consider opening an issue with steps to reproduce.",
);
});
Expand All @@ -231,7 +231,7 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>
};

await lsClient.sendRequest(ExecuteCommandRequest.type, params).then(undefined, async () => {
await vscode.window.showErrorMessage(
vscode.window.showErrorMessage(
`Failed to apply Ruff fixes to the document. Please consider opening an issue at ${issueTracker} with steps to reproduce.`,
);
});
Expand All @@ -247,7 +247,7 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>
};

await lsClient.sendRequest(ExecuteCommandRequest.type, params).then(undefined, async () => {
await vscode.window.showErrorMessage("Failed to print debug information.");
vscode.window.showErrorMessage("Failed to print debug information.");
});
}),
registerLanguageStatusItem(serverId, serverName, `${serverId}.showLogs`),
Expand Down

0 comments on commit 4060b01

Please sign in to comment.