fix: add explicit indent indicator to empty block scalar when followed by a document comment#689
Closed
naveentehrpariya wants to merge 2 commits into
Closed
Conversation
When an untagged scalar like `61e9540` matches the float-EXP regex but parseFloat() returns Infinity, the YAML 1.2 spec §2.4 says the tag may only be applied if the value round-trips faithfully. Returning Infinity for a value whose source text is a finite exponent violates that guarantee and causes surprising results (e.g. git SHAs parsed as Inf). Add a `matchDefault` predicate to `ScalarTag` that is checked only during automatic tag detection (i.e. no explicit tag). The `floatExp` tag in both the core and YAML 1.1 schemas sets `matchDefault` to reject values that parseFloat() cannot represent as a finite number, allowing them to fall through to the string tag instead. Explicit `!!float` tags are unaffected: they still use the `test` regex for format selection and resolve to Infinity when the value overflows, which matches the behaviour of other YAML libraries (e.g. PyYAML). Fixes eemeli#657
An empty block scalar (value '') with a document-level trailing comment was producing a toString() output where the comment line was at indent 0, which caused it to be re-absorbed as block-scalar content on round-trip. Before: '|5\n#comment' → toString → '|\n\n\n#comment\n' → reparse → '\n\n#comment\n' After: '|5\n#comment' → toString → '|1\n\n\n#comment\n' → reparse → '' The fix adds |1 (explicit indent indicator) when a non-empty indent context is present (forceBlockIndent or containsDocumentMarker), matching the behaviour of non-empty block scalars in the same context. Fixes eemeli#687
Owner
|
Closing due to undeclared LLM use. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
An empty block scalar whose document has a trailing comment produces a
toString()output that does not round-trip correctly.Reproducer (issue #687):
What goes wrong:
stringifyDocumentsetsctx.forceBlockIndent = truewhen a document comment exists (so that the comment can be placed after the block scalar). For non-empty block scalars,forceBlockIndentcauses a two-spaceindentin the header, which correctly pushes the comment outside the scalar's content. But the early-return path for empty strings (if (!value) return literal ? '|\n' : '>\n') ignoresindent, emitting a bare|with no explicit indent indicator.After the document comment is appended, the output looks like:
On re-parse,
#commentis block-scalar content, not a YAML comment — corrupting the round-trip.Fix
When
indentis non-empty (i.e.forceBlockIndentor a document-marker-safe context requires it), emit|1/>1instead of|/>for an empty block scalar. The explicit indent indicator1means content must start at indent ≥ 1; the comment at indent 0 therefore falls outside the scalar and is correctly interpreted as a YAML comment on re-parse.Tests
toString()incorrectly converts comment to string #687