Skip to content

fix: remove all manual folding ranges when selection is empty - #304793

Merged
Dmitriy Vasyura (dmitrivMS) merged 9 commits into
microsoft:mainfrom
yogeshwaran-c:fix/remove-manual-folding-empty-selection
Jul 29, 2026
Merged

fix: remove all manual folding ranges when selection is empty#304793
Dmitriy Vasyura (dmitrivMS) merged 9 commits into
microsoft:mainfrom
yogeshwaran-c:fix/remove-manual-folding-empty-selection

Conversation

@yogeshwaran-c

@yogeshwaran-c Yogeshwaran C (yogeshwaran-c) commented Mar 25, 2026 •
edited by dmitrivMS
Loading

Copy link
Copy Markdown
Contributor

Summary

When invoking editor.removeManualFoldingRanges with an empty selection (cursor only):

  1. If the cursor is on a manual folding range → removes only the innermost manual range (addresses [folding] Remove manual folding range should only remove most inner folding range #212599)
  2. If the cursor is NOT on any manual range → removes all manual folding ranges in the document (addresses editor.removeManualFoldingRanges should support empty selection #266597)

When text is explicitly selected, the existing behavior is preserved — only manual ranges intersecting the selection are removed.

This combined approach follows RedCMD (@RedCMD)'s suggestion from #266597 to detect whether a range intersects the empty selection, and also satisfies Martin Aeschlimann (@aeschli)'s request to support empty selection for removing all manual folding ranges.

Fixes #266597
Fixes #212599

Test plan

  • Open a file, manually create folding ranges, place cursor on one, run command → only innermost manual range removed
  • Place cursor on a line with NO manual folding range, run command → all manual ranges removed
  • Select text spanning manual ranges, run command → only intersecting ranges removed

@yogeshwaran-c
fix: remove all manual folding ranges when selection is empty
542ce16 
When editor.removeManualFoldingRanges is invoked with an empty selection
(just a cursor, no text selected), remove all manual folding ranges in
the document instead of only removing ranges intersecting the cursor
line. This makes it easy to clear broken manual folding ranges without
having to select all text first.

When text is explicitly selected, the existing behavior of only removing
manual ranges intersecting the selection is preserved.

Closes microsoft#266597
@vs-code-engineering vs-code-engineering Bot added this to the 1.114.0 milestone Mar 25, 2026
@yogeshwaran-c @claude
fix: combine innermost removal with full-range fallback
8b259d5 
When cursor is on a manual folding range, remove only the innermost one.
When cursor is NOT on any manual range, remove all manual ranges.
This addresses both microsoft#266597 and microsoft#212599 in a single approach.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@yogeshwaran-c

Yogeshwaran C (yogeshwaran-c) commented Mar 25, 2026

Copy link
Copy Markdown
Contributor Author

Updated the implementation based on RedCMD (@RedCMD)'s suggestion from #266597. The combined approach now:

  1. Cursor on a manual folding range → removes only the innermost manual range (walks up the tree via getParentIndex to find the closest non-provider range)
  2. Cursor NOT on any manual range → removes all manual ranges (uses full document range)

This addresses both #266597 and #212599 in a single, consistent behavior. PR #304808 is now superseded by this approach.

This was referenced Mar 25, 2026
Closed
Closed
aeschli
Martin Aeschlimann (aeschli) requested changes Mar 26, 2026
View reviewed changes
Comment thread src/vs/editor/contrib/folding/browser/folding.ts Outdated
@aeschli

Martin Aeschlimann (aeschli) commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Also please add tests.

@yogeshwaran-c @claude
refactor: move cursor-aware logic into removeManualRanges
d6d858f 
Address review feedback: move the "find innermost manual range at cursor,
or remove all" logic from RemoveFoldRangeFromSelectionAction.invoke()
into FoldingModel.removeManualRanges(), since that action is the only
caller. The action now simply passes raw selection ranges as before.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@yogeshwaran-c @claude
test: add tests for removeManualRanges cursor behavior
fd16b6a 
Tests cover three scenarios:
- Cursor on a manual range removes only the innermost manual range
- Cursor not on any manual range removes all manual ranges
- Selection range removes only intersecting manual ranges

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@yogeshwaran-c

Yogeshwaran C (yogeshwaran-c) commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

Added tests for removeManualRanges covering all three scenarios:

  1. Cursor on a manual range → removes only the innermost manual range (nested manual ranges peeled one at a time)
  2. Cursor not on any manual range → removes all manual ranges in the document
  3. Selection range → removes only intersecting manual ranges (existing behavior preserved)

@alexr00 Alex Ross (alexr00) removed this from the 1.114.0 milestone Mar 30, 2026
@manyiResearch

Manyi Wang (manyiResearch) commented Jul 3, 2026

Copy link
Copy Markdown

Hi, a small quick question — does this PR fix issues #266597 and #212599 together? Thanks!

Copilot AI review requested due to automatic review settings July 6, 2026 15:45
Copilot AI reviewed Jul 6, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR changes FoldingModel.removeManualRanges (used by the editor.removeManualFoldingRanges command) so that a cursor-only invocation is handled specially. When a range spans a single line, it now walks up from the innermost folding region at that line to find the nearest manual (non-provider) range and removes only that one (fixing #212599, where the outer range was removed instead of the innermost). If no manual range contains the cursor, it removes every manual range in the document (fixing #266597). Multi-line selections keep the previous "remove intersecting manual ranges" behavior.

Changes:

  • Added innermost-manual-range resolution and an "remove all manual ranges" fallback for single-line ranges in removeManualRanges.
  • Reworked the intersects helper to operate on the resolved ranges instead of the raw input ranges.
  • Added three unit tests covering cursor-on-manual, cursor-not-on-manual, and multi-line selection cases.
Show a summary per file
File Description
src/vs/editor/contrib/folding/browser/foldingModel.ts Adds single-line (cursor) handling: remove innermost manual range, else remove all manual ranges; intersects now uses resolved ranges.
src/vs/editor/contrib/folding/test/browser/foldingModel.test.ts Adds tests for innermost removal, remove-all fallback, and multi-line selection behavior; imports FoldingRegions, FoldRange, FoldSource.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread src/vs/editor/contrib/folding/browser/foldingModel.ts Outdated
@dmitrivMS
fix manual folding range removal
72b683b 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dmitrivMS

Dmitriy Vasyura (dmitrivMS) commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Addressed the review feedback in 72b683b16f9:

  • RemoveFoldRangeFromSelectionAction now passes the original selections to FoldingModel.removeManualRanges, preserving column information.
  • removeManualRanges uses Range.isEmpty to distinguish cursor-only invocations from explicit single-line text selections, so a non-empty selection no longer removes all manual ranges.
  • Cursor handling removes the exact innermost manual region by index. This also fixes the previous implementation's intersection behavior, which could remove containing manual ranges along with the innermost one.
  • Added regression coverage for a non-empty single-line selection outside manual ranges.

Validation: all 21 tests in foldingModel.test.ts pass, including the three removeManualRanges scenarios; pre-commit hygiene passes.

roblourens
roblourens previously approved these changes Jul 29, 2026
View reviewed changes
@dmitrivMS
test manual folding range edge cases
dfce22a 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dmitrivMS
Dmitriy Vasyura (dmitrivMS) dismissed stale reviews from roblourens and themself via dfce22a July 29, 2026 03:51
@dmitrivMS
Dmitriy Vasyura (dmitrivMS) enabled auto-merge (squash) July 29, 2026 05:12
@dmitrivMS
Dmitriy Vasyura (dmitrivMS) merged commit e85daf1 into microsoft:main Jul 29, 2026
29 checks passed
@vs-code-engineering vs-code-engineering Bot added this to the 1.132.0 milestone Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@dmitrivMS Dmitriy Vasyura (dmitrivMS) dmitrivMS approved these changes
@alexr00 Alex Ross (alexr00) alexr00 approved these changes
@roblourens roblourens roblourens left review comments
@hediet Henning Dieterichs (hediet) hediet left review comments
@aeschli Martin Aeschlimann (aeschli) aeschli left review comments
@vritant24 Vritant Bhardwaj (vritant24) vritant24 left review comments

Assignees

@aeschli Martin Aeschlimann (aeschli)

Labels

None yet

Projects

None yet

Milestone

1.132.0

Development

Successfully merging this pull request may close these issues.

editor.removeManualFoldingRanges should support empty selection [folding] Remove manual folding range should only remove most inner folding range

9 participants

@yogeshwaran-c @aeschli @manyiResearch @dmitrivMS @roblourens @hediet @vritant24 @alexr00