Keep the eol comment on an empty block sequence entry#123
Open
sarathfrancis90 wants to merge 2 commits into
Open
Keep the eol comment on an empty block sequence entry#123sarathfrancis90 wants to merge 2 commits into
sarathfrancis90 wants to merge 2 commits into
Conversation
A null (empty) block sequence entry has no value token for an
end-of-line comment to attach to, so the scanner queues that comment as
one preceding the next token. The parser let it fall through to the
sequence end, so dump(load(x)) was not idempotent for e.g.
- null # comment
On the first dump the comment stays on the entry's line, but reloading
that output moves it onto the sequence end, so a second dump drifts it
onto its own line. With several empty entries the comments were even
reassigned to the wrong items.
Attach a comment that sits on the same line as the '-' indicator to the
(empty) entry itself, mirroring how empty block mapping values are
already handled. This covers both regular and indentless sequences.
for more information, see https://pre-commit.ci
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
1 participant
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.
While round-tripping some config files I hit a case where
dump(load(x))isn't idempotent. A block sequence entry whose value is null/empty but carries an end-of-line comment moves the comment onto its own line on the second round-trip:Reload that output and dump again and you get:
It's worse with several empty entries: their comments all slide down and end up attached to the wrong items.
Root cause: an empty entry has no value token for the eol comment to attach to, so the scanner queues it as a comment preceding the next token.
parse_block_sequence_entry(and the indentless variant) then let it fall through to the sequence end instead of keeping it on the entry. Empty block mapping values (a: # comment) already handle this correctly, so the sequence path was just missing the equivalent step.The fix attaches a comment that sits on the same line as the
-indicator to the empty entry itself, leaving genuine standalone comments (on their own line) alone.Tested with two new cases in
_test/test_comments.pycovering the single-entry idempotency and the multi-entry attachment; the full_test/test_*.pysuite still passes.