From 5f313df36e25aa31ff071be75b8e8e591a05fbc9 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Wed, 22 May 2024 13:36:38 +0200 Subject: [PATCH] debt - some multi-tabs select cleanup (#213161) --- .../parts/editor/multiEditorTabsControl.ts | 65 +++---- .../parts/editor/multiRowEditorTabsControl.ts | 2 +- .../common/editor/editorGroupModel.ts | 178 ++++++++++-------- .../common/editor/filteredEditorGroupModel.ts | 2 +- 4 files changed, 137 insertions(+), 110 deletions(-) diff --git a/src/vs/workbench/browser/parts/editor/multiEditorTabsControl.ts b/src/vs/workbench/browser/parts/editor/multiEditorTabsControl.ts index a54b4a431d351..395c83aa7a172 100644 --- a/src/vs/workbench/browser/parts/editor/multiEditorTabsControl.ts +++ b/src/vs/workbench/browser/parts/editor/multiEditorTabsControl.ts @@ -889,14 +889,15 @@ export class MultiEditorTabsControl extends EditorTabsControl { const editor = this.tabsModel.getEditorByIndex(tabIndex); if (editor) { if (e.shiftKey) { - let anchor; + let anchor: EditorInput; if (this.lastSingleSelectSelectedEditor && this.tabsModel.isSelected(this.lastSingleSelectSelectedEditor)) { // The last selected editor is the anchor anchor = this.lastSingleSelectSelectedEditor; } else { // The active editor is the anchor - this.lastSingleSelectSelectedEditor = this.groupView.activeEditor!; - anchor = this.groupView.activeEditor!; + const activeEditor = assertIsDefined(this.groupView.activeEditor); + this.lastSingleSelectSelectedEditor = activeEditor; + anchor = activeEditor; } await this.selectEditorsBetween(editor, anchor); } else if ((e.ctrlKey && !isMacintosh) || (e.metaKey && isMacintosh)) { @@ -1292,7 +1293,7 @@ export class MultiEditorTabsControl extends EditorTabsControl { throw new BugIndicatingError(); } - const selection = this.groupView.selectedEditors; + let selection = this.groupView.selectedEditors; // Unselect editors on other side of anchor in relation to the target let currentIndex = anchorIndex; @@ -1308,7 +1309,7 @@ export class MultiEditorTabsControl extends EditorTabsControl { break; } - selection.filter(editor => !editor.matches(currentEditor)); + selection = selection.filter(editor => !editor.matches(currentEditor)); } // Select editors between anchor and target @@ -1334,7 +1335,7 @@ export class MultiEditorTabsControl extends EditorTabsControl { return; } - let newActiveEditor = this.groupView.activeEditor!; + let newActiveEditor = assertIsDefined(this.groupView.activeEditor); // If active editor is bing unselected then find the most recently opened selected editor // that is not the editor being unselected @@ -1355,7 +1356,8 @@ export class MultiEditorTabsControl extends EditorTabsControl { private async unselectAllEditors(): Promise { if (this.groupView.selectedEditors.length > 1) { - await this.groupView.setSelection(this.groupView.activeEditor!, []); + const activeEditor = assertIsDefined(this.groupView.activeEditor); + await this.groupView.setSelection(activeEditor, []); } } @@ -1643,7 +1645,6 @@ export class MultiEditorTabsControl extends EditorTabsControl { } private doRedrawTabActive(isGroupActive: boolean, allowBorderTop: boolean, editor: EditorInput, tabContainer: HTMLElement, tabActionBar: ActionBar): void { - const isActive = this.tabsModel.isActive(editor); const isSelected = this.tabsModel.isSelected(editor); @@ -1657,7 +1658,7 @@ export class MultiEditorTabsControl extends EditorTabsControl { if (isActive) { const activeTabBorderColorBottom = this.getColor(isGroupActive ? TAB_ACTIVE_BORDER : TAB_UNFOCUSED_ACTIVE_BORDER); tabContainer.classList.toggle('tab-border-bottom', !!activeTabBorderColorBottom); - tabContainer.style.setProperty('--tab-border-bottom-color', activeTabBorderColorBottom?.toString() ?? ''); + tabContainer.style.setProperty('--tab-border-bottom-color', activeTabBorderColorBottom ?? ''); } // Set border TOP if theme defined color @@ -2219,30 +2220,30 @@ export class MultiEditorTabsControl extends EditorTabsControl { else if (this.editorTransfer.hasData(DraggedEditorIdentifier.prototype)) { const data = this.editorTransfer.getData(DraggedEditorIdentifier.prototype); if (Array.isArray(data) && data.length > 0) { - const sourceGroup = data.length ? this.editorPartsView.getGroup(data[0].identifier.groupId) : undefined; - const isLocalMove = sourceGroup === this.groupView; - - // Keep the same order when moving / copying editors within the same group - for (const de of data) { - const editor = de.identifier.editor; - - // Only allow moving/copying from a single group source - if (!sourceGroup || sourceGroup.id !== de.identifier.groupId) { - continue; - } - - const sourceEditorIndex = sourceGroup.getIndexOfEditor(editor); - if (isLocalMove && sourceEditorIndex < targetEditorIndex) { - targetEditorIndex--; - } - - if (this.isMoveOperation(e, de.identifier.groupId, editor)) { - sourceGroup.moveEditor(editor, this.groupView, { ...options, index: targetEditorIndex }); - } else { - sourceGroup.copyEditor(editor, this.groupView, { ...options, index: targetEditorIndex }); + const sourceGroup = this.editorPartsView.getGroup(data[0].identifier.groupId); + if (sourceGroup) { + for (const de of data) { + const editor = de.identifier.editor; + + // Only allow moving/copying from a single group source + if (sourceGroup.id !== de.identifier.groupId) { + continue; + } + + // Keep the same order when moving / copying editors within the same group + const sourceEditorIndex = sourceGroup.getIndexOfEditor(editor); + if (sourceGroup === this.groupView && sourceEditorIndex < targetEditorIndex) { + targetEditorIndex--; + } + + if (this.isMoveOperation(e, de.identifier.groupId, editor)) { + sourceGroup.moveEditor(editor, this.groupView, { ...options, index: targetEditorIndex }); + } else { + sourceGroup.copyEditor(editor, this.groupView, { ...options, index: targetEditorIndex }); + } + + targetEditorIndex++; } - - targetEditorIndex++; } } diff --git a/src/vs/workbench/browser/parts/editor/multiRowEditorTabsControl.ts b/src/vs/workbench/browser/parts/editor/multiRowEditorTabsControl.ts index 908b7d85cfb80..83823d3ec8fa8 100644 --- a/src/vs/workbench/browser/parts/editor/multiRowEditorTabsControl.ts +++ b/src/vs/workbench/browser/parts/editor/multiRowEditorTabsControl.ts @@ -199,7 +199,7 @@ export class MultiRowEditorControl extends Disposable implements IEditorTabsCont return this.stickyEditorTabsControl.getHeight() + this.unstickyEditorTabsControl.getHeight(); } - public override dispose(): void { + override dispose(): void { this.parent.classList.toggle('two-tab-bars', false); super.dispose(); diff --git a/src/vs/workbench/common/editor/editorGroupModel.ts b/src/vs/workbench/common/editor/editorGroupModel.ts index d8b7d956adc2d..6f04a87f062fc 100644 --- a/src/vs/workbench/common/editor/editorGroupModel.ts +++ b/src/vs/workbench/common/editor/editorGroupModel.ts @@ -183,7 +183,7 @@ export interface IReadonlyEditorGroupModel { isActive(editor: EditorInput | IUntypedEditorInput): boolean; isPinned(editorOrIndex: EditorInput | number): boolean; isSticky(editorOrIndex: EditorInput | number): boolean; - isSelected(editor: EditorInput | number): boolean; + isSelected(editorOrIndex: EditorInput | number): boolean; isTransient(editorOrIndex: EditorInput | number): boolean; isFirst(editor: EditorInput, editors?: EditorInput[]): boolean; isLast(editor: EditorInput, editors?: EditorInput[]): boolean; @@ -222,9 +222,6 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel { private selection: EditorInput[] = []; // editors in selected state, first one is active - private set active(editor: EditorInput | null) { - this.selection = editor ? [editor] : []; - } private get active(): EditorInput | null { return this.selection[0] ?? null; } @@ -299,8 +296,8 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel { return this.active; } - isActive(editor: EditorInput | IUntypedEditorInput): boolean { - return this.matches(this.active, editor); + isActive(candidate: EditorInput | IUntypedEditorInput): boolean { + return this.matches(this.active, candidate); } get previewEditor(): EditorInput | null { @@ -311,7 +308,7 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel { const makeSticky = options?.sticky || (typeof options?.index === 'number' && this.isSticky(options.index)); const makePinned = options?.pinned || options?.sticky; const makeTransient = !!options?.transient; - const makeActive = options?.active || !this.activeEditor || (!makePinned && this.matches(this.preview, this.activeEditor)); + const makeActive = options?.active || !this.activeEditor || (!makePinned && this.preview === this.activeEditor); const existingEditorAndIndex = this.findEditor(candidate, options); @@ -413,7 +410,7 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel { }; this._onDidModelChange.fire(event); - // Handle active & selection + // Handle active editor / selected editors this.setSelection(makeActive ? newEditor : this.activeEditor, options?.inactiveSelection ?? []); return { @@ -434,7 +431,7 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel { this.doPin(existingEditor, existingEditorIndex); } - // Activate / select it + // Handle active editor / selected editors this.setSelection(makeActive ? existingEditor : this.activeEditor, options?.inactiveSelection ?? []); // Respect index @@ -553,8 +550,8 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel { const editor = this.editors[index]; const sticky = this.isSticky(index); - // Active Editor closed - const isActiveEditor = this.matches(this.active, editor); + // Active editor closed + const isActiveEditor = this.active === editor; if (openNext && isActiveEditor) { // More than one editor @@ -570,27 +567,29 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel { } } - const newSelection = this.selection.filter(selected => !selected.matches(newActive) && !selected.matches(editor)); - this.doSetSelection(newActive, this.editors.indexOf(newActive), newSelection); + // Select editor as active + const newInactiveSelectedEditors = this.selection.filter(selected => selected !== editor && selected !== newActive); + this.doSetSelection(newActive, this.editors.indexOf(newActive), newInactiveSelectedEditors); } - // One Editor + // Last editor closed: clear selection else { - this.active = null; + this.doSetSelection(null, undefined, []); } } - // Remove from selection + // Inactive editor closed else if (!isActiveEditor) { - const wasSelected = !!this.selection.find(selected => this.matches(selected, editor)); - if (wasSelected) { - const newSelection = this.selection.filter(selected => !selected.matches(editor)); - this.doSetSelection(this.activeEditor!, this.indexOf(this.activeEditor), newSelection); + + // Remove editor from inactive selection + if (this.doIsSelected(editor)) { + const newInactiveSelectedEditors = this.selection.filter(selected => selected !== editor && selected !== this.activeEditor); + this.doSetSelection(this.activeEditor, this.indexOf(this.activeEditor), newInactiveSelectedEditors); } } // Preview Editor closed - if (this.matches(this.preview, editor)) { + if (this.preview === editor) { this.preview = null; } @@ -685,72 +684,99 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel { const [editor, editorIndex] = res; - this.doSetActive(editor, editorIndex); + this.doSetSelection(editor, editorIndex, []); return editor; } - private doSetActive(editor: EditorInput, editorIndex: number): void { - if (this.matches(this.active, editor)) { - this.selection = [editor]; - return; // already active - } - - this.active = editor; + get selectedEditors(): EditorInput[] { + return this.editors.filter(editor => this.doIsSelected(editor)); // return in sequential order + } - // Bring to front in MRU list - const mruIndex = this.indexOf(editor, this.mru); - this.mru.splice(mruIndex, 1); - this.mru.unshift(editor); + isSelected(editorCandidateOrIndex: EditorInput | number): boolean { + let editor: EditorInput | undefined; + if (typeof editorCandidateOrIndex === 'number') { + editor = this.editors[editorCandidateOrIndex]; + } else { + editor = this.findEditor(editorCandidateOrIndex)?.[0]; + } - // Event - const event: IGroupEditorChangeEvent = { - kind: GroupModelChangeKind.EDITOR_ACTIVE, - editor, - editorIndex - }; - this._onDidModelChange.fire(event); + return !!editor && this.doIsSelected(editor); } - public get selectedEditors(): EditorInput[] { - // Return selected editors in sequential order - return this.editors.filter(editor => this.isSelected(editor)); + private doIsSelected(editor: EditorInput): boolean { + return this.selection.includes(editor); } - isSelected(editor: EditorInput | number): boolean { - if (typeof editor === 'number') { - editor = this.editors[editor]; + setSelection(activeSelectedEditorCandidate: EditorInput, inactiveSelectedEditorCandidates: EditorInput[]): void { + const res = this.findEditor(activeSelectedEditorCandidate); + if (!res) { + return; // not found } - return !!this.selection.find(selectedEditor => this.matches(selectedEditor, editor)); + const [activeSelectedEditor, activeSelectedEditorIndex] = res; + + const inactiveSelectedEditors = new Set(); + for (const inactiveSelectedEditorCandidate of inactiveSelectedEditorCandidates) { + const res = this.findEditor(inactiveSelectedEditorCandidate); + if (!res) { + return; // not found + } + + const [inactiveSelectedEditor] = res; + if (inactiveSelectedEditor === activeSelectedEditor) { + continue; // already selected + } + + inactiveSelectedEditors.add(inactiveSelectedEditor); + } + + this.doSetSelection(activeSelectedEditor, activeSelectedEditorIndex, Array.from(inactiveSelectedEditors)); } - setSelection(activeSelectedEditor: EditorInput, inactiveSelectedEditors: EditorInput[]): void { - const res = this.findEditor(activeSelectedEditor); - if (!res) { - return; + private doSetSelection(activeSelectedEditor: EditorInput | null, activeSelectedEditorIndex: number | undefined, inactiveSelectedEditors: EditorInput[]): void { + const previousActiveEditor = this.activeEditor; + const previousSelection = this.selection; + + let newSelection: EditorInput[]; + if (activeSelectedEditor) { + newSelection = [activeSelectedEditor, ...inactiveSelectedEditors]; + } else { + newSelection = []; } - const [newActiveEditor, newActiveEditorIndex] = res; + // Update selection + this.selection = newSelection; - this.doSetSelection(newActiveEditor, newActiveEditorIndex, inactiveSelectedEditors); - } + // Update active editor if it has changed + const activeEditorChanged = activeSelectedEditor && typeof activeSelectedEditorIndex === 'number' && previousActiveEditor !== activeSelectedEditor; + if (activeEditorChanged) { - private doSetSelection(newActiveEditor: EditorInput, activeEditorIndex: number, inactiveSelectedEditors: EditorInput[]): void { - this.doSetActive(newActiveEditor, activeEditorIndex); + // Bring to front in MRU list + const mruIndex = this.indexOf(activeSelectedEditor, this.mru); + this.mru.splice(mruIndex, 1); + this.mru.unshift(activeSelectedEditor); - for (const candidate of inactiveSelectedEditors) { - const editor = this.findEditor(candidate)?.[0]; - if (editor && !this.isSelected(editor)) { - this.selection.push(editor); - } + // Event + const event: IGroupEditorChangeEvent = { + kind: GroupModelChangeKind.EDITOR_ACTIVE, + editor: activeSelectedEditor, + editorIndex: activeSelectedEditorIndex + }; + this._onDidModelChange.fire(event); } - // Event - const event: IGroupModelChangeEvent = { - kind: GroupModelChangeKind.EDITORS_SELECTION, - }; - this._onDidModelChange.fire(event); + // Fire event if the selection has changed + if ( + activeEditorChanged || + previousSelection.length !== newSelection.length || + previousSelection.some(editor => !newSelection.includes(editor)) + ) { + const event: IGroupModelChangeEvent = { + kind: GroupModelChangeKind.EDITORS_SELECTION + }; + this._onDidModelChange.fire(event); + } } setIndex(index: number) { @@ -838,12 +864,12 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel { } } - isPinned(editorOrIndex: EditorInput | number): boolean { + isPinned(editorCandidateOrIndex: EditorInput | number): boolean { let editor: EditorInput; - if (typeof editorOrIndex === 'number') { - editor = this.editors[editorOrIndex]; + if (typeof editorCandidateOrIndex === 'number') { + editor = this.editors[editorCandidateOrIndex]; } else { - editor = editorOrIndex; + editor = editorCandidateOrIndex; } return !this.matches(this.preview, editor); @@ -980,16 +1006,16 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel { this._onDidModelChange.fire(event); } - isTransient(editorOrIndex: EditorInput | number): boolean { + isTransient(editorCandidateOrIndex: EditorInput | number): boolean { if (this.transient.size === 0) { return false; // no transient editor } let editor: EditorInput | undefined; - if (typeof editorOrIndex === 'number') { - editor = this.editors[editorOrIndex]; + if (typeof editorCandidateOrIndex === 'number') { + editor = this.editors[editorCandidateOrIndex]; } else { - editor = this.findEditor(editorOrIndex)?.[0]; + editor = this.findEditor(editorCandidateOrIndex)?.[0]; } return !!editor && this.transient.has(editor); @@ -1140,7 +1166,7 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel { clone.editors = this.editors.slice(0); clone.mru = this.mru.slice(0); clone.preview = this.preview; - clone.active = this.active; + clone.selection = this.selection.slice(0); clone.sticky = this.sticky; // Ensure to register listeners for each editor @@ -1242,7 +1268,7 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel { this.mru = coalesce(data.mru.map(i => this.editors[i])); - this.active = this.mru[0]; + this.selection = this.mru.length > 0 ? [this.mru[0]] : []; if (typeof data.preview === 'number') { this.preview = this.editors[data.preview]; diff --git a/src/vs/workbench/common/editor/filteredEditorGroupModel.ts b/src/vs/workbench/common/editor/filteredEditorGroupModel.ts index fcda0bbed00a1..61d4f6a7c8009 100644 --- a/src/vs/workbench/common/editor/filteredEditorGroupModel.ts +++ b/src/vs/workbench/common/editor/filteredEditorGroupModel.ts @@ -42,7 +42,7 @@ abstract class FilteredEditorGroupModel extends Disposable implements IReadonlyE isTransient(editorOrIndex: EditorInput | number): boolean { return this.model.isTransient(editorOrIndex); } isSticky(editorOrIndex: EditorInput | number): boolean { return this.model.isSticky(editorOrIndex); } isActive(editor: EditorInput | IUntypedEditorInput): boolean { return this.model.isActive(editor); } - isSelected(editor: EditorInput | number): boolean { return this.model.isSelected(editor); } + isSelected(editorOrIndex: EditorInput | number): boolean { return this.model.isSelected(editorOrIndex); } isFirst(editor: EditorInput): boolean { return this.model.isFirst(editor, this.getEditors(EditorsOrder.SEQUENTIAL));