Allow to disable LSP or Kernel completions independently.

[Carreau]

Some user love LSP for many of its features, but prefer to keep
completions via the kernel.

This adds two boolean flags in the configuration to not send completions
request to LSP and not send completions requests to the kernel.

References

#440 - I'm not sure how to toggle it on a per-notebook basis; I can likely add local state to LSPConnector which I guess is persisted on a per notebook basis but would welcome guidance on the best place to store that.

Code changes

• Add 2 boolean to the setting.
• turns those two booleans into properties on the LSPConnector,
• when false skip requesting completions for the corresponding source.

User-facing changes

None by default. User may disable one or two completer sources.

Backwards-incompatible changes

None

Chores

• linted
• tested - manually for now I can try to add an automatic test.
• documented
• changelog entry

Note: I haven't touched typescript and frontend in years, so I'm likely doing some things wrong;

[bollwyvl]

@krassowski would have to weigh in on this: if both of ours are turned off, is there some other place they might come from?

[krassowski]

If am not mistaken we are taking an absolute control of completer and nothing else can provide completions just yet, but this will change very soon allowing Kite and Snippets to provide custom completion items to our endpoint (and afterwards we will move it to JupyterLab for 4.0).

By very soon I mean not earlier than end of May (sadly won't have time to on it earlier).

[bollwyvl]

Yes, we have LSP in the top-level description, and the name of the project, sure, but perhaps useLangugeServerCompletions. And certainly to the language server

[bollwyvl]

Actually, as i expand this big-ol-file to look below... we already have kernelCompletionsFirst. I wonder if these could be condensed into something like the below, and put a 4.0 deprecation warning on KCF:

completionSources:
  description: The sources from which to fetch completions. Order determines tiebreakers. An empty list will disable completions provided by this plugin.
  type: array
  default: [languageServer, kernel]
  uniqueItems: true
  items:
    enum: [languageServer, kernel]

Thoughts @krassowski

[krassowski]

The completionSources would work for me to but I would want to avoid enum to allow for completions from other sources too. In the end the plan is that it will be all removed when JupyterLab 4.0 is released and takes care of that for us (this is when we PR a multi-source completion mechanism which will prevent extension clashes).

[Carreau]

I'm fine with anything you like.
I also thought that bool could at some point in the future allow to generate checkboxes, and easier to toggle via an action, but up to you.

[bollwyvl]

Re: notebook. Gah. Yeah. Been thinking about this for a long time.

Having implemented this in a number of places, I'd love it if per-notebook settings just worked across every plugin in a jupyterlab-settings key that followed the structure of overrides.json (no JSON5, please), but that might be beyond us to be able to declare as a thing.

Alternately, we've talked about having per-(glob|fnmatch|regex) (probably the latter) settings which could be configured per-plugin.

At any rate, I don't think we tackle that on this PR, but should probably hoist it (at least) to an issue on this repo, if not to core.

[bollwyvl]

Commented here: jupyterlab/jupyterlab#8148 (comment)

[bollwyvl]

Woo green!

[krassowski]

Thank you @Carreau. I wonder if having this as a single setting with an array of completion sources to disable (e.g. disableCompletionsFrom) would make sense given that we will want to make it possible for other extensions (Kite, snippets) to integrate well with our completions mechanism (and ideally move the motion of having multiple completion sources to the JupyterLab core).

[Carreau]

I wonder if having this as a single setting with an array of completion sources to disable (e.g. disableCompletionsFrom) would make sense given that we will want to make it possible for other extensions (Kite, snippets) to integrate well with our completions mechanism (and ideally move the motion of having multiple completion sources to the JupyterLab core).

That was my initial thought as well, either an include and exclude list. as I pointed above that would be fine with me; though it might be a bit harder to provide actions to enable disable, I mean that's only a couple of lines of code so I can switch to that.

[Carreau]

Updated to take comments into accounts.

[krassowski]

Thank you. The one final thing I would like your feedback on is naming of the options. I already introduced a concept of "CompletionSource" to make it easy to add new sources and to display the name of the source in the interface to make user aware why they do not see e.g. documentation for a specific completion item. These are currently named "Kernel" and "LSP":

https://github.com/krassowski/jupyterlab-lsp/blob/39010530eba400bffc56282709343e9fcf8bc778/packages/jupyterlab-lsp/src/features/completion/completion_handler.ts#L398-L402

https://github.com/krassowski/jupyterlab-lsp/blob/39010530eba400bffc56282709343e9fcf8bc778/packages/jupyterlab-lsp/src/features/completion/completion_handler.ts#L451-L455

Should we align the source "name" that is displayed with the identifier in settings? I am not saying that they need to be identical. Just curious if you have thoughts whether using "languageServer" (identifier) + "Language Server" (name in UI) is actually better than using "lsp" (idnetifier) and "LSP" (name in the UI). Both would work for me.

[bollwyvl]

I'm 👍 to languageServer and Language Server. Better still would be if we could get the actual name of the server/kernel in the UI. At the end of it, they're both being strictly transformed into LSP messages.

Further, if the jiggery-pokery around the transorm_response could make its way inside the kernel thingy, so they both returned LSP CompletionResponse (or whatever it's called), that would further simplify the code, and remove (some) instances of special casing.

[Carreau]

I've change the options "LSP" and "Kernel" for the options values; I like "LSP", it's short and hard to misspell – which is good especially if you have to type it in the settings.

I don't have particular preferences otherwise.

@bollwyvl I agree with the refactor, but would prefer to do it separately, as I'm not yet that comfortable with typescript. I was thinking that a dict of names/promises or Promise.resolve(null) could be an easy generic way of having multiple providers and unconditionally merge (even if there is only one provider) would be cleaner.

[bollwyvl]

LGTM

[krassowski]

Thank you!