-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
Consider adopting a new yaml grammar: Built in YAML grammar left-strips the first character of scalars that are not surrounded by quotes #180523
Comments
Thanks for creating this issue! It looks like you may be using an old version of VS Code, the latest stable release is 1.77.3. Please try upgrading to the latest version and checking whether this issue remains. Happy Coding! |
@VSCodeTriageBot Issue also happens in 1.77.3. |
Interesting. @EarthmanT is this causing a problem for you? |
@alexr00 I suppose there's a chance that it's not related to any issues that I'm experiencing. I suspect it is causing word completion in my language server to match based starting on the 2nd character of most tokens. |
We currently get our yaml grammar from https://github.com/textmate/yaml.tmbundle, but it looks to be unmaintained. Repurposing this issue to track adopting a new yaml grammar. |
Tried out the grammar at https://github.com/redhat-developer/vscode-yaml/tree/main/syntaxes, but it has the same problem. |
Following the advice written in the yaml.tmLanguage.json grammar, I had submitted a fix for this in the textmate/yaml.tmbundle repo: textmate/yaml.tmbundle#45 |
Hello @alexr00 what would be some things I need to look out for/need to do? I also see that Tree-sitter is coming along https://yaml.org/spec/1.2.2/ seems comprehensive |
Hi @RedCMD, we have no timeline for retiring TextMate support, so I expect a new TextMate grammar for yaml would be used for as long as you maintain it and it is better/better maintained than the tree-sitter grammar. This is just a guess, but 3+ years seems like a safe guess. Each time we switch a built in language to a tree-sitter grammar there will likely be noticeable syntax highlighting differences, so we will likely stick with an existing, well-maintained, TextMate grammar over a tree-sitter grammar for languages where such a TextMate grammar exists. As you noticed in #207416 (comment), I have concerns about how well the tree-sitter grammar is maintained, so I would definitely be interested in trying out a yaml TextMate grammar that you fork. For adopting tree-sitter grammars, I will start first with the ones that we'll get the biggest gain from, which from #207416 is HTML, XML, and Groovy, before moving on to those that are less certain (yaml and ini). For the license, if you use MIT then VS Code will be good to use it. As for things to look out for, we do have integration tests for grammars in this repo, but you do need to follow all the steps to build the project then run |
cool cool |
Awesome. Yes, please just message when you think it's ready! |
I may have gone a little overboard there's still a few little bugs I haven't set up a automated test yet do you know some extensions that have automated testing? I've just ran the
(the other YAML files do pass) |
@RedCMD is this the right grammar to use? https://github.com/RedCMD/YAML-Syntax-Highlighter/blob/main/syntaxes/yaml.tmLanguage.json |
@alexr00 that's the main file yes I'm also just waiting on the outcome of the indentation Pre-existing grammar issues has a list of the existing bugs I've fixed/made sure not to reopen |
I took a look, and I agree that based on the VS Code tests your grammar looks good! There's one place where I see an obvious difference. In your grammar you have the scope |
RE the |
that's my mistake
|
For automated testing, it looks like there are some automated tests for shellscript here: https://github.com/jeff-hykin/better-shell-syntax/blob/6d0bc37a6b8023a5fddf75bd2b4eb1e1f962e4c2/commands/project/test#L3-L7 |
@alexr00 should I move the 1.2.tmLanguage.json grammar into the yaml.tmLanguage.json file? it would probably help keep compatibility with old bots copying the single file directly from VSCode |
just fixed the |
Doesn't matter, I've already updated the script here: #219833 |
My usual approach is to keep new grammars in VS Code insiders for a couple months before releasing them to stable. This means I'll just revert back to the old grammar before release for the July release. We'll see how many issues we get before the August release and assess if it's time to fully commit to the new grammar in time for the August release. |
* Try out new YAML grammar Part of #180523 * Pull in other/plain update
Grammar switchover is in! We plan to release stable today, which means the new grammar should go out in insiders tomorrow. |
* Try out new YAML grammar Part of microsoft#180523 * Pull in other/plain update
Currently folding block scalars show up as red >1
folding block-scalar
shows up as red because VSCode hasn't copied the vscode/extensions/yaml/package.json Lines 75 to 79 in d4c164e
"unbalancedBracketScopes": [
"invalid.illegal",
"storage.type.tag.shorthand.yaml",
"keyword.control.flow"
] and for some reason the indentation is whack? vscode/extensions/yaml/package.json Lines 53 to 80 in d4c164e
|
Let's release this in the upcoming release. |
Marking as verified because the colorization tests pass. |
Does this issue occur when all extensions are disabled?: Yes
Steps to Reproduce:
*.yaml
.Expected behavior:
The entire word should show up in the scope inspector window.
Actual behavior:
However, the word is missing the first character. For example "baz", will show up as "az". Selecting to the left of "baz', we see only "b".
Changing the file type fixes the behavior. Use any other file type, "txt" or "json", etc. Even including inline JSON in the YAML file, and surrounding the words with quotes fixes the behavior. This appears to be an issue with the json->yaml tree in the grammar.
The text was updated successfully, but these errors were encountered: