jsonEdit: preserve comments above a property when it is deleted#309171
Open
yogeshwaran-c wants to merge 1 commit intomicrosoft:mainfrom
Open
jsonEdit: preserve comments above a property when it is deleted#309171yogeshwaran-c wants to merge 1 commit intomicrosoft:mainfrom
yogeshwaran-c wants to merge 1 commit intomicrosoft:mainfrom
Conversation
When removeProperty (used e.g. to reset a setting to its default value)
removed the first/only property or a property that was not the first in
the object, it calculated a removal range that started before the property
itself — often including comment lines that appeared immediately above it.
The root causes:
- propertyIndex === 0: removeBegin was set to parent.offset + 1 (just after
the opening brace), which includes any comments between '{' and the
property.
- propertyIndex > 0: removeBegin was set to previous.offset + previous.length
(the trailing comma of the previous property), and removeEnd to the end of
the current property. Any comment between the previous property and the
current one fell inside this range.
Fix: anchor the removal to the property's own line rather than to the
surrounding structure. When content (comments) is detected between the
previous anchor point and the start of the property's line, use two
separate edits — one to remove the trailing comma and one to remove the
property line — so the intermediate content is preserved. Update the
test suite with cases that cover both the first-property and non-first-
property scenarios.
Fixes microsoft#275792
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.
When
removeProperty(called e.g. when a setting is reset to its default value via the Settings UI or keyboard shortcuts) removed a property fromsettings.json, it also silently deleted any comment placed on the line immediately above the property.Root cause
The deletion range was anchored to surrounding structure rather than to the property's own line:
removeBegin(before fix)parent.offset + 1(just after{){and the propertyprevious.offset + previous.length(trailing comma of the previous sibling)Fix
Anchor the removal to the property's own line (using the newline that precedes the line as the boundary):
propertyIndex === 0):removeBeginis set to the start of the property's line (including its preceding newline) instead ofparent.offset + 1.propertyIndex > 0) with a comment between the previous property and this one: two separate edits are returned — one that removes only the trailing comma from the previous sibling, and one that removes the property's line (including its own trailing comma for middle properties). The intermediate content (comments) is left intact.Tests
Three new test cases in
jsonEdit.test.ts:Fixes #275792