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

Added YAML support #85

Merged
merged 6 commits into from
Feb 23, 2024
Merged

Added YAML support #85

merged 6 commits into from
Feb 23, 2024

Conversation

imolorhe
Copy link
Collaborator

Closes #84

  • Added YAML support
  • Also updated the DOM rendering bits to use markdown instead of setting the HTML directly, which is a safer approach
  • Also slightly improved the demo UI (it didn't look good with black text on black background)
  • Also added a bunch of tests

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).

Also updated the DOM rendering bits to use markdown instead of setting
the HTML directly, which is a safer approach
@imolorhe imolorhe requested a review from acao February 22, 2024 17:38
Copy link

changeset-bot bot commented Feb 22, 2024

🦋 Changeset detected

Latest commit: f5b1ebf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
codemirror-json-schema Minor

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

Copy link

netlify bot commented Feb 22, 2024

Deploy Preview for codemirror-json-schema ready!

Name Link
🔨 Latest commit f5b1ebf
🔍 Latest deploy log https://app.netlify.com/sites/codemirror-json-schema/deploys/65d7fb736df40d0008720113
😎 Deploy Preview https://deploy-preview-85--codemirror-json-schema.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@imolorhe imolorhe mentioned this pull request Feb 22, 2024
@acao
Copy link
Collaborator

acao commented Feb 23, 2024

oh this is BEAUTIFUL

mode: MODES.YAML,
},
])(
"should return full pointer path for a position for $name, mode: $mode",
Copy link
Collaborator

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);
Copy link
Collaborator

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

Copy link
Collaborator

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean linkify?

dev/index.ts Show resolved Hide resolved
Copy link
Collaborator

@acao acao left a 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!

@imolorhe imolorhe merged commit c694451 into main Feb 23, 2024
3 checks passed
@imolorhe imolorhe deleted the imolorhe/yaml-support branch February 23, 2024 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

YAML support
2 participants