fix: ensure inline container tightening for deletions - #278
Merged
Conversation
…ongside InlineTable When the only item is removed from a single-line InlineArray, mark it for bracket tightening just like InlineTable. Rename the tracking helpers from inlineTableNeedingTighten to inlineContainerNeedingTighten and add the InlineArray case to the post-patch tightening traversal. Extract tightenInlineContainerEnd() to share the end-column logic between InlineTable and InlineArray cases.
…iners
Three related issues where removing items from single-line inline arrays
or inline tables left trailing whitespace inside brackets:
1. InlineArray was missing from the "needs tightening" tracking in
writer.ts — only InlineTable was marked when its last item was
removed. Generalized the helpers to inlineContainersNeedingTighten.
2. The post-patch tightening traversal in patch.ts only handled
InlineTable. Added InlineArray case, extracted shared logic into
tightenInlineContainerEnd().
3. recalcInlineContainerEnds only looked through InlineItems wrapping
KeyValues (inline table entries), missing InlineItems wrapping
InlineTable/InlineArray directly (inline array elements like
`[ {} ]`). Added resolveInnerEndCol() to handle both cases.
There was a problem hiding this comment.
Pull request overview
This PR generalizes the “tighten inline container end after deletions” behavior so that it applies to both single-line InlineTable and InlineArray cases, fixing trailing-whitespace artifacts when the only element is removed.
Changes:
- Generalize writer-side tracking from inline tables to inline containers (tables + arrays) for post-remove tightening.
- Add shared helpers in
patch.tsto tighten container end columns and to correctly recompute parent container ends when nested containers are tightened. - Convert previously failing regression tests for these whitespace issues into passing tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/writer.ts | Generalizes “needs tighten” tracking to cover single-line inline arrays as well as inline tables during remove(). |
| src/patch.ts | Applies tightening for both inline tables/arrays and refactors end-column recalculation logic via helpers. |
| src/tests/patch.test.ts | Promotes three inline-array/table whitespace regression cases from expected-fail to passing tests. |
Suppressed comments (1)
src/patch.ts:1675
- This docstring still refers specifically to a nested
InlineTable, but this code path is now used for both InlineTables and InlineArrays. Updating the wording will keep the comment accurate as the behavior generalizes.
* After tightening a nested single-line InlineTable (whose only item was
* removed), recalculate the end positions of any parent single-line InlineTable
* or InlineArray containers. The remove() call zeroed the offset for the
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
DecimalTurn
changed the title
…ory leaks Only has/delete are needed, so a WeakSet is sufficient and prevents retaining CST nodes if an exception occurs before cleanup or if a caller forgets to delete.
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.
No description provided.