fix: preserve LiteralNode content indentation in Replace operations#864
Open
babarot wants to merge 1 commit intogoccy:masterfrom
Open
fix: preserve LiteralNode content indentation in Replace operations#864babarot wants to merge 1 commit intogoccy:masterfrom
babarot wants to merge 1 commit intogoccy:masterfrom
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #864 +/- ##
==========================================
+ Coverage 80.65% 80.67% +0.01%
==========================================
Files 22 22
Lines 6845 6845
==========================================
+ Hits 5521 5522 +1
+ Misses 905 903 -2
- Partials 419 420 +1 🚀 New features to boost your workflow:
|
MappingValueNode.Replace and SequenceNode.Replace corrupted literal block scalar content because AddColumn adjusts Position.Column but LiteralNode.String() uses the raw Origin text, which was never updated to reflect the new position. Recalculate column deltas for structured node types (MappingNode, SequenceNode, multiline StringNode, LiteralNode) based on the key/parent column position. For LiteralNode, rebuild Origin from the actual Value string with correct indentation to avoid cumulative corruption when the same node is replaced into multiple locations. Fixes goccy#636
babarot
force-pushed
the
fix/literal-node-replace
branch
from
6e7454e to
737662c
Compare
babarot
added a commit
to babarot/gh-infra
that referenced
this pull request
Apr 8, 2026
Link to goccy/go-yaml#864 and #636 in SetLiteral, replaceLiteralContent, and replaceWithLiteralBlock so the workaround can be removed once the upstream fix is merged.
goccy
added a commit
that referenced
this pull request
Apr 11, 2026
MappingValueNode.Replace and SequenceNode.Replace had two related defects
that produced wrong indentation in the resulting document:
1. Literal block scalars (`|`) ignored their content getting moved to a
new column, because LiteralNode.String() and the multiline path of
StringNode.String() bypass Position.Column. Replacing such a value via
parser.ParseBytes (yields *LiteralNode) or yaml.ValueToNode with
UseLiteralStyleIfMultiline (yields *StringNode) left the new content
indented at the source's column rather than the destination's.
2. Replacing a block-style mapping or sequence used the value's
GetToken().Position.Column as the source/destination anchor, but
*MappingNode.GetToken() returns the parser-internal Start token (often
the first colon) whose column varies between parser-built and
ValueToNode-built nodes. The result was an off-by-one indent for any
replacement that mixed those two construction paths.
The fix introduces helpers in ast/ast.go:
- detectLiteralContentIndent / applyLiteralContentIndent rebuild a
*LiteralNode's inner Origin from its Value, or pin a multiline
*StringNode's Position.Column / IndentNum so its String() output
lands at the destination indent. The rebuild path is idempotent
across repeated replacements (e.g. `[*]`), so applying the same
parsed node into multiple slots no longer accumulates corruption.
- replaceColumns / anchorColumn / isBlockStructure compute the column
delta from each node's first content position rather than from its
Start token, so block-style replacements work consistently regardless
of how the value was constructed. When a scalar slot is being
replaced by a block structure (no source-side indent to inherit),
the caller-supplied "parent_key + 2" convention default is used.
Adds TestPath_ReplaceWithNode_PreserveIndent in path_test.go covering
both construction paths, 2-space and 4-space and 3-space source indents,
multi-slot `[*]` replacements, YAML-special characters at line starts,
and sequence-element literal replacement.
Fixes #636. Supersedes #864.
4 tasks
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
2 participants
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.
Purpose
Fix
MappingValueNode.ReplaceandSequenceNode.Replacecorrupting literal block scalar (|) content when replacing nodes. Characters were eaten from the beginning of each content line becauseAddColumnadjustsPosition.ColumnbutLiteralNode.String()uses the rawOrigintext, which was never updated to reflect the new position.This was originally reported in #636.
Root cause
MappingValueNode.Replacecomputes a column delta and callsvalue.AddColumn(delta). For most node types,String()usesPosition.Columnto reconstruct output, so this works correctly. However,LiteralNode.String()ignoresPosition.Columnentirely and returns the rawOrigintext from the scanner. TheOriginretains the indentation from the source document (the replacement value), not the destination, causing the mismatch.Fix
MappingValueNode.ReplaceandSequenceNode.Replace, recalculate the column delta forMappingNode,SequenceNode, multilineStringNode, andLiteralNodebased on the key/parent column position rather than the value token position.LiteralNode, rebuild theOrigintext from the actualValuestring with the correct target indentation. This avoids cumulative corruption when the same node is replaced into multiple locations (e.g. via[*]or..paths).Tests added
TestPath_ReplaceWithNode_ValueToNodeIndent: MappingNode replacement viaValueToNode(Wrong indentation when path.ReplaceWithNode with an ast.Node created by the function yaml.ValueToNode #636 scenario)TestPath_ReplaceWithNode_LiteralNodeIndent: LiteralNode replacement viaValueToNodeTestPath_ReplaceWithParsedLiteralNodeIndent: LiteralNode replacement viaparser.ParseBytesTestPath_ReplaceWithParsedLiteralNodeIndent_MultipleMatches:[*]path replacing same node into multiple locationsTestPath_ReplaceWithParsedLiteralNodeIndent_SpecialCharacters: content with YAML-special characters (*,/,:) at line beginningsTestPath_ReplaceSequenceElementWithParsedLiteralNodeIndent:SequenceNode.Replacewith literal block scalar