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

JSON fails to parse with a trailing comment at the end of an array #11545

Open
elibarzilay opened this issue Oct 20, 2021 · 7 comments
Open

JSON fails to parse with a trailing comment at the end of an array #11545

elibarzilay opened this issue Oct 20, 2021 · 7 comments
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Resolution-External For issues that are outside this codebase
Milestone

Comments

@elibarzilay
Copy link

Windows Terminal version (or Windows build number)

1.10.2714.0

Other Software

No response

Steps to reproduce

In the settings file, put a comment at the end of an object with a comma after it.

This fails:

"actions": [
  {},
  // ...
],

Dropping either the , or the comment makes it work.

Expected Behavior

Should be parsed as usual since both terminating commas and comments are allowed, or forbid trailing commas (assuming that comments are a desired feature).

Actual Behavior

Settings could not be loaded from file. Check for syntax errors, including trailing commas.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Oct 20, 2021
@zadjii-msft
Copy link
Member

I suspect this is a jsoncpp bug. I know that they don't support a "trailing" comment, the comment has to be before some real value that it can attach the comment too. That being said, there's nothing open on their repo (or maybe I'm just bad at searching). We should probably drum up a minimal repro without the Terminal involved and post this upstream

@zadjii-msft zadjii-msft added Area-Settings Issues related to settings and customizability, for console or terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Resolution-External For issues that are outside this codebase labels Oct 20, 2021
@zadjii-msft zadjii-msft added this to the Backlog milestone Feb 17, 2022
@zadjii-msft zadjii-msft added Help Wanted We encourage anyone to jump in on these. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Feb 17, 2022
@zadjii-msft zadjii-msft changed the title Fishy parsing of settings json JSON fails to parse with a trailing comment at the end of an array Aug 15, 2022
@zadjii-msft zadjii-msft added the Product-Terminal The new Windows Terminal. label Aug 15, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Aug 15, 2022
@jamespack
Copy link
Contributor

I have a minimal repro example ready to go to file a bug at jsoncpp. But before I did, I did some searching and I cant find where trailing commas are permitted by the spec and most searches on StackOverflow indicate that they are not permitted.

https://stackoverflow.com/questions/201782/can-you-use-a-trailing-comma-in-a-json-object

From MDN:

image

Maybe a better error message could be produced?

@jamespack
Copy link
Contributor

Still happy to open the issue if you think it'd be prudent :)

@lhecker
Copy link
Member

lhecker commented Jul 10, 2023

Many JSON parsers allow trailing commas as an extension to the spec, including jsoncpp with the allowTrailingCommas option. This is helpful, because trailing commas make editing and diffing JSON files easier.

@jamespack
Copy link
Contributor

Right on. I will open an with them today then. Thanks for the clarification!

@jamespack
Copy link
Contributor

Here you go

open-source-parsers/jsoncpp#1500

@sba923
Copy link

sba923 commented Sep 30, 2023

Many JSON parsers allow trailing commas as an extension to the spec, including jsoncpp with the allowTrailingCommas option. This is helpful, because trailing commas make editing and diffing JSON files easier.

VScode doesn't like Windows Terminal's defaults.json:

image

@nguyen-dows does this deserve an issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Resolution-External For issues that are outside this codebase
Projects
None yet
Development

No branches or pull requests

5 participants