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

Codemirror: Switch legacy yaml to yaml #20837

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

silamon
Copy link
Contributor

@silamon silamon commented May 20, 2024

Proposed change

Switch the legacy yaml (codemirror 5 ported language) to yaml (codemirror 6).
Note: the highlighting seems to be different, as in the language tokens don't match, not sure if that's important.

Edit: I've setup a demo page to test for anyone that wants to: https://deploy-preview-20844--home-assistant-gallery.netlify.app/#misc/codemirror

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@steverep
Copy link
Member

Note: the highlighting seems to be different, as in the language tokens don't match, not sure if that's important.

Does the theme need to be tweaked?

@silamon
Copy link
Contributor Author

silamon commented May 21, 2024

Note: the highlighting seems to be different, as in the language tokens don't match, not sure if that's important.

Does the theme need to be tweaked?

I've setup a demo page to test for anyone that wants to: https://deploy-preview-20844--home-assistant-gallery.netlify.app/#misc/codemirror
Unfortunately, I don't use yaml too much to see most of the changes.

One of the most visible changes in the given example below is the key being colored differently (yellow vs brownish red):

# Basic syntax - key and value separated by colon and space before the value
key: value

@bramkragten
Copy link
Member

bramkragten commented May 29, 2024

It doesnt make a difference anymore based on the type of the value (string, number, boolean)

The other differences in color could be fixed in the theme.

CleanShot 2024-05-29 at 12 31 19@2x

@silamon
Copy link
Contributor Author

silamon commented Jun 3, 2024

It doesn't seem like the tokenizer of the new incremental grammar supports that. The token should be defined as a boolean or a number, and we define a style for that, but it isn't tagged as a boolean or a number token at first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Escaping of ' is missing in the Developer tools, leading to wrong syntax highlighting
3 participants