Maybe refactor features

[krassowski]

Feature = LSP functionality, e.g. completion, jumper, diagnostics.

Make it possible to split up the features and install the extra features.

What are you trying to do?

Expose a token LSPFeatureManager which will be used to register the features. It would have a register method:

class LSPFeatureManager implements TokenPattern {
    register(LSPFeatureManager.IFeatureOptions) {
        // add the feature to the list of features
    }
}

interface LSPFeatureManager.IFeatureOptions {
    feature: IFeature;
    /** ids of the features this feature relies on  */
    requires?: string[];
    /** ids of the features this feature wants to disable; 
    use it to override the default feature implementations with your custom implementation
    (e.g. a custom completer from Kite)  */
    supersedes?: string[];
}

The new feature interface:

import { CodeEditor } from '@jupyterlab/codeeditor';
import { CodeMirrorEditor } from '@jupyterlab/codemirror';

export interface IFeature {
    id: string;
    // used in the GUI
    name: string;
    commands: Commands;
    // each feature can be written in mind with mind to support one or more editors;
    // by default we target CodeMirrorEditor, but let's think about allowing others too
    // supportedEditors: typeof CodeEditor[] = [CodeMirrorEditor];
    editorIntergration: Record<typeof CodeEditor, typeof FeatureEditorIntegration<CodeEditor>>;   
    labIntegration?: IFeatureLabIntegration;
    // note: settings could be one-per plugin rather than centrally managed by LSP (as proposed here);
    // the per-plugin settings may be nicer for the user, and would keep the schema near the plugin
    // but then combining per-language settings etc would be a pain
    register(settings: Settings): void;
}

export class CodeMirrorIntegration implements FeatureEditorIntegration<CodeMirrorEditor> {
   // some staff useful for CM integraion
}

each feature would then be an extension:

const FEATURE_ID = '@jupyterlab-lsp/feature-completion:plugin'

import {BaseFeature, CodeMirrorIntegration} from 'jupyterlab-lsp'


class CompletionCMIntegration extents CodeMirrorIntegration {
     // register cm-specific actions
}

class CompletionLabIntegraion implements IFeatureLabIntegration {
    register() {
        // register custom data connector for completion
    }
}

class CompletionFeature extends BaseFeature implements IFeature {
   // does not have to be the same, but why not
   id = FEATURE_ID,
   name = 'Completion'

   editorIntergration = {CodeMirrorEditor: CompletionCMIntegration}
   labIntegration: CompletionLabIntegraion
}

const plugin: JupyterFrontEndPlugin<void> = {
  id: FEATURE_ID,
  requires: ILSPFeatureManager,
  activate: (app: JupyterFrontEnd, featureManager: ILSPFeatureManager) {
    featureManager.register(
        {
             // may end up being a simple object instead
             feature: CompletionFeature
        }
    )
  }

To consider:

• commands and shortcut registration could now be delegated to the features more easily because they become standalone plugins; the question is how to disable the commands if a user wants us to disable a feature temporarily.

How is it done today, and what are the limits of current practice?

• CodeMirror features have nice, well defined class hierarchy but cannot be dynamically added.
• Lab integration of completer is defined as a lab component which is odd and makes completer live in two separate places
  • the settings for completer have to be passed to both places - suboptimal

What is new in your approach and why do you think it will be successful?

• It will enable Kite to wrap their extension into the feature and just swap our completer with theirs
• It will enable other thridparties to contribute features without requirement of navigating our complex codebase
• It will clean up our codebase

[bollwyvl]

Some thoughts here:

On the implementation front:

Anything that starts splitting up the (multiple) god objects, and makes it easier to externally build and test new/alternate features is good.

However, so long as all the features it can support are hard-coded, this will only take us so far, as it will not be simple to dynamically extend with one of the 100 messages we don't support yet. So being able to also claim access to some specific messages/responses may be important. I've started down this road on the kernel PR, as having to update message names in 3/4 places is not going to scale, long term, and I think it's folly to try to "protect" features developers from LSP.

On the architecture front:

Absolutely, each feature should be a plugin, and should document/expose itself with an interface which can be replaced, so that other interfaces, and features, can rely on features.

Where possible, we should go further, such as with diagnostics, and split out the current ui panel to just have to listen to what the Feature is putting out.

So...

@k/jlsp -> @k/jlsp-ft-diagnostics -> @k/jslp-ui-diagnostic-panel

On the packaging front:

It is not strictly required to make each plugin a separate package... a single npm package can expose a list of separately-ided plugins, which can then be disabled indepenently by id (or even regexen of ids!).

Some deployment scenarios:

1. jupyter labextension install @krassowski/jupyterlab-lsp

• install: 1 package,
  • nothing changes to the user
• disable: jupyter labextension disable @krassowski/jupyterlab-lsp:plugin-completion
• uninstall: nothing changes to the user

2. jupyter labextension install @krassowski/jupyterlab-lsp-core @krassowski/jupyterlab-lsp-completion ... n

• install: 1 + n packages
  • a lot more complicated, more user-visible packages mean more versions to wrangle
• disable: as above
• uninstall: again, rather verbose

3. jupyter labextension install @krassowski/jupyterlab-lsp

• install: 2 + n packages
  • kind of like the first option, but under the hood does what the second option does
• disable: as above, but the underlying package name might well be in the id, e.g. @krassowski/jupyterlab-lsp-ft-completion:plugin
• uninstall: again, kind of like the first option

1 is mutually exclusive with 3, probably.

[krassowski]

Thanks for sharing thoughts, all makes sense!

I think it's folly to try to "protect" features developers from LSP

Right!

Absolutely, each feature should be a plugin

Done!

and should document/expose itself with an interface which can be replaced so that other interfaces, and features, can rely on features.

Kind of done, but not really the way you would expect I guess. For now, dependencies are supported via accessing the sibling-features from a map fed to the feature by featureManager (rather than by tokens). This is required by the current feature architecture, and while I would like to have them as tokens it is out of scope for this issue.

Where possible, we should go further, such as with diagnostics, and split out the current ui panel to just have to listen to what the Feature is putting out.

So say we have token ILSPDiagnostics, and diagnostics panel is another plugin which requires ILSPDiagnostics and they communicate via public interface? I like the idea (though not this time unless I get a grip on how to coerce features into tokens without breaking everything - small steps!)

It is not strictly required to make each plugin a separate package... a single npm package can expose a list of separately-ided plugins, which can then be disabled independently by id (or even regexen of ids!).

Yes, I'm for (1) for the default plugins. A thing to consider is how do we disable features from the GUI (say the user does not want the diagnostics). Disabling from the command line (i.e. the actual plugin/npm package) is not strictly needed for the immediate use-case of kite integration which can just request to disable our completer in favours of theirs (via supersedes).

[krassowski]

Closing as the refactor is done and we are tracking the architecture/source organization elsewhere.