Repository navigation
fix(stringify): use indentStep length for mapping block scalar indent indicator - #738
princejain756 wants to merge 1 commit into
Conversation
… indicator When a block scalar starts with a space and is serialized inside a mapping or set, its content is indented by `ctx.indentStep` relative to its parent node. Previously `blockString()` hardcoded `'2'` whenever `indent` was non-empty, causing `stringify()` with `indent` other than 2 to emit an indentation indicator that disagreed with the content indentation. Reset `implicitKey` to `undefined` for block sequence items (which always indent by 2 spaces relative to `-`) and use `ctx.indentStep.length` when `ctx.implicitKey === false`. Fixes eemeli#733
eemeli
left a comment
eemeli
left a comment
There was a problem hiding this comment.
Please rewrite the description in your own words.
@char0n's comment on the parent issue, "You should be fine, AI or no AI.", does not follow the contributing guide instructions:
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.
He is also not a maintainer of the project. I probably need to refactor the guide once again to really underline the point that LLM use is acceptable for code contributions only.
| } = ctx | ||
| const itemIndent = indent + ' ' | ||
| const itemCtx = { ...ctx, indent: itemIndent } | ||
| const itemCtx = { ...ctx, implicitKey: undefined, indent: itemIndent } |
There was a problem hiding this comment.
Why is this required? And is this covered by the tests added here?
There was a problem hiding this comment.
When a block sequence is the value of a mapping pair (like seq: [' lead\nx\n']), stringifyPair() sets ctx.implicitKey = false before stringifying the value. Because #stringifyBlockSeq() spreads ctx into itemCtx and always indents sequence items by 2 spaces (indent + ' '), leaving implicitKey: false on itemCtx causes blockString() to emit - |4 instead of - |2 when indent: 4, which strips the two leading spaces on parse.
Yes, the nested case in the test (const nested = { a: { k: ' lead\nx\n' }, seq: [' lead\nx\n', { m: ' lead\nx\n' }] } with { indent: 4 }) covers this: without resetting implicitKey here, YAML.parse(strNested).seq[0] comes back as 'lead\nx\n' instead of ' lead\nx\n'.
|
Sorry for the confusion around @char0n's comment earlier. I rewrote the PR description in my own words and replied inline on the |
|
@eemeli is right, sorry for interfering with the repo processes. |
Issue
fixes #733
LLM Use
I used an LLM tool when working on the code change, and wrote this PR description and comments myself.
Description
In
blockString(), the indentation indicator for a block scalar starting with spaces was hardcoded to'2'wheneverindentwas non-empty. That works for the default 2-space indent and for block sequence items (since#stringifyBlockSeq()always indents item content by' '), but mapping and set values are indented byctx.indentStep. Whenstringify()is called with a customindentlike1,3, or4, the header still emitted|2or>2while the body was indented byctx.indentStep.lengthspaces, so parsing the output back either failed or shifted the leading spaces.This change uses
String(ctx.indentStep.length)inblockString()whenctx.implicitKey === false(mapping and set values), and resetsimplicitKey: undefinedinYAMLSeq.#stringifyBlockSeq()so block sequences nested inside a mapping value still use'2'to match their 2-space item indent.