Fix round-trip of block scalars whose content starts with a space - #708
Fix round-trip of block scalars whose content starts with a space#708UlikGames wants to merge 1 commit into
Conversation
|
Did you read and follow the contributing guide before filling this, in particular this part? |
|
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. |
|
I was able to recreate the issue on my own system. I followed the code through |
|
Before I actually look at the code, this is the part that I'm pretty dubious about:
Among other tells, I have literally never before LLMs seen a PR mention something like
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. |
|
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. |
Fixes #692
At the root
ctx.indentis empty, so the body of a block scalar goes out unindented while the header still claims|1:PyYAML reads
|1-\n a\n b\nas' a\n b'too, so the parser is doing the right thing here and the header is the part that's wrong.blockStringnow 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
foldFlowLinesis in this diff as well.consumeMoreIndentedLinesallowsindent.lengthof 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: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 wheneverindentisn't 2, since a map value getsindent + indentStepwhileYAMLSequsesindent + ' 'no matter whatindentStepis. Getting it right needs the parent indent available inStringifyContext, 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.