Add option to enable/disable clangd

[ryanplusplus]

I know that this could be done by disabling the extension for a given workspace, but that requires everyone working in the repository to do that via the VS Code GUI. This allows the extension to be disabled in the workspace settings which can be committed to the repo.

In our organization we're looking to transition from using cpptools to clangd and having an easy way to have both extensions installed but only one enabled in a given workspace will be extremely helpful for us. We can already have a workspace that has been migrated to clangd disable Intellisense via cpptools, but there's no equivalent on the clangd side that I was able to find. For workspaces that haven't been migrated, clangd and cpptools will conflict until the clangd extension is disabled manually.

[ryanplusplus]

@sam-mccall @hokein sorry to bother you, but could you or one of your associates take a quick look at this? It's pretty short :)

[edvard-pettersen-elas-ai]

I support the addition of this feature. In our organization we also use a mix between cpptools and clangd between different code bases, and it would be nice to have this setting available to lessen the overhead of going back and forth between different workspace setups.

[HighCommander4]

Thanks for the patch, this looks like a reasonable change to me.

Needs a rebase.

[HighCommander4]

Any reason not to move the check even earlier (in particular before the install.activate call, but maybe even at the top of the activate function in extension.ts, thereby avoiding the need for the added check there)?

[ryanplusplus]

No reason except my lack of familiarity with the code. Thanks for the feedback! I'll give this a go.

[ryanplusplus]

I tried this out and it's a bit more complicated than I expected. In extension.ts I can't return early because activate needs to return a ClangdExtension so at least some of that code needs to run to create and configure the context used to create the ClangdExtensionImpl. I think the safest simplification I can make that doesn't result in a half-initialized context is to do this in extension.ts:

  let shouldCheck = false;

  if (!vscode.workspace.getConfiguration('clangd').get<boolean>('enable')) {
    await clangdContext.activate(context.globalStoragePath, outputChannel);

    shouldCheck = vscode.workspace.getConfiguration('clangd').get<boolean>(
      'detectExtensionConflicts') ?? false;
  }

  if (shouldCheck) {
    const interval = setInterval(function() {
      ...

Would you like me to make this change?

[HighCommander4]

This actually raises an interesting issue. The reason activate() returns anything is that we expose an API for use by other extensions. So the question is: how should the API behave if clangd.enable is set to false.

With the current patch, it looks like we pass a null client to ClangdExtension. This is technically not permitted by the API, since the client object it exposes has type BaseLanguageClient, not BaseLanguageClient | null. However, it type-checks because we fudge the check on the nullness of ClangdContext.client itself.

I guess we already have a codepath that can leave ClangdContext.client as null, so adding another one is fine for now. But we should revisit this in a follow-up issue and consider changing the API to explicitly make the exposed client object nullable (or maybe allow a null ClangdExtension return value for activate itself).

TL;DR: Yes, please go ahead and make the described simplification.

[HighCommander4]

we should revisit this in a follow-up issue and consider changing the API to explicitly make the exposed client object nullable (or maybe allow a null ClangdExtension return value for activate itself).

I filed #721 about this.

[ryanplusplus]

Thanks again for your help. I rebased, applied the simplified change, and re-tested this.

[HighCommander4]

LGTM

[HighCommander4]

Released in 0.1.30.