search

json-schema-org / JSON-Schema-Test-Suite

A language agnostic test suite for the JSON Schema specifications
MIT License
603 stars 205 forks source link

Use spaces instead of tabs #741

Closed harrel56 closed 2 months ago

harrel56 commented 2 months ago

Funny stuff: my YAML 1.1 parser panics on these tabs, wondering if it's mandated by spec or just this lib specific behavior.

Julian commented 2 months ago

Regardless we use spaces for indentation in the repo (and ideally should have CI ensuring that at some point) so this looks good to merge to me. I suspect (but won't bother checking) that even though tabs are almost certainly allowed in YAML, mixing them as it seems has happened here is a recipe for really poor behavior. Thanks.

gregsdennis commented 2 months ago

I don't get this. All these files are pure JSON, which disregards whitespace. Given that YAML is a superset of JSON, I'd expect any YAML parser to handle tabs.

harrel56 commented 2 months ago

Given that YAML is a superset of JSON

That's what I read and believed to be true. Tried to find answers in YAML spec but I've given up. Probably just the parser I use (https://bitbucket.org/snakeyaml/snakeyaml) is faulty but cannot say this with 100% certainty.

gregsdennis commented 2 months ago

You might consider opening a bug with them. They can investigate, and if something is wrong, hopefully they fix it.

karenetheridge commented 2 months ago

my YAML 1.1 parser panics on these tabs

These files are JSON, not YAML. You should use a JSON parser to read these files.

Contrary to what some claim, JSON is not a subset of YAML.

https://john-millikin.com/json-is-not-a-yaml-subset
https://news.ycombinator.com/item?id=31406473

edit: I'm told that JSON is a subset of YAML 1.2 (but not 1.1). I would suggest using a 1.2 parser, as there were a lot of enhancements and improvements made to that spec.

perlpunk commented 2 months ago

YAML 1.2 is a superset of JSON, at least noone was able to prove otherwise :)

YAML 1.2 parsers that didn't like those tabs aren't implemented correctly.

ingydotnet commented 2 months ago

Just confirming that what @perlpunk says.

@karenetheridge if you read through the comments of the https://news.ycombinator.com/item?id=31406473 post you referenced, you'll see that the YAML Development Team got involved and showed that about 5 cases brought up asserting that specific JSON inputs were not valid YAML 1.2, were shown to be incorrect according to the YAML 1.2 spec.

This is not to say that someone won't find one, but as of this moment no one has found any to my knowledge.

@harrel56 snakeyaml and many others are far from 100% correct as shown by @perlpunk's https://matrix.yaml.info/

Also @karenetheridge's advice to use a JSON parser to parse JSON is a good idea, unless you know that the YAML implementation you are using is in fact correct.