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

Remove incorrect block indentation indicator from 7T8X out-yaml #27

Merged
merged 1 commit into from Jan 26, 2020

Conversation

eemeli
Copy link
Member

@eemeli eemeli commented Feb 18, 2018

The 2 indentation indicator is unnecessary, and therefore against the guideline given at least in the 1.2 version of the spec:

It is always valid to specify an indentation indicator for a block scalar node, though a YAML processor should only emit an explicit indentation indicator for cases where detection will fail.

Furthermore, the 2 in this case doesn't even match the actual indentation of the contents. As "Document nodes are indented as if they have a parent indented at -1 spaces", and the example uses two spaces for indentation, the indicator here would need to be 3 for the in.yaml and out.yaml contents to match.

eemeli added a commit to eemeli/yaml-test-suite that referenced this pull request Feb 18, 2018
eemeli added a commit to eemeli/yaml that referenced this pull request Feb 18, 2018
@perlpunk
Copy link
Member

I agree that the 2 should be removed. The out.yaml was generated by libyaml, so it seems libyaml is doing the "wrong" thing here.
It seems to happen only if the first line is empty.
@ingydotnet @flyx any idea why libyaml is doing this?

I also agree that logically, it should be a 3, although that doesn't make much sense from a user perspective.

eemeli added a commit to eemeli/yaml-test-suite that referenced this pull request Jul 1, 2018
@perlpunk perlpunk added this to Backlog in Release 2020-02-11 Dec 27, 2019
@perlpunk perlpunk changed the base branch from master to release January 26, 2020 21:19
@perlpunk perlpunk merged commit 1933ac9 into yaml:release Jan 26, 2020
@perlpunk
Copy link
Member

Thanks! merged to release branch

@perlpunk perlpunk moved this from Backlog to Merged to release branch in Release 2020-02-11 Jan 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Release 2020-02-11
  
Merged to release branch
Development

Successfully merging this pull request may close these issues.

None yet

2 participants