Skip to content

Fix round-trip of block scalars whose content starts with a space - #708

Open
UlikGames wants to merge 1 commit into
eemeli:mainfrom
UlikGames:fix/692-block-scalar-leading-space
Open

Fix round-trip of block scalars whose content starts with a space#708
UlikGames wants to merge 1 commit into
eemeli:mainfrom
UlikGames:fix/692-block-scalar-leading-space

Conversation

@UlikGames

Copy link
Copy Markdown

Fixes #692

At the root ctx.indent is empty, so the body of a block scalar goes out unindented while the header still claims |1:

parse(stringify('  a\n  b'))            // ' a\n b', lost a space
parse(stringify('  indented\nlines'))   // throws

PyYAML reads |1-\n a\n b\n as ' a\n b' too, so the parser is doing the right thing here and the header is the part that's wrong. blockString now indents the block whenever the content starts with a space, so the indicator matches what actually gets written.

That alone isn't enough, which is why foldFlowLines is in this diff as well. consumeMoreIndentedLines allows indent.length of leading whitespace before it decides a line is more-indented, but the first line is the one line that doesn't carry the indent yet, so its leading space gets eaten and the line is folded anyway. Reproduces on main today:

stringify({ k: ' first more-indented line\nnext line\n' }, { lineWidth: 20 })
// 'k: >2\n   first\n  more-indented line\n  next line\n'
// reads back as ' first\nmore-indented line next line\n'

Fix one without the other and stringify stops emitting unparseable YAML but starts emitting parseable YAML with the wrong value in it.

Fuzzed 20k random strings built from spaces, tabs, newlines and a few indicator characters through parse(stringify(x)) === x. 1971 failures before, 102 after. All 102 leftovers are values made entirely of whitespace, and those look like a parser problem rather than a stringify one: parse('|2+\n \n') gives '\n' here but ' \n' in PyYAML.

The other thing I noticed and left alone is that the indicator is still hardcoded to '2'. That's wrong for map values whenever indent isn't 2, since a map value gets indent + indentStep while YAMLSeq uses indent + ' ' no matter what indentStep is. Getting it right needs the parent indent available in StringifyContext, which is more surgery than this fix wants. Happy to open issues for that or the parser one, or to have a go at either.

One test expectation changed. "More-indented first line (#55)" asserted >1\n first more-indented line\nnext line\n, which neither we nor PyYAML will parse, so it was pinning invalid output in place. Updated it and added the round-trip check.

Full suite green, submodules included.

@eemeli

eemeli commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Did you read and follow the contributing guide before filling this, in particular this part?

@UlikGames

Copy link
Copy Markdown
Author

Yup, I read the guide. If you want I can explain any part of the change.

P.S. I’m at work rn, so I may only be able to reply properly later tonight.

@UlikGames

Copy link
Copy Markdown
Author

I was able to recreate the issue on my own system. I followed the code through blockString and foldFlowLines. While working on the main bug, I also found the two other problems mentioned in the PR description. I didn’t include fixes for those here because they appear to be independent issues.

@eemeli

eemeli commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Before I actually look at the code, this is the part that I'm pretty dubious about:

Prose contributions and comments must be your own writing, not the product of large language models (LLMs) or other tools. This includes code comments added in PRs, as well as all issue and PR contents.

If you would like to use an LLM to contribute code to yaml, please request and discuss this in an appropriate issue first. LLM use must be clearly indicated and pre-approved. If there is indication of undeclared LLM assistance, the issue or pull request will be declined.

Among other tells, I have literally never before LLMs seen a PR mention something like

Full suite green, submodules included.

So was that you sounding like an LLM, or LLM text that you copied?

If you're willing to participate in this as a human, I'll be happy to continue; just please be honest.

@UlikGames

Copy link
Copy Markdown
Author

When I wrote that "Full suite green, submodules included" I meant that I setup the submodules and ran the tests (all of which passed). Summing up the result of a test like this is a common, especially when they’re short.

My bad for being unclear, if you have any questions about the code I'm happy to answer them.

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.

stringify() of multiline string with leading space on first line does not round-trip through parse()

2 participants