Skip to content

Commit

Permalink
editors - some 💄 from code review (#212886)
Browse files Browse the repository at this point in the history
  • Loading branch information
bpasero authored May 16, 2024
1 parent 462a2a7 commit bb68461
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 32 deletions.
8 changes: 3 additions & 5 deletions src/vs/workbench/browser/parts/editor/editorCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -902,11 +902,10 @@ function registerCloseEditorCommands() {

for (const { editor, group } of editorsAndGroup) {
const untypedEditor = editor.toUntyped();

// Resolver can only resolve untyped editors
if (!untypedEditor) {
return;
return; // Resolver can only resolve untyped editors
}

untypedEditor.options = { ...editorService.activeEditorPane?.options, override: EditorResolution.PICK };
const resolvedEditor = await editorResolverService.resolveEditor(untypedEditor, group);
if (!isEditorInputWithOptionsAndGroup(resolvedEditor)) {
Expand All @@ -925,7 +924,6 @@ function registerCloseEditorCommands() {
});

// Telemetry

type WorkbenchEditorReopenClassification = {
owner: 'rebornix';
comment: 'Identify how a document is reopened';
Expand Down Expand Up @@ -1403,7 +1401,7 @@ export function getEditorsFromContext(accessor: ServicesAccessor, resourceOrCont
if (!editor || !group) {
return undefined;
}
return { editor: editor, group: group };
return { editor, group };
});

return editorsAndGroup.filter(group => !!group);
Expand Down
16 changes: 8 additions & 8 deletions src/vs/workbench/browser/parts/editor/editorDropTarget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ class DropOverlay extends Themable {
isCopy = this.isCopyOperation(e);
} else if (isDraggingEditor) {
const data = this.editorTransfer.getData(DraggedEditorIdentifier.prototype);
if (Array.isArray(data)) {
if (Array.isArray(data) && data.length > 0) {
isCopy = this.isCopyOperation(e, data[0].identifier);
}
}
Expand Down Expand Up @@ -234,15 +234,15 @@ class DropOverlay extends Themable {
// Check for group transfer
if (this.groupTransfer.hasData(DraggedEditorGroupIdentifier.prototype)) {
const data = this.groupTransfer.getData(DraggedEditorGroupIdentifier.prototype);
if (Array.isArray(data)) {
if (Array.isArray(data) && data.length > 0) {
return this.editorGroupService.getGroup(data[0].identifier);
}
}

// Check for editor transfer
else if (this.editorTransfer.hasData(DraggedEditorIdentifier.prototype)) {
const data = this.editorTransfer.getData(DraggedEditorIdentifier.prototype);
if (Array.isArray(data)) {
if (Array.isArray(data) && data.length > 0) {
return this.editorGroupService.getGroup(data[0].identifier.groupId);
}
}
Expand All @@ -267,7 +267,7 @@ class DropOverlay extends Themable {
// Check for group transfer
if (this.groupTransfer.hasData(DraggedEditorGroupIdentifier.prototype)) {
const data = this.groupTransfer.getData(DraggedEditorGroupIdentifier.prototype);
if (Array.isArray(data)) {
if (Array.isArray(data) && data.length > 0) {
const sourceGroup = this.editorGroupService.getGroup(data[0].identifier);
if (sourceGroup) {
if (typeof splitDirection !== 'number' && sourceGroup === this.groupView) {
Expand Down Expand Up @@ -306,7 +306,7 @@ class DropOverlay extends Themable {
// Check for editor transfer
else if (this.editorTransfer.hasData(DraggedEditorIdentifier.prototype)) {
const data = this.editorTransfer.getData(DraggedEditorIdentifier.prototype);
if (Array.isArray(data)) {
if (Array.isArray(data) && data.length > 0) {
const draggedEditors = data;
const firstDraggedEditor = data[0].identifier;

Expand All @@ -333,8 +333,8 @@ class DropOverlay extends Themable {
{
editor: draggedEditor.identifier.editor,
options: fillActiveEditorViewState(sourceGroup, draggedEditor.identifier.editor, {
pinned: true, // always pin dropped editor
sticky: sourceGroup.isSticky(firstDraggedEditor.editor), // preserve sticky state
pinned: true, // always pin dropped editor
sticky: sourceGroup.isSticky(firstDraggedEditor.editor) // preserve sticky state
})
}
));
Expand All @@ -357,7 +357,7 @@ class DropOverlay extends Themable {
// Check for tree items
else if (this.treeItemsTransfer.hasData(DraggedTreeItemsIdentifier.prototype)) {
const data = this.treeItemsTransfer.getData(DraggedTreeItemsIdentifier.prototype);
if (Array.isArray(data)) {
if (Array.isArray(data) && data.length > 0) {
const editors: IUntypedEditorInput[] = [];
for (const id of data) {
const dataTransferItem = await this.treeViewsDragAndDropService.removeDragOperationTransfer(id.identifier);
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/browser/parts/editor/editorTabsControl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export interface IEditorTabsControl extends IDisposable {
stickEditor(editor: EditorInput): void;
unstickEditor(editor: EditorInput): void;
setActive(isActive: boolean): void;
setEditorSelections(editor: EditorInput[], selected: boolean): void;
setEditorSelections(editors: EditorInput[], selected: boolean): void;
updateEditorLabel(editor: EditorInput): void;
updateEditorDirty(editor: EditorInput): void;
layout(dimensions: IEditorTitleControlDimensions): Dimension;
Expand Down Expand Up @@ -503,7 +503,7 @@ export abstract class EditorTabsControl extends Themable implements IEditorTabsC

abstract setActive(isActive: boolean): void;

abstract setEditorSelections(editor: EditorInput[], selected: boolean): void;
abstract setEditorSelections(editors: EditorInput[], selected: boolean): void;

abstract updateEditorLabel(editor: EditorInput): void;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1535,7 +1535,7 @@ export class MultiEditorTabsControl extends EditorTabsControl {
// Borders / outline
this.redrawTabBorders(tabIndex, tabContainer);

// Active / dirty state
// Selection / active / dirty state
this.redrawTabSelectedActiveAndDirty(this.groupsView.activeGroup === this.groupView, editor, tabContainer, tabActionBar);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export class NoEditorTabsControl extends EditorTabsControl {

setActive(isActive: boolean): void { }

setEditorSelections(editor: EditorInput[], selected: boolean): void { }
setEditorSelections(editors: EditorInput[], selected: boolean): void { }

updateEditorLabel(editor: EditorInput): void { }

Expand Down
10 changes: 3 additions & 7 deletions src/vs/workbench/browser/parts/editor/singleEditorTabsControl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,19 +179,15 @@ export class SingleEditorTabsControl extends EditorTabsControl {
this.ifEditorIsActive(editor, () => this.redraw());
}

stickEditor(editor: EditorInput): void {
// Sticky editors are not presented any different with tabs disabled
}
stickEditor(editor: EditorInput): void { }

unstickEditor(editor: EditorInput): void {
// Sticky editors are not presented any different with tabs disabled
}
unstickEditor(editor: EditorInput): void { }

setActive(isActive: boolean): void {
this.redraw();
}

setEditorSelections(editor: EditorInput[], selected: boolean): void { }
setEditorSelections(editors: EditorInput[], selected: boolean): void { }

updateEditorLabel(editor: EditorInput): void {
this.ifEditorIsActive(editor, () => this.redraw());
Expand Down
14 changes: 8 additions & 6 deletions src/vs/workbench/common/editor/editorGroupModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,17 +221,18 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel {

private locked = false;

private selected: EditorInput[] = []; // editors in selected state, first one is active
private selected: EditorInput[] = []; // editors in selected state, first one is active

private set active(editor: EditorInput | null) {
this.selected = editor ? [editor] : [];
}
private get active(): EditorInput | null {
return this.selected[0] || null;
return this.selected[0] ?? null;
}

private preview: EditorInput | null = null; // editor in preview state
private sticky = -1; // index of first editor in sticky state
private readonly transient = new Set<EditorInput>(); // editors in transient state
private preview: EditorInput | null = null; // editor in preview state
private sticky = -1; // index of first editor in sticky state
private readonly transient = new Set<EditorInput>(); // editors in transient state

private editorOpenPositioning: ('left' | 'right' | 'first' | 'last') | undefined;
private focusRecentEditorAfterClose: boolean | undefined;
Expand Down Expand Up @@ -582,6 +583,7 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel {
this.active = null;
}
}

// Remove from selection
else if (!isActiveEditor) {
const wasSelected = !!this.selected.find(selected => this.matches(selected, editor));
Expand Down Expand Up @@ -729,7 +731,7 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel {
return this.editors.filter(editor => this.isSelected(editor));
}

isSelected(editor: number | EditorInput): boolean {
isSelected(editor: EditorInput | number): boolean {
if (typeof editor === 'number') {
editor = this.editors[editor];
}
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/common/editor/filteredEditorGroupModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: number | EditorInput): boolean { return this.model.isSelected(editor); }
isSelected(editor: EditorInput | number): boolean { return this.model.isSelected(editor); }

isFirst(editor: EditorInput): boolean {
return this.model.isFirst(editor, this.getEditors(EditorsOrder.SEQUENTIAL));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,6 @@ export interface IEditorGroup {
*/
selectEditor(editor: EditorInput, active?: boolean): Promise<void>;


/**
* Selects the editors in the group. If activeEditor is provided,
* it will be the active editor in the group.
Expand Down

0 comments on commit bb68461

Please sign in to comment.