-
Notifications
You must be signed in to change notification settings - Fork 13
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
Added YAML support #85
Conversation
Also updated the DOM rendering bits to use markdown instead of setting the HTML directly, which is a safer approach
🦋 Changeset detectedLatest commit: f5b1ebf The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for codemirror-json-schema ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
oh this is BEAUTIFUL |
mode: MODES.YAML, | ||
}, | ||
])( | ||
"should return full pointer path for a position for $name, mode: $mode", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
brilliant use of it.each() here!
@@ -0,0 +1,4 @@ | |||
import markdownit from "markdown-it"; | |||
export function renderMarkdown(markdown: string) { | |||
return markdownit().renderInline(markdown); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe let's add autolinker here to be safe? otherwise the new markdown-it default seems to pretty GFM compatible, which is what I assume users will expect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok how about this - it would complexity things just a wee bit, considering all the top level interfaces, but what if we allowed the user to pass plugins via a config?
I can do this as a follow up if you prefer, not a huge priority
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean linkify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say this is good to go! Awesome work. It just keeps getting better!
Closes #84
To support YAML (and potentially other formats), I updated the logic to "resolve" tokens before using them. This is important since the YAML grammar/tokens is different from the JSON grammar/tokens (which is very similar to the JSON5 grammar). So we map the tokens from the different grammars to the equivalent token in the "base" grammar (currently this is the JSON grammar tokens but I think we can/should map to some other arbitrary token set which will force us to properly implement the mappings, otherwise additional changes made may work in JSON mode but without proper testing, may not work in YAML mode, if the mappings were not added).