From 89c30e1b86f941db095d9f52b128287e5039e004 Mon Sep 17 00:00:00 2001 From: Martin Aeschlimann Date: Wed, 27 Jul 2022 23:02:44 +0200 Subject: manual folding ranges: memento does not store all sources (#156273) use source: fix memento does not store all sources --- src/vs/editor/contrib/folding/browser/folding.ts | 5 +- .../editor/contrib/folding/browser/foldingModel.ts | 24 ++--- .../contrib/folding/browser/foldingRanges.ts | 69 ++++++++----- .../folding/test/browser/foldingRanges.test.ts | 108 ++++++++++----------- 4 files changed, 109 insertions(+), 97 deletions(-) (limited to 'src/vs') diff --git a/src/vs/editor/contrib/folding/browser/folding.ts b/src/vs/editor/contrib/folding/browser/folding.ts index fec26e69c77..c35cdb69a65 100644 --- a/src/vs/editor/contrib/folding/browser/folding.ts +++ b/src/vs/editor/contrib/folding/browser/folding.ts @@ -32,7 +32,7 @@ import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegis import { editorSelectionBackground, iconForeground, registerColor, transparent } from 'vs/platform/theme/common/colorRegistry'; import { registerThemingParticipant, ThemeIcon } from 'vs/platform/theme/common/themeService'; import { foldingCollapsedIcon, FoldingDecorationProvider, foldingExpandedIcon, foldingManualCollapsedIcon, foldingManualExpandedIcon } from './foldingDecorations'; -import { FoldingRegion, FoldingRegions, FoldRange, ILineRange } from './foldingRanges'; +import { FoldingRegion, FoldingRegions, FoldRange, FoldSource, ILineRange } from './foldingRanges'; import { SyntaxRangeProvider } from './syntaxRangeProvider'; import { INotificationService } from 'vs/platform/notification/common/notification'; import Severity from 'vs/base/common/severity'; @@ -1097,8 +1097,7 @@ class FoldRangeFromSelectionAction extends FoldingAction { endLineNumber: endLineNumber, type: undefined, isCollapsed: true, - isUserDefined: true, - isRecovered: false + source: FoldSource.userDefined }); editor.setSelection({ startLineNumber: selection.startLineNumber, diff --git a/src/vs/editor/contrib/folding/browser/foldingModel.ts b/src/vs/editor/contrib/folding/browser/foldingModel.ts index f3bc09b1ce3..936e83b605f 100644 --- a/src/vs/editor/contrib/folding/browser/foldingModel.ts +++ b/src/vs/editor/contrib/folding/browser/foldingModel.ts @@ -5,7 +5,7 @@ import { Emitter, Event } from 'vs/base/common/event'; import { IModelDecorationOptions, IModelDecorationsChangeAccessor, IModelDeltaDecoration, ITextModel } from 'vs/editor/common/model'; -import { FoldingRegion, FoldingRegions, ILineRange, FoldRange } from './foldingRanges'; +import { FoldingRegion, FoldingRegions, ILineRange, FoldRange, FoldSource } from './foldingRanges'; import { hash } from 'vs/base/common/hash'; export interface IDecorationProvider { @@ -22,7 +22,7 @@ export interface FoldingModelChangeEvent { interface ILineMemento extends ILineRange { checksum?: number; isCollapsed?: boolean; - isUserDefined?: boolean; + source?: FoldSource; } export type CollapseMemento = ILineMemento[]; @@ -64,7 +64,7 @@ export class FoldingModel { const endLineNumber = this._regions.getEndLineNumber(k); const isCollapsed = this._regions.isCollapsed(k); if (endLineNumber <= dirtyRegionEndLine) { - const isManual = this.regions.isUserDefined(k) || this.regions.isRecovered(k); + const isManual = this.regions.getSource(k) !== FoldSource.provider; accessor.changeDecorationOptions(this._editorDecorationIds[k], this._decorationProvider.getDecorationOption(isCollapsed, endLineNumber <= lastHiddenLine, isManual)); } if (isCollapsed && endLineNumber > lastHiddenLine) { @@ -104,7 +104,7 @@ export class FoldingModel { }; for (let i = 0; i < this._regions.length; i++) { const foldRange = this._regions.toFoldRange(i); - if (!foldRange.isUserDefined && !foldRange.isRecovered || !intersects(foldRange)) { + if (foldRange.source === FoldSource.provider || !intersects(foldRange)) { newFoldingRanges.push(foldRange); } } @@ -124,7 +124,7 @@ export class FoldingModel { const startLineNumber = newRegions.getStartLineNumber(index); const endLineNumber = newRegions.getEndLineNumber(index); const isCollapsed = newRegions.isCollapsed(index); - const isManual = newRegions.isUserDefined(index) || newRegions.isRecovered(index); + const isManual = newRegions.getSource(index) !== FoldSource.provider; const decorationRange = { startLineNumber: startLineNumber, startColumn: this._textModel.getLineMaxColumn(startLineNumber), @@ -155,9 +155,8 @@ export class FoldingModel { const foldedRanges: FoldRange[] = []; for (let i = 0, limit = this._regions.length; i < limit; i++) { let isCollapsed = this.regions.isCollapsed(i); - const isUserDefined = this.regions.isUserDefined(i); - const isRecovered = this.regions.isRecovered(i); - if (isCollapsed || isUserDefined || isRecovered) { + const source = this.regions.getSource(i); + if (isCollapsed || source !== FoldSource.provider) { const foldRange = this._regions.toFoldRange(i); const decRange = this._textModel.getDecorationRange(this._editorDecorationIds[i]); if (decRange) { @@ -169,8 +168,7 @@ export class FoldingModel { endLineNumber: decRange.endLineNumber, type: foldRange.type, isCollapsed, - isUserDefined, - isRecovered + source }); } } @@ -192,7 +190,7 @@ export class FoldingModel { startLineNumber: range.startLineNumber, endLineNumber: range.endLineNumber, isCollapsed: range.isCollapsed, - isUserDefined: range.isRecovered, + source: range.source, checksum: checksum }); } @@ -214,14 +212,12 @@ export class FoldingModel { } const checksum = this._getLinesChecksum(range.startLineNumber + 1, range.endLineNumber); if (!range.checksum || checksum === range.checksum) { - const isUserDefined = range.isUserDefined === true; rangesToRestore.push({ startLineNumber: range.startLineNumber, endLineNumber: range.endLineNumber, type: undefined, isCollapsed: range.isCollapsed ?? true, - isUserDefined, - isRecovered: !isUserDefined + source: range.source ?? FoldSource.provider }); } } diff --git a/src/vs/editor/contrib/folding/browser/foldingRanges.ts b/src/vs/editor/contrib/folding/browser/foldingRanges.ts index 2dfb1e50003..8beb820778d 100644 --- a/src/vs/editor/contrib/folding/browser/foldingRanges.ts +++ b/src/vs/editor/contrib/folding/browser/foldingRanges.ts @@ -8,13 +8,18 @@ export interface ILineRange { endLineNumber: number; } +export const enum FoldSource { + provider = 0, + userDefined = 1, + recovered = 2 +} + export interface FoldRange { startLineNumber: number; endLineNumber: number; type: string | undefined; isCollapsed: boolean; - isUserDefined: boolean; - isRecovered: boolean; + source: FoldSource; } export const MAX_FOLDING_REGIONS = 0xFFFF; @@ -123,22 +128,44 @@ export class FoldingRegions { this._collapseStates.set(index, newState); } - public isUserDefined(index: number): boolean { + private isUserDefined(index: number): boolean { return this._userDefinedStates.get(index); } - public setUserDefined(index: number, newState: boolean) { + private setUserDefined(index: number, newState: boolean) { return this._userDefinedStates.set(index, newState); } - public isRecovered(index: number): boolean { + private isRecovered(index: number): boolean { return this._recoveredStates.get(index); } - public setRecovered(index: number, newState: boolean) { + private setRecovered(index: number, newState: boolean) { return this._recoveredStates.set(index, newState); } + public getSource(index: number): FoldSource { + if (this.isUserDefined(index)) { + return FoldSource.userDefined; + } else if (this.isRecovered(index)) { + return FoldSource.recovered; + } + return FoldSource.provider; + } + + public setSource(index: number, source: FoldSource): void { + if (source === FoldSource.userDefined) { + this.setUserDefined(index, true); + this.setRecovered(index, false); + } else if (source === FoldSource.recovered) { + this.setUserDefined(index, false); + this.setRecovered(index, true); + } else { + this.setUserDefined(index, false); + this.setRecovered(index, false); + } + } + public setCollapsedAllOfType(type: string, newState: boolean) { let hasChanged = false; if (this._types) { @@ -203,10 +230,16 @@ export class FoldingRegions { return -1; } + private readonly sourceAbbr = { + [FoldSource.provider]: ' ', + [FoldSource.userDefined]: 'u', + [FoldSource.recovered]: 'r', + }; + public toString() { const res: string[] = []; for (let i = 0; i < this.length; i++) { - res[i] = `[${this.isUserDefined(i) ? '*' : ' '}${this.isRecovered(i) ? 'r' : ' '}${this.isCollapsed(i) ? '+' : '-'}] ${this.getStartLineNumber(i)}/${this.getEndLineNumber(i)}`; + res[i] = `[${this.sourceAbbr[this.getSource(i)]}${this.isCollapsed(i) ? '+' : '-'}] ${this.getStartLineNumber(i)}/${this.getEndLineNumber(i)}`; } return res.join(', '); } @@ -217,8 +250,7 @@ export class FoldingRegions { endLineNumber: this._endIndexes[index] & MAX_LINE_NUMBER, type: this._types ? this._types[index] : undefined, isCollapsed: this.isCollapsed(index), - isUserDefined: this.isUserDefined(index), - isRecovered: this.isRecovered(index) + source: this.getSource(index) }; } @@ -245,12 +277,7 @@ export class FoldingRegions { if (ranges[i].isCollapsed) { regions.setCollapsed(i, true); } - if (ranges[i].isUserDefined) { - regions.setUserDefined(i, true); - } - if (ranges[i].isRecovered) { - regions.setRecovered(i, true); - } + regions.setSource(i, ranges[i].source); } return regions; } @@ -296,23 +323,21 @@ export class FoldingRegions { let useRange: FoldRange | undefined = undefined; if (nextB && (!nextA || nextA.startLineNumber >= nextB.startLineNumber)) { if (nextA && nextA.startLineNumber === nextB.startLineNumber) { - if (nextB.isUserDefined) { + if (nextB.source === FoldSource.userDefined) { // a user defined range (possibly unfolded) useRange = nextB; } else { // a previously folded range or a (possibly unfolded) recovered range useRange = nextA; useRange.isCollapsed = nextB.isCollapsed && nextA.endLineNumber === nextB.endLineNumber; - useRange.isUserDefined = false; - useRange.isRecovered = false; + useRange.source = FoldSource.provider; } nextA = getA(++indexA); // not necessary, just for speed } else { useRange = nextB; - if (nextB.isCollapsed && !nextB.isUserDefined && !nextB.isRecovered) { + if (nextB.isCollapsed && nextB.source === FoldSource.provider) { // a previously collapsed range - useRange.isRecovered = true; - useRange.isUserDefined = false; + useRange.source = FoldSource.recovered; } } nextB = getB(++indexB); @@ -326,7 +351,7 @@ export class FoldingRegions { useRange = nextA; break; // no conflict, use this nextA } - if (prescanB.isUserDefined && prescanB.endLineNumber > nextA!.endLineNumber) { + if (prescanB.source === FoldSource.userDefined && prescanB.endLineNumber > nextA!.endLineNumber) { // we found a user folded range, it wins break; // without setting nextResult, so this nextA gets skipped } diff --git a/src/vs/editor/contrib/folding/test/browser/foldingRanges.test.ts b/src/vs/editor/contrib/folding/test/browser/foldingRanges.test.ts index 671277579ee..9f6a381afbd 100644 --- a/src/vs/editor/contrib/folding/test/browser/foldingRanges.test.ts +++ b/src/vs/editor/contrib/folding/test/browser/foldingRanges.test.ts @@ -5,7 +5,7 @@ import * as assert from 'assert'; import { FoldingMarkers } from 'vs/editor/common/languages/languageConfiguration'; -import { MAX_FOLDING_REGIONS, FoldRange, FoldingRegions } from 'vs/editor/contrib/folding/browser/foldingRanges'; +import { MAX_FOLDING_REGIONS, FoldRange, FoldingRegions, FoldSource } from 'vs/editor/contrib/folding/browser/foldingRanges'; import { computeRanges } from 'vs/editor/contrib/folding/browser/indentRangeProvider'; import { createTextModel } from 'vs/editor/test/common/testTextModel'; @@ -14,30 +14,22 @@ const markers: FoldingMarkers = { end: /^\s*#endregion\b/ }; -enum State { - none = 0, - userDefined = 1, - recovered = 2 -} - suite('FoldingRanges', () => { - const foldRange = (from: number, to: number, collapsed: boolean | undefined = undefined, state: State = State.none, type: string | undefined = undefined) => + const foldRange = (from: number, to: number, collapsed: boolean | undefined = undefined, source: FoldSource = FoldSource.provider, type: string | undefined = undefined) => { startLineNumber: from, endLineNumber: to, type: type, isCollapsed: collapsed || false, - isUserDefined: state === State.userDefined, - isRecovered: state === State.recovered, + source }; const assertEqualRanges = (range1: FoldRange, range2: FoldRange, msg: string) => { assert.strictEqual(range1.startLineNumber, range2.startLineNumber, msg + ' start'); assert.strictEqual(range1.endLineNumber, range2.endLineNumber, msg + ' end'); assert.strictEqual(range1.type, range2.type, msg + ' type'); assert.strictEqual(range1.isCollapsed, range2.isCollapsed, msg + ' collapsed'); - assert.strictEqual(range1.isUserDefined, range2.isUserDefined, msg + ' userDefined'); - assert.strictEqual(range1.isRecovered, range2.isRecovered, msg + ' recovered'); + assert.strictEqual(range1.source, range2.source, msg + ' source'); }; test('test max folding regions', () => { @@ -130,88 +122,88 @@ suite('FoldingRanges', () => { test('sanitizeAndMerge1', () => { const regionSet1: FoldRange[] = [ foldRange(0, 100), // invalid, should be removed - foldRange(1, 100, false, State.none, 'A'), // valid - foldRange(1, 100, false, State.none, 'Z'), // invalid, duplicate start + foldRange(1, 100, false, FoldSource.provider, 'A'), // valid + foldRange(1, 100, false, FoldSource.provider, 'Z'), // invalid, duplicate start foldRange(10, 10, false), // invalid, should be removed - foldRange(20, 80, false, State.none, 'C1'), // valid inside 'B' - foldRange(22, 80, true, State.none, 'D1'), // valid inside 'C1' + foldRange(20, 80, false, FoldSource.provider, 'C1'), // valid inside 'B' + foldRange(22, 80, true, FoldSource.provider, 'D1'), // valid inside 'C1' foldRange(90, 101), // invalid, should be removed ]; const regionSet2: FoldRange[] = [ foldRange(20, 80, true), // should merge with C1 foldRange(18, 80, true), // invalid, out of order - foldRange(21, 81, true, State.none, 'Z'), // invalid, overlapping - foldRange(22, 80, true, State.none, 'D2'), // should merge with D1 + foldRange(21, 81, true, FoldSource.provider, 'Z'), // invalid, overlapping + foldRange(22, 80, true, FoldSource.provider, 'D2'), // should merge with D1 ]; const result = FoldingRegions.sanitizeAndMerge(regionSet1, regionSet2, 100); assert.strictEqual(result.length, 3, 'result length1'); - assertEqualRanges(result[0], foldRange(1, 100, false, State.none, 'A'), 'A1'); - assertEqualRanges(result[1], foldRange(20, 80, true, State.none, 'C1'), 'C1'); - assertEqualRanges(result[2], foldRange(22, 80, true, State.none, 'D1'), 'D1'); + assertEqualRanges(result[0], foldRange(1, 100, false, FoldSource.provider, 'A'), 'A1'); + assertEqualRanges(result[1], foldRange(20, 80, true, FoldSource.provider, 'C1'), 'C1'); + assertEqualRanges(result[2], foldRange(22, 80, true, FoldSource.provider, 'D1'), 'D1'); }); test('sanitizeAndMerge2', () => { const regionSet1: FoldRange[] = [ - foldRange(1, 100, false, State.none, 'a1'), // valid - foldRange(2, 100, false, State.none, 'a2'), // valid - foldRange(3, 19, false, State.none, 'a3'), // valid - foldRange(20, 71, false, State.none, 'a4'), // overlaps b3 - foldRange(21, 29, false, State.none, 'a5'), // valid - foldRange(81, 91, false, State.none, 'a6'), // overlaps b4 + foldRange(1, 100, false, FoldSource.provider, 'a1'), // valid + foldRange(2, 100, false, FoldSource.provider, 'a2'), // valid + foldRange(3, 19, false, FoldSource.provider, 'a3'), // valid + foldRange(20, 71, false, FoldSource.provider, 'a4'), // overlaps b3 + foldRange(21, 29, false, FoldSource.provider, 'a5'), // valid + foldRange(81, 91, false, FoldSource.provider, 'a6'), // overlaps b4 ]; const regionSet2: FoldRange[] = [ - foldRange(30, 39, true, State.none, 'b1'), // valid, will be recovered - foldRange(40, 49, true, State.userDefined, 'b2'), // valid - foldRange(50, 100, true, State.userDefined, 'b3'), // overlaps a4 - foldRange(80, 90, true, State.userDefined, 'b4'), // overlaps a6 - foldRange(92, 100, true, State.userDefined, 'b5'), // valid + foldRange(30, 39, true, FoldSource.provider, 'b1'), // valid, will be recovered + foldRange(40, 49, true, FoldSource.userDefined, 'b2'), // valid + foldRange(50, 100, true, FoldSource.userDefined, 'b3'), // overlaps a4 + foldRange(80, 90, true, FoldSource.userDefined, 'b4'), // overlaps a6 + foldRange(92, 100, true, FoldSource.userDefined, 'b5'), // valid ]; const result = FoldingRegions.sanitizeAndMerge(regionSet1, regionSet2, 100); assert.strictEqual(result.length, 9, 'result length1'); - assertEqualRanges(result[0], foldRange(1, 100, false, State.none, 'a1'), 'P1'); - assertEqualRanges(result[1], foldRange(2, 100, false, State.none, 'a2'), 'P2'); - assertEqualRanges(result[2], foldRange(3, 19, false, State.none, 'a3'), 'P3'); - assertEqualRanges(result[3], foldRange(21, 29, false, State.none, 'a5'), 'P4'); - assertEqualRanges(result[4], foldRange(30, 39, true, State.recovered, 'b1'), 'P5'); - assertEqualRanges(result[5], foldRange(40, 49, true, State.userDefined, 'b2'), 'P6'); - assertEqualRanges(result[6], foldRange(50, 100, true, State.userDefined, 'b3'), 'P7'); - assertEqualRanges(result[7], foldRange(80, 90, true, State.userDefined, 'b4'), 'P8'); - assertEqualRanges(result[8], foldRange(92, 100, true, State.userDefined, 'b5'), 'P9'); + assertEqualRanges(result[0], foldRange(1, 100, false, FoldSource.provider, 'a1'), 'P1'); + assertEqualRanges(result[1], foldRange(2, 100, false, FoldSource.provider, 'a2'), 'P2'); + assertEqualRanges(result[2], foldRange(3, 19, false, FoldSource.provider, 'a3'), 'P3'); + assertEqualRanges(result[3], foldRange(21, 29, false, FoldSource.provider, 'a5'), 'P4'); + assertEqualRanges(result[4], foldRange(30, 39, true, FoldSource.recovered, 'b1'), 'P5'); + assertEqualRanges(result[5], foldRange(40, 49, true, FoldSource.userDefined, 'b2'), 'P6'); + assertEqualRanges(result[6], foldRange(50, 100, true, FoldSource.userDefined, 'b3'), 'P7'); + assertEqualRanges(result[7], foldRange(80, 90, true, FoldSource.userDefined, 'b4'), 'P8'); + assertEqualRanges(result[8], foldRange(92, 100, true, FoldSource.userDefined, 'b5'), 'P9'); }); test('sanitizeAndMerge3', () => { const regionSet1: FoldRange[] = [ - foldRange(1, 100, false, State.none, 'a1'), // valid - foldRange(10, 29, false, State.none, 'a2'), // matches manual hidden - foldRange(35, 39, true, State.recovered, 'a3'), // valid + foldRange(1, 100, false, FoldSource.provider, 'a1'), // valid + foldRange(10, 29, false, FoldSource.provider, 'a2'), // matches manual hidden + foldRange(35, 39, true, FoldSource.recovered, 'a3'), // valid ]; const regionSet2: FoldRange[] = [ - foldRange(10, 29, true, State.recovered, 'b1'), // matches a - foldRange(20, 28, true, State.none, 'b2'), // should remain - foldRange(30, 39, true, State.recovered, 'b3'), // should remain + foldRange(10, 29, true, FoldSource.recovered, 'b1'), // matches a + foldRange(20, 28, true, FoldSource.provider, 'b2'), // should remain + foldRange(30, 39, true, FoldSource.recovered, 'b3'), // should remain ]; const result = FoldingRegions.sanitizeAndMerge(regionSet1, regionSet2, 100); assert.strictEqual(result.length, 5, 'result length3'); - assertEqualRanges(result[0], foldRange(1, 100, false, State.none, 'a1'), 'R1'); - assertEqualRanges(result[1], foldRange(10, 29, true, State.none, 'a2'), 'R2'); - assertEqualRanges(result[2], foldRange(20, 28, true, State.recovered, 'b2'), 'R3'); - assertEqualRanges(result[3], foldRange(30, 39, true, State.recovered, 'b3'), 'R3'); - assertEqualRanges(result[4], foldRange(35, 39, true, State.recovered, 'a3'), 'R4'); + assertEqualRanges(result[0], foldRange(1, 100, false, FoldSource.provider, 'a1'), 'R1'); + assertEqualRanges(result[1], foldRange(10, 29, true, FoldSource.provider, 'a2'), 'R2'); + assertEqualRanges(result[2], foldRange(20, 28, true, FoldSource.recovered, 'b2'), 'R3'); + assertEqualRanges(result[3], foldRange(30, 39, true, FoldSource.recovered, 'b3'), 'R3'); + assertEqualRanges(result[4], foldRange(35, 39, true, FoldSource.recovered, 'a3'), 'R4'); }); test('sanitizeAndMerge4', () => { const regionSet1: FoldRange[] = [ - foldRange(1, 100, false, State.none, 'a1'), // valid + foldRange(1, 100, false, FoldSource.provider, 'a1'), // valid ]; const regionSet2: FoldRange[] = [ - foldRange(20, 28, true, State.none, 'b1'), // hidden - foldRange(30, 38, true, State.none, 'b2'), // hidden + foldRange(20, 28, true, FoldSource.provider, 'b1'), // hidden + foldRange(30, 38, true, FoldSource.provider, 'b2'), // hidden ]; const result = FoldingRegions.sanitizeAndMerge(regionSet1, regionSet2, 100); assert.strictEqual(result.length, 3, 'result length4'); - assertEqualRanges(result[0], foldRange(1, 100, false, State.none, 'a1'), 'R1'); - assertEqualRanges(result[1], foldRange(20, 28, true, State.recovered, 'b1'), 'R2'); - assertEqualRanges(result[2], foldRange(30, 38, true, State.recovered, 'b2'), 'R3'); + assertEqualRanges(result[0], foldRange(1, 100, false, FoldSource.provider, 'a1'), 'R1'); + assertEqualRanges(result[1], foldRange(20, 28, true, FoldSource.recovered, 'b1'), 'R2'); + assertEqualRanges(result[2], foldRange(30, 38, true, FoldSource.recovered, 'b2'), 'R3'); }); }); -- cgit v1.2.3