Refactor completer to support external providers

[krassowski]

References

Closes #236 and closes #440. Addresses https://github.com/krassowski/jupyterlab-lsp/issues/208#issuecomment-813589346.

Very early WIP, probably broken (not tested yet). TODO:

• port changes from Improve: kernel completions in R, completions in strings and paths #686
• fallback connectors & error handling,
• allow to customize logging by swapping logger to avoid verbosity
• rethink how to do async operations:
  • dynamic resolution of details (e.g. documentation)
  • adding extra completion suggestions after the initial query (isIncomplete) and how it plays out with sorting

Code changes

• Create @krassowski/completion-manager (which hopefully - after improvements - will end up in @jupyterlab/ for JupyterLab 4.0) hosting:
  • ICompletionProviderManager token (CompletionManager is already taken by the core) - this is what other extensions should use to register custom providers (ICompletionProviderManager.register(provider))
  • ICompletionProvider - this is interface that the providers in other extensions should implement; it gets more situational awareness than the core lab offers, providing ICompletionsReply with trigger information and additional CompletionContext); it also allows to provide custom renderer (which will be used on completions from this provider) and describe the source of completions.
  • a number of interfaces extending the core counterparts, e.g. IExtendedCompletionItem, ICompletionsReply, ICompleterRenderer - this allows us to add new functionality easily before it gets added to the "core completer" (or at all for features that might never make it into the core completer)
• Split up the completer code that was long overdue a refactor (too much debt) into more manageable chunks

User-facing changes

None. All settings remain as they were; the refactor allows us to add new settings, but for simplicity sake I believe we should avoid it for now as it would be a breaking change (I'm thinking it should accompany 3.1 or 4.0 lab release as those will be breaking anyways).

Backwards-incompatible changes

Every extension that relied on our internal completer implementation (which I believe is none in the open-source realm) will be impacted.

Chores

• linted
• tested
• documented
• changelog entry

[krassowski]

Make it into an extension (enabled by default) and then disable it in lsp extension using: https://jupyterlab.readthedocs.io/en/stable/extension/extension_dev.html?#disabling-other-extensions

[krassowski]

• Show language server name here?
• Think about allowing completions from multiple language servers; these would be just different sources (one provider can provide completions from multiple sources). The change would be in using multiple connections in a loop rather than a single connection.

[hbcarlos]

Hi Michal, this is great. I love the idea of having providers!

I have a couple of questions regarding the ICompletionProviderManager:

• Why do we need to change the completer's model? Is it due to having the filters methods in the model?
  • https://github.com/krassowski/jupyterlab-lsp/blob/4a745d6e182cb35723aada356978f589762ba1ba/packages/completion-manager/src/manager.ts#L111
• Are we creating a new package in JupyterLab for the provider manager, or are we adding it to the completer package?

Thanks for starting this refactor!

[krassowski]

Are we creating a new package in JupyterLab for the provider manager, or are we adding it to the completer package?

I wanted to bring it up today on dev meeting.

Why do we need to change the completer's model? Is it due to having the filters methods in the model?

Yes, but not only. The thing is that upstream model is rigid when it comes to the type of items (CompletionHandler.ICompletionItem) and also treats them more as associate arrays rather than first-class objects; what I mean is that it is convenient to have an item that implements the interface but using getters rather than attributes, e.g. to expose lazy-loaded documentation without introducing async complexities in the completer code. Also, if a provide wants to have their custom type of the item it would be lost in the old completer and type casting with as (which is error-prone) would be needed. So instead I propose to have a enericCompleterModel:

export class GenericCompleterModel<T extends CompletionHandler.ICompletionItem> extends CompleterModel {
  public settings: GenericCompleterModel.IOptions;
  completionItems(): T[];
  /** ... **/
}

This can be seamlessly up-streamed by setting a default T to CompletionHandler.ICompletionItem I believe.

Also there are some fixes to HTML escaping.

[hbcarlos]

Why don't we try to improve the Model in the completer and move the logic for filtering to Providers. This way, every provider could decide which order to use, and there is no need to change the model. Also, I saw in some discussions the possibility of having ranks for items, so providers can insert the items ordered (exist the possibility of having developers ranking its items higher to have the first, but I think we can trust them)

[hbcarlos]

The thing is that upstream model is rigid when it comes to the type of items (CompletionHandler.ICompletionItem) and also treats them more as associate arrays rather than first-class objects

I think would be fine to change it and improve it.

[krassowski]

in the completer and move the logic for filtering to Providers. This way, every provider could decide which order to use, and there is no need to change the model. Also, I saw in some discussions the possibility of having ranks for items, so providers can insert the items ordered (exist the possibility of having developers ranking its items higher to have the first, but I think we can trust them)

Yes I was a proponent of using ranks back in the day but today I think that using sortText is better due to compatibility with the rest of the world (LSP) and greater flexibility (want to put something on front/end? just slap some a/z prefixes).

I see some benefits of separating filter from sort but I am not fully convinced yet. Indeed providers could implement their own filter, but this might confuse the user due to potentially inconsistent behaviour. I believe we should use harmonized filterText field and let users tweak the details (like whether to use case-sensitive filtering or not). Maybe, in addition to that we could indeed allow to dynamically dispatch filtering to providers that do have a filter?(item: T, {caseSensitive: boolean, <other user options in the future>}) method.

[krassowski]

CC @edzkite as this might make your code base so much cleaner once this gets moved into JuptyerLab core. Actually I have an important question: would the dynamic details (e.g. documentation) fetching implementation that I use, this is having each item as an object with getters (e.g. get documentation(): str) that gets updated once the promise with a request resolves (which gets triggered when the items are close to being shown to the user) work for you as well?

[krassowski]

Closing in favour of jupyterlab/jupyterlab#11795 and jupyterlab/jupyterlab#11969 - big changes coming with JupyterLab 4.0 :)