-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Prepare Emitter API to go public #3621
Comments
Considering pulling sublanguage fully private as well and we'd allow for We do need some way to alternative emitters to do this if they want to be fully compatible with some themes that depend on this naming system in the HTML. |
I mainly need As you’re sort-of talking about classes with sublanguages, you could also turn the dot notation into actual classes. I’m doing that manually now: https://github.com/wooorm/lowlight/blob/ebddb1040b8b4adc78abb502e59ddd378a56bd8b/lib/core.js#L267-L272. Last, instead of a function-based API, you could also build an AST. Basically what I am using the emitter for. Others can transform that AST into whatever they want. |
Well, built on top of, yes... I'd move the functions back into the core library and out of the emitter public API.
Sort of, but everything is a
I prefer an immutable API with a changeable data structure than the opposite... I think that's easier and more flexible to maintain and as you've seen it's just as easy to work with in practice. In the future we may go back to just joining strings rather than a tree - so I'm still not interested in solidifying that piece.
I'll consider this. |
Any interest in this? Instead of the middle man step of converting your BNF grammars to Highlight.js grammars if you have an existing BNF parser you could just use that to do the parsing and then emit the results directly into our output pipeline. I wonder if that would help simplify things on your end? And then people would have another easy way to define HLJS grammars - using BNF. (well I guess technically your hybrid, but surely it'd be possible to compile BNF into BNF/JSON?) I wonder how BNF parsers tend to do with 'getting back on track' when presented with wholly invalid data? That's often a thing with code snippets - incomplete context, typos, etc... |
@joshgoebel as long as there are ways to bypass the default tokenizer and there is a way to provide the actual token (parsed with an external parser), my addon is fine. I don't have the time at this point to test it, but I will do it once it is live.
This is part of error recovery. The approach can be simple or complex. A simple approach is discard everything (eg highlight as default text) until a valid sequence is found, then start again. |
That's what #3620 allows. It should land in 11.7 (as a beta feature) with a full release with v12. |
I think this landed now right? I updated and I’m getting errors. Here’s how I migrated, how it went!
All together: wooorm/lowlight@0ab194d |
If I make a branch/PR can you test it's types? I honestly didn't look at types the entire time I did this work... so I can go back and try to clean that up now. |
Closing this out as complete by the release of 11.8. |
Apparently highlight.js Emitter api has changed - the addKeyword function has been removed and the startScope function and the endScope function have been added. Oddly enough, they removed the openNode and closeNode functions from .d.ts, but they are still called. More info: highlightjs/highlight.js#3621
In preparation for #3619 and #3620 I'd live to review out Emitter API and rename some things to better match our own internal naming. I think the original was a bit too specific with regards to internal implementations and the new version should be a little more abstract:
Old:
addKeyword(text, scope)
addText(text)
addSublanguage(emitter, subLanguageName)
finalize()
openNode(scope)
closeNode()
closeAllNodes()
toHTML()
New:
addText(text)
startScope(scope)
(or pushScope?)endScope()
(or popScope? or finishScope?)__addSublanguage(emitter, subLanguageName)
- (may stay private)finalize()
- thoughts on naming?toHTML()
Removed:
- pull back into an internal helperaddKeyword(text, scope)
- Lets just let finalize be responsible for all cleanup.closeAllNodes()
For writing custom grammar parsers one would really only need to concern themselves with 3 API calls I think:
@wooorm I know you had plenty of thoughts on this before... it's possible I'm a little more receptive now... I do plan on reducing a API a bit if we're going public (which I recall you pushed for previously).
The text was updated successfully, but these errors were encountered: