Skip to content

Use spaces instead of tabs #741

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

Merged
merged 1 commit into from
Apr 21, 2024

Conversation

harrel56
Copy link
Contributor

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

@harrel56 harrel56 requested a review from a team as a code owner April 21, 2024 07:45
@Julian
Copy link
Member

Julian commented Apr 21, 2024

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.

@Julian Julian merged commit ff29264 into json-schema-org:main Apr 21, 2024
1 check passed
@harrel56 harrel56 deleted the chore/tabs-to-spaces branch April 21, 2024 08:18
@gregsdennis
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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

@karenetheridge
Copy link
Member

karenetheridge commented Apr 21, 2024

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
Copy link

perlpunk commented Apr 22, 2024

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
Copy link

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.

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

Successfully merging this pull request may close these issues.

6 participants