yaml / yaml-test-suite

Comprehensive, language independent Test Suite for YAML
MIT License
181 stars 62 forks source link

N782 should be an error #10

Closed flyx closed 7 years ago

flyx commented 7 years ago

this test should result in an error, since c-forbidden in the spec forbids these markers everywhere in the document, including in flow mode.

Opinions?

perlpunk commented 7 years ago

good catch. So, if this is true for all kinds of content including quoted scalars and flow style, then it makes it possible to just skip documents for whatever reason (for example if one document in a stream is invalid)

ingydotnet commented 7 years ago

I'm of the opinion that it should work in 1.3.

flyx commented 7 years ago

I think the question we should try to answer is what a human would expect. If one knows that --- and ... are document delimiters, would one rather expect this to provoke an error or would one think „hey, we're still inside flow content, this cannot end the document“? I am not sure there is any benefit in allowing these delimiters as scalars in flow content, for two reasons:

  1. If someone writes YAML and forgets to close some flow collection, but just writes ..., the error message they will be given will point to somewhere in the following document, because ... is parsed as a flow scalar.
  2. If, on the other hand, someone wants to include --- or ... as flow scalar in a flow collection, they will probably think about „hey, this has a special meaning in YAML, I should probably quote it“.

So, I mainly argue that this should be an error because if it occurs in an actual YAML document, it is likely that it actually is an error of the writer. In the case that we forbid it and someone tries to use it nonetheless, they will be pointed exactly at the error's location and it's an easy fix. In the case that we allow it and someone uses it when trying to actually end the document, they may have a hard time locating the error. So I think from a usability perspective, it would be better to forbid it.

ingydotnet commented 7 years ago

I am currently aligned with @flyx thinking. Let's make this test case be an error.

ingydotnet commented 7 years ago

Also libyaml doesn't allow this.

perlpunk commented 7 years ago

I merged #11