Conversation
parseJsonc removed trailing commas with a regex over the whole text after
strings had been copied through verbatim, so a comma inside a string that
was followed by `}` or `]` was deleted too: a regex like `[^,]+` in a
setting became `[^]+`, `[a, ]` became `[a ]`, and `**/*.{js,}` became
`**/*.{js}`. Every reader of settings.json, keybindings.json and theme
files (tode --import, removalMasked, foreignBindings, tode --theme) saw
the altered value.
Drop the comma while walking the text instead: a comma is trailing only
when the next token past whitespace and comments closes the container,
and strings never reach that branch because they are consumed whole.
|
Someone is attempting to deploy a commit to the zenbu-labs Team on Vercel. A member of the Team first needs to authorize it. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
parseJsoncalters string values when it reads a settings, keybindings or theme file: a comma inside a string that happens to sit before}or]is deleted. A regex setting such as"([^,]+)"comes back as"([^]+)", a glob"**/*.{js,}"as"**/*.{js}", a snippet-style"[a, ]"as"[a ]".tode --importthen writes the altered value into the tode profile, and the keybindings / theme readers see it too.Root cause
parseJsonc(src/jsonc.ts) walks the text once, dropping comments and copying string literals through whole, and then removes trailing commas without.replace(/,(\s*[}\]])/g, "$1")over the entire result. That regex cannot tell a structural comma from one inside a string, so,]and,}sequences inside string literals are rewritten beforeJSON.parseruns.Fix
src/jsonc.ts: handle the trailing comma during the walk instead of with a regex afterwards. When the walker meets a,outside a string, it looks past whitespace and comments with the existingskipTriviahelper; if the next token is}or]the comma is dropped, otherwise it is kept. Strings are consumed whole before this branch, so their contents are never touched. Thereplacecall is removed andJSON.parse(out)is used directly.Tests
test/theme.test.js: new test "jsonc reading leaves a comma inside a string alone" feeds an object with"([^,]+)","[a, ]","**/*.{js,}"and an array holding"ctrl+,"and"x, ]", with real trailing commas after the last value and the last array element. Without the fix it fails with every in-string comma removed; with it the values round-trip and the trailing commas are still stripped.npm run typecheckclean.Not changed
setKey/readKey/locatealready tokenise strings correctly and were not touched.readStringstill drops the backslash of an escape when building the value it compares keys against; that only affects key lookup for escaped key names and is out of scope here.▚▚ Shipped by breken — self-healing software. This one's on us. breken.ai