[feature request] After clangd binary is install invoke 'clangd.restart' command

[k0zmo]

Right now, after the installation of clangd is finished, the extension prompts us to reload a window. I think we should be able to just restart the server (clangd.restart command) instead. This is much faster, especially in the context of remote/devcontainer workflow.

I don't have a full picture here but I believe we could even do it automatically, without the user input.

[HighCommander4]

I'm not the original author of this code, but taking a quick look, I think you're right that restarting the server (or to be more precise, just starting the server which has now been downloaded) should be sufficient.

Are you interested in putting together a PR that makes this change? Note that the relevant code is spread across two repos, this one and https://github.com/clangd/node-clangd/.

[k0zmo]

I can take a shoot at this.

I suppose node-clangd is used across many different editors. We need to be extra cautious when making a change there, or is the breaking change allowed? Maybe we just need to make a change in this repo, changing the handler behavior for promptReload? We don't know if other editors can cope with just restarting the server (or do we?).

[HighCommander4]

I suppose node-clangd is used across many different editors. We need to be extra cautious when making a change there, or is the breaking change allowed?

In general, breaking changes to node-clangd are fine. Its consumers should be relying on a specific version, such that upgrading to a new version requires an intentional code change on their part; if/when they do that, they can update their usage to reflect any breaking changes.

As an npm package, node-clangd does follow semantic versioning, but since the major version is 0, we do not make any guarantees about stability at this time.

That said...

Maybe we just need to make a change in this repo, changing the handler behavior for promptReload? We don't know if other editors can cope with just restarting the server (or do we?).

... for this change in particular, I think it's fine to keep the method name promptReload(), and just adjust its description to something like "reload the plugin, or continue its loading to pick up a newly installed clangd executable, prompting the user if appropriate". Different consumers can then implement the method in a way appropriate to the client.

[k0zmo]

I played around with the code. The easiest and most straightforward change would be to just change the workbench.action.reloadWindow into clangd.restart, leaving the prompt there.

I don't think we need to prompt the user anymore with invoking just the clangd.restart. Removing the prompt is not that easy, though. There's a small race condition between assigning new value to clangd.path and reading it in common.prepare(ui, ...) when we call ClangdContext.create. The function vscode.workspace.getConfiguration().update is asynchronous. From my observation it can take up to 200ms to get the newly set value using vscode.workspace.getConfiguration('clangd').get.

The most straightforward solution to this issue would be to change the set clangdPath to async function (which can't be a setter) but that would require a change in node-clangd repository and would spill the implementation details - vscode needing await after setting configuration value.

Because of above, I suggest to add a 'asynchronous condition variable' that a promptReload function can wait on to ensure the next time we call WorkspaceConfiguration.get it gets the most recent value.

[HighCommander4]

This was fixed in #749.