Fix MergeYaml inserting a new entry after surrounding blank lines#8232
Open
bmuschko wants to merge 1 commit into
Open
Fix MergeYaml inserting a new entry after surrounding blank lines#8232bmuschko wants to merge 1 commit into
MergeYaml inserting a new entry after surrounding blank lines#8232bmuschko wants to merge 1 commit into
Conversation
When `MergeYaml` merged a new entry into a mapping whose surrounding blank lines are stored on the document end prefix or on a sibling entry's prefix, the recipe treated that plain trailing whitespace as an inline comment and moved it onto the newly inserted entry. As a result the new entry was pushed past the blank lines instead of landing directly next to its sibling. Two fixes: - Only treat a document end prefix as a comment when it actually contains one, so plain trailing blank lines stay at the document end. - When inserting `Before` an entry whose prefix carries leading blank lines (but no comment), keep those blank lines at the top of the block and place the new entry directly before the existing sibling. Fixes moderneinc/support-morganstanley#274
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.
What's changed?
When
MergeYamlmerges a new entry into a mapping that is surrounded by blank lines, the recipe mistakes that plain trailing whitespace for an inline comment and moves it onto the newly inserted entry. The new entry is therefore pushed past the blank lines instead of landing directly next to its sibling.Blank lines after the last sibling — the trailing whitespace lives on the document end prefix:
Blank lines before the entry, inserting
Beforeit — the blank lines live on the entry's prefix:The fix
Two targeted changes in
MergeYamlVisitor.mergeMapping:#; plain trailing blank lines now stay at the document end. (Mirrors the guard already present on the sibling-mapping branch.)Beforeinsertion — when inserting before an entry whose prefix carries leading blank lines but no comment, keep those blank lines at the top of the block and place the new entry directly before the existing sibling.Testing
Two parameterized tests in
MergeYamlTestexercise everyinsertMode/insertPropertyplacement (null,Last,After,Before) for both the blank-lines-after and blank-lines-before scenarios. ExistingBefore/comment tests (e.g.insertBeforeElementWithCommentsWithNesting) confirm full-line comments above the target still stay attached to it, and the wholerewrite-yamlmodule is green.