From fdf29107ad6598c0db815998b33ea4562a7e5c7b Mon Sep 17 00:00:00 2001 From: isidor Date: Fri, 28 Aug 2020 18:32:57 +0200 Subject: [PATCH] repl: simplify filter work --- .../workbench/contrib/debug/browser/repl.ts | 26 ++--- .../contrib/debug/browser/replFilter.ts | 103 ++++-------------- .../contrib/debug/common/replModel.ts | 11 +- .../contrib/debug/test/browser/repl.test.ts | 25 +++-- .../contrib/markers/browser/messages.ts | 2 +- 5 files changed, 46 insertions(+), 121 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/repl.ts b/src/vs/workbench/contrib/debug/browser/repl.ts index 14d45b12304..7aa395b1c9b 100644 --- a/src/vs/workbench/contrib/debug/browser/repl.ts +++ b/src/vs/workbench/contrib/debug/browser/repl.ts @@ -59,7 +59,7 @@ import { ReplGroup } from 'vs/workbench/contrib/debug/common/replModel'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; import { EDITOR_FONT_DEFAULTS, EditorOption } from 'vs/editor/common/config/editorOptions'; import { MOUSE_CURSOR_TEXT_CSS_CLASS_NAME } from 'vs/base/browser/ui/mouseCursor/mouseCursor'; -import { ReplFilter, TreeFilterState, TreeFilterPanelActionViewItem } from 'vs/workbench/contrib/debug/browser/replFilter'; +import { ReplFilter, ReplFilterState, ReplFilterActionViewItem } from 'vs/workbench/contrib/debug/browser/replFilter'; const $ = dom.$; @@ -95,8 +95,8 @@ export class Repl extends ViewPane implements IHistoryNavigationWidget { private completionItemProvider: IDisposable | undefined; private modelChangeListener: IDisposable = Disposable.None; private filter: ReplFilter; - private filterState: TreeFilterState; - private filterActionViewItem: TreeFilterPanelActionViewItem | undefined; + private filterState: ReplFilterState; + private filterActionViewItem: ReplFilterActionViewItem | undefined; constructor( options: IViewPaneOptions, @@ -121,11 +121,7 @@ export class Repl extends ViewPane implements IHistoryNavigationWidget { this.history = new HistoryNavigator(JSON.parse(this.storageService.get(HISTORY_STORAGE_KEY, StorageScope.WORKSPACE, '[]')), 50); this.filter = new ReplFilter(); - this.filterState = this._register(new TreeFilterState({ - filterText: '', - filterHistory: [], - layout: new dom.Dimension(0, 0), - })); + this.filterState = new ReplFilterState(); codeEditorService.registerDecorationType(DECORATION_KEY, {}); this.registerListeners(); @@ -249,13 +245,10 @@ export class Repl extends ViewPane implements IHistoryNavigationWidget { this.setMode(); })); - this._register(this.filterState.onDidChange((e) => { - if (e.filterText) { - this.filter.filterQuery = this.filterState.filterText; - if (this.tree) { - this.tree.refilter(); - } - } + this._register(this.filterState.onDidChange(() => { + this.filter.filterQuery = this.filterState.filterText; + this.tree.refilter(); + revealLastElement(this.tree); })); } @@ -448,7 +441,6 @@ export class Repl extends ViewPane implements IHistoryNavigationWidget { this.replInputContainer.style.height = `${replInputHeight}px`; this.replInput.layout({ width: width - 30, height: replInputHeight }); - this.filterState.layout = new dom.Dimension(width, height); } focus(): void { @@ -459,7 +451,7 @@ export class Repl extends ViewPane implements IHistoryNavigationWidget { if (action.id === SelectReplAction.ID) { return this.instantiationService.createInstance(SelectReplActionViewItem, this.selectReplAction); } else if (action.id === FILTER_ACTION_ID) { - this.filterActionViewItem = this.instantiationService.createInstance(TreeFilterPanelActionViewItem, action, localize('workbench.debug.filter.placeholder', "Filter. E.g.: text, !exclude"), this.filterState); + this.filterActionViewItem = this.instantiationService.createInstance(ReplFilterActionViewItem, action, localize('workbench.debug.filter.placeholder', "Filter (e.g. text, !exclude)"), this.filterState); return this.filterActionViewItem; } diff --git a/src/vs/workbench/contrib/debug/browser/replFilter.ts b/src/vs/workbench/contrib/debug/browser/replFilter.ts index e5675cf2bab..036e0309564 100644 --- a/src/vs/workbench/contrib/debug/browser/replFilter.ts +++ b/src/vs/workbench/contrib/debug/browser/replFilter.ts @@ -15,7 +15,7 @@ import { IAction } from 'vs/base/common/actions'; import { HistoryInputBox } from 'vs/base/browser/ui/inputbox/inputBox'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IContextViewService } from 'vs/platform/contextview/browser/contextView'; -import { toDisposable, Disposable } from 'vs/base/common/lifecycle'; +import { toDisposable } from 'vs/base/common/lifecycle'; import { Event, Emitter } from 'vs/base/common/event'; import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent'; import { KeyCode } from 'vs/base/common/keyCodes'; @@ -76,64 +76,37 @@ export class ReplFilter implements ITreeFilter { } } -export interface IReplFiltersChangeEvent { - filterText?: boolean; - layout?: boolean; -} - -export interface IReplFiltersOptions { - filterText: string; - filterHistory: string[]; - layout: DOM.Dimension; -} - -export class TreeFilterState extends Disposable { +export class ReplFilterState { - private readonly _onDidChange: Emitter = this._register(new Emitter()); - readonly onDidChange: Event = this._onDidChange.event; - - constructor(options: IReplFiltersOptions) { - super(); - this._filterText = options.filterText; - this.filterHistory = options.filterHistory; - this._layout = options.layout; + private readonly _onDidChange: Emitter = new Emitter(); + get onDidChange(): Event { + return this._onDidChange.event; } - private _filterText: string; + private _filterText = ''; + get filterText(): string { return this._filterText; } + set filterText(filterText: string) { if (this._filterText !== filterText) { this._filterText = filterText; - this._onDidChange.fire({ filterText: true }); - } - } - - filterHistory: string[]; - - private _layout: DOM.Dimension = new DOM.Dimension(0, 0); - get layout(): DOM.Dimension { - return this._layout; - } - set layout(layout: DOM.Dimension) { - if (this._layout.width !== layout.width || this._layout.height !== layout.height) { - this._layout = layout; - this._onDidChange.fire({ layout: true }); + this._onDidChange.fire(); } } } -export class TreeFilterPanelActionViewItem extends BaseActionViewItem { +export class ReplFilterActionViewItem extends BaseActionViewItem { private delayedFilterUpdate: Delayer; - private container: HTMLElement | undefined; - private filterInputBox: HistoryInputBox | undefined; + private container!: HTMLElement; + private filterInputBox!: HistoryInputBox; constructor( action: IAction, private placeholder: string, - private filters: TreeFilterState, + private filters: ReplFilterState, @IInstantiationService private readonly instantiationService: IInstantiationService, @IThemeService private readonly themeService: IThemeService, @IContextViewService private readonly contextViewService: IContextViewService) { @@ -150,34 +123,28 @@ export class TreeFilterPanelActionViewItem extends BaseActionViewItem { this.element.className = this.class; this.createInput(this.element); this.updateClass(); - - this.adjustInputBox(); + this.filterInputBox.inputElement.style.paddingRight = DOM.hasClass(this.element, 'small') ? '25px' : '150px'; } focus(): void { - if (this.filterInputBox) { - this.filterInputBox.focus(); - } + this.filterInputBox.focus(); } private clearFilterText(): void { - if (this.filterInputBox) { - this.filterInputBox.value = ''; - } + this.filterInputBox.value = ''; } private createInput(container: HTMLElement): void { this.filterInputBox = this._register(this.instantiationService.createInstance(ContextScopedHistoryInputBox, container, this.contextViewService, { placeholder: this.placeholder, - history: this.filters.filterHistory + history: [] })); this._register(attachInputBoxStyler(this.filterInputBox, this.themeService)); this.filterInputBox.value = this.filters.filterText; + this._register(this.filterInputBox.onDidChange(() => this.delayedFilterUpdate.trigger(() => this.onDidInputChange(this.filterInputBox!)))); - this._register(this.filters.onDidChange((event: IReplFiltersChangeEvent) => { - if (event.filterText) { - this.filterInputBox!.value = this.filters.filterText; - } + this._register(this.filters.onDidChange(() => { + this.filterInputBox.value = this.filters.filterText; })); this._register(DOM.addStandardDisposableListener(this.filterInputBox.inputElement, DOM.EventType.KEY_DOWN, (e: any) => this.onInputKeyDown(e))); this._register(DOM.addStandardDisposableListener(container, DOM.EventType.KEY_DOWN, this.handleKeyboardEvent)); @@ -186,19 +153,11 @@ export class TreeFilterPanelActionViewItem extends BaseActionViewItem { e.stopPropagation(); e.preventDefault(); })); - this._register(this.filters.onDidChange(e => this.onDidFiltersChange(e))); - } - - private onDidFiltersChange(e: IReplFiltersChangeEvent): void { - if (e.layout) { - this.updateClass(); - } } private onDidInputChange(inputbox: HistoryInputBox) { inputbox.addToHistory(); this.filters.filterText = inputbox.value; - this.filters.filterHistory = inputbox.getHistory(); } // Action toolbar is swallowing some keys for action items which should not be for an input box @@ -220,27 +179,7 @@ export class TreeFilterPanelActionViewItem extends BaseActionViewItem { } } - private adjustInputBox(): void { - if (this.element && this.filterInputBox) { - this.filterInputBox.inputElement.style.paddingRight = DOM.hasClass(this.element, 'small') ? '25px' : '150px'; - } - } - - protected updateClass(): void { - if (this.element && this.container) { - this.element.className = this.class; - DOM.toggleClass(this.container, 'grow', DOM.hasClass(this.element, 'grow')); - this.adjustInputBox(); - } - } - protected get class(): string { - if (this.filters.layout.width > 800) { - return 'panel-action-tree-filter grow'; - } else if (this.filters.layout.width < 600) { - return 'panel-action-tree-filter small'; - } else { - return 'panel-action-tree-filter'; - } + return 'panel-action-tree-filter'; } } diff --git a/src/vs/workbench/contrib/debug/common/replModel.ts b/src/vs/workbench/contrib/debug/common/replModel.ts index b0aa0128449..87d3348b97c 100644 --- a/src/vs/workbench/contrib/debug/common/replModel.ts +++ b/src/vs/workbench/contrib/debug/common/replModel.ts @@ -174,18 +174,13 @@ export class ReplGroup implements IReplElement { } } -type FilterFunc = ((element: IReplElement) => void); - export class ReplModel { private replElements: IReplElement[] = []; private readonly _onDidChangeElements = new Emitter(); readonly onDidChangeElements = this._onDidChangeElements.event; - private filterFunc: FilterFunc | undefined; getReplElements(): IReplElement[] { - return this.replElements.filter(element => - this.filterFunc ? this.filterFunc(element) : true - ); + return this.replElements; } async addReplExpression(session: IDebugSession, stackFrame: IStackFrame | undefined, name: string): Promise { @@ -320,10 +315,6 @@ export class ReplModel { } } - setFilter(filterFunc: FilterFunc): void { - this.filterFunc = filterFunc; - } - removeReplExpressions(): void { if (this.replElements.length > 0) { this.replElements = []; diff --git a/src/vs/workbench/contrib/debug/test/browser/repl.test.ts b/src/vs/workbench/contrib/debug/test/browser/repl.test.ts index 7440c4df8b8..787328d4e5d 100644 --- a/src/vs/workbench/contrib/debug/test/browser/repl.test.ts +++ b/src/vs/workbench/contrib/debug/test/browser/repl.test.ts @@ -197,10 +197,13 @@ suite('Debug - REPL', () => { const repl = new ReplModel(); const replFilter = new ReplFilter(); - repl.setFilter((element) => { - const filterResult = replFilter.filter(element, TreeVisibility.Visible); - return filterResult === true || filterResult === TreeVisibility.Visible; - }); + const getFilteredElements = () => { + const elements = repl.getReplElements(); + return elements.filter(e => { + const filterResult = replFilter.filter(e, TreeVisibility.Visible); + return filterResult === true || filterResult === TreeVisibility.Visible; + }); + }; repl.appendToRepl(session, 'first line\n', severity.Info); repl.appendToRepl(session, 'second line\n', severity.Info); @@ -208,19 +211,19 @@ suite('Debug - REPL', () => { repl.appendToRepl(session, 'fourth line\n', severity.Info); replFilter.filterQuery = 'first'; - let r1 = repl.getReplElements(); + let r1 = getFilteredElements(); assert.equal(r1.length, 1); assert.equal(r1[0].value, 'first line\n'); replFilter.filterQuery = '!first'; - let r2 = repl.getReplElements(); + let r2 = getFilteredElements(); assert.equal(r1.length, 1); assert.equal(r2[0].value, 'second line\n'); assert.equal(r2[1].value, 'third line\n'); assert.equal(r2[2].value, 'fourth line\n'); replFilter.filterQuery = 'first, line'; - let r3 = repl.getReplElements(); + let r3 = getFilteredElements(); assert.equal(r3.length, 4); assert.equal(r3[0].value, 'first line\n'); assert.equal(r3[1].value, 'second line\n'); @@ -228,22 +231,22 @@ suite('Debug - REPL', () => { assert.equal(r3[3].value, 'fourth line\n'); replFilter.filterQuery = 'line, !second'; - let r4 = repl.getReplElements(); + let r4 = getFilteredElements(); assert.equal(r4.length, 3); assert.equal(r4[0].value, 'first line\n'); assert.equal(r4[1].value, 'third line\n'); assert.equal(r4[2].value, 'fourth line\n'); replFilter.filterQuery = '!second, line'; - let r4_same = repl.getReplElements(); + let r4_same = getFilteredElements(); assert.equal(r4.length, r4_same.length); replFilter.filterQuery = '!line'; - let r5 = repl.getReplElements(); + let r5 = getFilteredElements(); assert.equal(r5.length, 0); replFilter.filterQuery = 'smth'; - let r6 = repl.getReplElements(); + let r6 = getFilteredElements(); assert.equal(r6.length, 0); }); }); diff --git a/src/vs/workbench/contrib/markers/browser/messages.ts b/src/vs/workbench/contrib/markers/browser/messages.ts index 4fbffb9d7d4..4652f48e7f1 100644 --- a/src/vs/workbench/contrib/markers/browser/messages.ts +++ b/src/vs/workbench/contrib/markers/browser/messages.ts @@ -33,7 +33,7 @@ export default class Messages { public static MARKERS_PANEL_ACTION_TOOLTIP_FILTER: string = nls.localize('markers.panel.action.filter', "Filter Problems"); public static MARKERS_PANEL_ACTION_TOOLTIP_QUICKFIX: string = nls.localize('markers.panel.action.quickfix', "Show fixes"); public static MARKERS_PANEL_FILTER_ARIA_LABEL: string = nls.localize('markers.panel.filter.ariaLabel', "Filter Problems"); - public static MARKERS_PANEL_FILTER_PLACEHOLDER: string = nls.localize('markers.panel.filter.placeholder', "Filter. E.g.: text, **/*.ts, !**/node_modules/**"); + public static MARKERS_PANEL_FILTER_PLACEHOLDER: string = nls.localize('markers.panel.filter.placeholder', "Filter (e.g. text, **/*.ts, !**/node_modules/**)"); public static MARKERS_PANEL_FILTER_ERRORS: string = nls.localize('markers.panel.filter.errors', "errors"); public static MARKERS_PANEL_FILTER_WARNINGS: string = nls.localize('markers.panel.filter.warnings', "warnings"); public static MARKERS_PANEL_FILTER_INFOS: string = nls.localize('markers.panel.filter.infos', "infos"); -- GitLab