Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow registering additional CodeMirror mode loaders #5829

Merged
merged 1 commit into from Jan 26, 2019

Conversation

@bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Jan 3, 2019

At present, the options for resolving a CodeMirror mode are:

  • no-op, the full mode implementation was already defineModed and/or defineMIMEd and modeInfo.pushd
  • the mode is required from the "big-ol-bundle of modes which will never get any bigger"
  • fails with a a console error and returns "plain text"

This adds a rankable extension point to Mode for alternate CodeMirror mode loaders, as well as some tests for Mode, which revealed some surprises like not being able to use webpack's require in jest.

motivation

Concretely, #5504 (comment) is experimenting with codemirror-textmate which has a heavy-duty (500kb) dependency on onigasm. To achieve the initial demo (stan, stata and sql) without loading onigasm and all the JSON on first load, I've monkeypatched Mode.ensure, but having a first-class way to provide more modes would be much better.

There are other goodies in that repo, including a schema for simplemode, icons, etc. but the marquee feature is definitely the tmlanguage stuff.

stan in the place your live

challenges

There are a number of additional issues like having to patch mimetypes to serve wasm, and just the size of onigasm, but...

payoff

...the upside is there are probably thousands of existing textmate/sublime/vscode modes defined as static, schema-conforming JSON (or equivalent plist or CSON or XML) which could be added to a user's environment without a rebuild and minimal maintenance. These can also be used with monaco (indeed, the implementation already has monaco-textmate as a dependency).

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Jan 3, 2019

Wow, thanks for your work on this!

@jasongrout jasongrout added this to the 1.0 milestone Jan 3, 2019
@bollwyvl
Copy link
Contributor Author

@bollwyvl bollwyvl commented Jan 3, 2019

Copy link
Member

@saulshanabrook saulshanabrook left a comment

This looks really tight and nice!

The abstraction makes sense to me. Thanks for thinking about this use case.

I tried it out locally and it didn't seem to break anything.

@saulshanabrook saulshanabrook merged commit 1377bd1 into jupyterlab:master Jan 26, 2019
3 checks passed
@bollwyvl
Copy link
Contributor Author

@bollwyvl bollwyvl commented Jan 26, 2019

@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants