From c87f59c8d05f174b5714b6f7824fb238874bbccd Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 14 Nov 2017 22:09:20 +0100 Subject: [PATCH 01/19] debug: simplify reacting to breakpoint decoration events fixes #20601 --- .../debug/browser/debugEditorModelManager.ts | 15 +++++---------- src/vs/workbench/parts/debug/common/debug.ts | 2 +- src/vs/workbench/parts/debug/common/debugModel.ts | 6 ++---- .../parts/debug/electron-browser/debugService.ts | 7 ++----- 4 files changed, 10 insertions(+), 20 deletions(-) diff --git a/src/vs/workbench/parts/debug/browser/debugEditorModelManager.ts b/src/vs/workbench/parts/debug/browser/debugEditorModelManager.ts index debfbe42761..58d8b202867 100644 --- a/src/vs/workbench/parts/debug/browser/debugEditorModelManager.ts +++ b/src/vs/workbench/parts/debug/browser/debugEditorModelManager.ts @@ -4,7 +4,6 @@ *--------------------------------------------------------------------------------------------*/ import * as nls from 'vs/nls'; -import { TPromise } from 'vs/base/common/winjs.base'; import * as objects from 'vs/base/common/objects'; import * as lifecycle from 'vs/base/common/lifecycle'; import { Constants } from 'vs/editor/common/core/uint'; @@ -197,7 +196,7 @@ export class DebugEditorModelManager implements IWorkbenchContribution { return; } const newBreakpointRange = modelData.model.getDecorationRange(breakpointDecoration.decorationId); - if (newBreakpointRange && (breakpointDecoration.range.startColumn !== newBreakpointRange.startColumn || breakpointDecoration.range.endLineNumber !== newBreakpointRange.endLineNumber)) { + if (newBreakpointRange && (!breakpointDecoration.range.equalsRange(newBreakpointRange))) { somethingChanged = true; } }); @@ -209,12 +208,11 @@ export class DebugEditorModelManager implements IWorkbenchContribution { const data: { [id: string]: DebugProtocol.Breakpoint } = Object.create(null); const breakpoints = this.debugService.getModel().getBreakpoints(); const modelUri = modelData.model.uri; - const toRemove: string[] = []; for (let i = 0, len = modelData.breakpointDecorations.length; i < len; i++) { const breakpointDecoration = modelData.breakpointDecorations[i]; const decorationRange = modelData.model.getDecorationRange(breakpointDecoration.decorationId); // check if the line got deleted. - if (decorationRange && decorationRange.endColumn > decorationRange.startColumn) { + if (decorationRange) { const breakpoint = breakpoints.filter(bp => bp.getId() === breakpointDecoration.modelId).pop(); // since we know it is collapsed, it cannot grow to multiple lines if (breakpoint) { @@ -224,15 +222,11 @@ export class DebugEditorModelManager implements IWorkbenchContribution { verified: breakpoint.verified }; } - } else { - toRemove.push(breakpointDecoration.modelId); } } modelData.dirty = this.debugService.state !== State.Inactive; - TPromise.join(toRemove.map(id => this.debugService.removeBreakpoints(id, true))).then(() => { - this.debugService.updateBreakpoints(modelUri, data); - }); + this.debugService.updateBreakpoints(modelUri, data); } private onBreakpointsChange(): void { @@ -274,9 +268,10 @@ export class DebugEditorModelManager implements IWorkbenchContribution { private createBreakpointDecorations(model: IModel, breakpoints: IBreakpoint[]): { range: Range; options: IModelDecorationOptions; }[] { return breakpoints.map((breakpoint) => { + const column = model.getLineFirstNonWhitespaceColumn(breakpoint.lineNumber); const range = model.validateRange( breakpoint.column ? new Range(breakpoint.lineNumber, breakpoint.column, breakpoint.lineNumber, breakpoint.column + 1) - : new Range(breakpoint.lineNumber, 1, breakpoint.lineNumber, Constants.MAX_SAFE_SMALL_INTEGER) // Decoration has to have a width #20688 + : new Range(breakpoint.lineNumber, column, breakpoint.lineNumber, column + 1) // Decoration has to have a width #20688 ); return { options: this.getBreakpointDecorationOptions(breakpoint), diff --git a/src/vs/workbench/parts/debug/common/debug.ts b/src/vs/workbench/parts/debug/common/debug.ts index 9a5da755506..b6c74071cb4 100644 --- a/src/vs/workbench/parts/debug/common/debug.ts +++ b/src/vs/workbench/parts/debug/common/debug.ts @@ -531,7 +531,7 @@ export interface IDebugService { * Removes all breakpoints. If id is passed only removes the breakpoint associated with that id. * Notifies debug adapter of breakpoint changes. */ - removeBreakpoints(id?: string, skipEmit?: boolean): TPromise; + removeBreakpoints(id?: string): TPromise; /** * Adds a new no name function breakpoint. The function breakpoint should be renamed once user enters the name. diff --git a/src/vs/workbench/parts/debug/common/debugModel.ts b/src/vs/workbench/parts/debug/common/debugModel.ts index e3f4233b098..c8ef0226454 100644 --- a/src/vs/workbench/parts/debug/common/debugModel.ts +++ b/src/vs/workbench/parts/debug/common/debugModel.ts @@ -875,11 +875,9 @@ export class Model implements IModel { return newBreakpoints; } - public removeBreakpoints(toRemove: IBreakpoint[], skipEvent?: boolean): void { + public removeBreakpoints(toRemove: IBreakpoint[]): void { this.breakpoints = this.breakpoints.filter(bp => !toRemove.some(toRemove => toRemove.getId() === bp.getId())); - if (!skipEvent) { - this._onDidChangeBreakpoints.fire(); - } + this._onDidChangeBreakpoints.fire(); } public updateBreakpoints(data: { [id: string]: DebugProtocol.Breakpoint }): void { diff --git a/src/vs/workbench/parts/debug/electron-browser/debugService.ts b/src/vs/workbench/parts/debug/electron-browser/debugService.ts index eb6472aae64..96ec6785ce1 100644 --- a/src/vs/workbench/parts/debug/electron-browser/debugService.ts +++ b/src/vs/workbench/parts/debug/electron-browser/debugService.ts @@ -578,15 +578,12 @@ export class DebugService implements debug.IDebugService { return this.sendBreakpoints(uri); } - public removeBreakpoints(id?: string, skipEmit?: boolean): TPromise { + public removeBreakpoints(id?: string): TPromise { const toRemove = this.model.getBreakpoints().filter(bp => !id || bp.getId() === id); toRemove.forEach(bp => aria.status(nls.localize('breakpointRemoved', "Removed breakpoint, line {0}, file {1}", bp.lineNumber, bp.uri.fsPath))); const urisToClear = distinct(toRemove, bp => bp.uri.toString()).map(bp => bp.uri); - this.model.removeBreakpoints(toRemove, skipEmit); - if (skipEmit) { - return TPromise.as(null); - } + this.model.removeBreakpoints(toRemove); return TPromise.join(urisToClear.map(uri => this.sendBreakpoints(uri))); } From 9fd562538b797195dca0e68df1275051f93c47b8 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Tue, 14 Nov 2017 22:37:44 +0100 Subject: [PATCH 02/19] #38329 Always compute most relevant line offset --- .../preferences/common/preferencesModels.ts | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/vs/workbench/parts/preferences/common/preferencesModels.ts b/src/vs/workbench/parts/preferences/common/preferencesModels.ts index 55712af7626..4c650a8c81b 100644 --- a/src/vs/workbench/parts/preferences/common/preferencesModels.ts +++ b/src/vs/workbench/parts/preferences/common/preferencesModels.ts @@ -7,7 +7,7 @@ import * as nls from 'vs/nls'; import { assign } from 'vs/base/common/objects'; import { tail } from 'vs/base/common/arrays'; import URI from 'vs/base/common/uri'; -import { IReference } from 'vs/base/common/lifecycle'; +import { IReference, Disposable } from 'vs/base/common/lifecycle'; import Event, { Emitter } from 'vs/base/common/event'; import { Registry } from 'vs/platform/registry/common/platform'; import { visit, JSONVisitor } from 'vs/base/common/json'; @@ -499,17 +499,20 @@ export class WorkspaceConfigModel extends SettingsEditorModel implements ISettin } } -export class DefaultSettings { - +export class DefaultSettings extends Disposable { private _allSettingsGroups: ISettingsGroup[]; private _content: string; private _settingsByName: Map; + private _onDidChange: Emitter = this._register(new Emitter()); + readonly onDidChange: Event = this._onDidChange.event; + constructor( private _mostCommonlyUsedSettingsKeys: string[], readonly configurationScope: ConfigurationScope, ) { + super(); } public get content(): string { @@ -527,12 +530,16 @@ export class DefaultSettings { } parse(): string { + const currentContent = this._content; const configurations = Registry.as(Extensions.Configuration).getConfigurations().slice(); const settingsGroups = this.removeEmptySettingsGroups(configurations.sort(this.compareConfigurationNodes).reduce((result, config, index, array) => this.parseConfig(config, result, array), [])); this.initAllSettingsMap(settingsGroups); const mostCommonlyUsed = this.getMostCommonlyUsedSettings(settingsGroups); this._allSettingsGroups = [mostCommonlyUsed, ...settingsGroups]; this._content = this.toContent(mostCommonlyUsed, settingsGroups); + if (this._content !== currentContent) { + this._onDidChange.fire(); + } return this._content; } @@ -678,9 +685,6 @@ export class DefaultSettingsEditorModel extends AbstractSettingsModel implements private _model: IModel; private _settingsByName: Map; - private _mostRelevantLineOffset: number; - - private _settingsGroups: ISettingsGroup[]; private _onDidChangeGroups: Emitter = this._register(new Emitter()); readonly onDidChangeGroups: Event = this._onDidChangeGroups.event; @@ -689,20 +693,15 @@ export class DefaultSettingsEditorModel extends AbstractSettingsModel implements private _uri: URI, reference: IReference, readonly configurationScope: ConfigurationScope, - defaultSettings: DefaultSettings + private readonly defaultSettings: DefaultSettings ) { super(); - this._settingsGroups = defaultSettings.settingsGroups; + this._register(defaultSettings.onDidChange(() => this._onDidChangeGroups.fire())); this._model = reference.object.textEditorModel; this._register(this.onDispose(() => reference.dispose())); - this._register(this._model.onDidChangeContent(() => { - this._settingsGroups = defaultSettings.settingsGroups; - this._onDidChangeGroups.fire(); - })); this.initAllSettingsMap(); - this._mostRelevantLineOffset = tail(this.settingsGroups).range.endLineNumber + 2; } public get uri(): URI { @@ -710,7 +709,7 @@ export class DefaultSettingsEditorModel extends AbstractSettingsModel implements } public get settingsGroups(): ISettingsGroup[] { - return this._settingsGroups; + return this.defaultSettings.settingsGroups; } public filterSettings(filter: string, groupFilter: IGroupFilter, settingFilter: ISettingFilter, mostRelevantSettings?: string[]): IFilterResult { @@ -733,7 +732,8 @@ export class DefaultSettingsEditorModel extends AbstractSettingsModel implements } private renderMostRelevantSettings(mostRelevantSettings: string[]): ISettingsGroup { - const builder = new SettingsContentBuilder(this._mostRelevantLineOffset - 1); + const mostRelevantLineOffset = tail(this.settingsGroups).range.endLineNumber + 2; + const builder = new SettingsContentBuilder(mostRelevantLineOffset - 1); builder.pushLine(','); const mostRelevantGroup = this.getMostRelevantSettings(mostRelevantSettings); builder.pushGroups([mostRelevantGroup]); @@ -746,7 +746,7 @@ export class DefaultSettingsEditorModel extends AbstractSettingsModel implements { text: mostRelevantContent, forceMoveMarkers: false, - range: new Range(this._mostRelevantLineOffset, 1, mostRelevantEndLine, 1), + range: new Range(mostRelevantLineOffset, 1, mostRelevantEndLine, 1), identifier: { major: 1, minor: 0 } } ]); From 508614fc9bfa7fefbaf44bb8ca8710974b38e52d Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 14 Nov 2017 22:57:06 +0100 Subject: [PATCH 03/19] debug: option to control when to show debug status fixes #37954 --- .../parts/debug/browser/debugStatus.ts | 52 ++++++++++++++----- .../browser/media/debug.contribution.css | 6 ++- src/vs/workbench/parts/debug/common/debug.ts | 1 + .../electron-browser/debug.contribution.ts | 5 ++ 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/vs/workbench/parts/debug/browser/debugStatus.ts b/src/vs/workbench/parts/debug/browser/debugStatus.ts index 4ed7c675b90..6e2ef96a9d9 100644 --- a/src/vs/workbench/parts/debug/browser/debugStatus.ts +++ b/src/vs/workbench/parts/debug/browser/debugStatus.ts @@ -10,22 +10,25 @@ import { IDisposable, dispose } from 'vs/base/common/lifecycle'; import { IQuickOpenService } from 'vs/platform/quickOpen/common/quickOpen'; import { IThemeService } from 'vs/platform/theme/common/themeService'; import { IStatusbarItem } from 'vs/workbench/browser/parts/statusbar/statusbar'; -import { IDebugService, State } from 'vs/workbench/parts/debug/common/debug'; +import { IDebugService, State, IDebugConfiguration } from 'vs/workbench/parts/debug/common/debug'; import { Themable, STATUS_BAR_FOREGROUND } from 'vs/workbench/common/theme'; +import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; const $ = dom.$; export class DebugStatus extends Themable implements IStatusbarItem { private toDispose: IDisposable[]; private container: HTMLElement; + private statusBarItem: HTMLElement; private label: HTMLElement; private icon: HTMLElement; - private hidden = true; + private showInStatusBar: string; constructor( @IQuickOpenService private quickOpenService: IQuickOpenService, @IDebugService private debugService: IDebugService, - @IThemeService themeService: IThemeService + @IThemeService themeService: IThemeService, + @IConfigurationService configurationService: IConfigurationService ) { super(themeService); this.toDispose = []; @@ -33,9 +36,24 @@ export class DebugStatus extends Themable implements IStatusbarItem { this.setLabel(); })); this.toDispose.push(this.debugService.onDidChangeState(state => { - if (state !== State.Inactive && this.hidden) { - this.hidden = false; - this.render(this.container); + if (state !== State.Inactive && this.showInStatusBar === 'onFirstSessionStart') { + this.doRender(); + } + })); + this.showInStatusBar = configurationService.getValue('debug').showInStatusBar; + this.toDispose.push(configurationService.onDidChangeConfiguration(e => { + if (e.affectsConfiguration('debug.showInStatusBar')) { + this.showInStatusBar = configurationService.getValue('debug').showInStatusBar; + if (this.showInStatusBar === 'never' && this.statusBarItem) { + this.statusBarItem.hidden = true; + } else { + if (this.statusBarItem) { + this.statusBarItem.hidden = false; + } + if (this.showInStatusBar === 'always') { + this.doRender(); + } + } } })); } @@ -49,24 +67,30 @@ export class DebugStatus extends Themable implements IStatusbarItem { public render(container: HTMLElement): IDisposable { this.container = container; - if (!this.hidden) { - const statusBarItem = dom.append(container, $('.debug-statusbar-item')); - this.toDispose.push(dom.addDisposableListener(statusBarItem, 'click', () => { + if (this.showInStatusBar === 'always') { + this.doRender(); + } + // noop, we render when we decide is best + return this; + } + + private doRender(): void { + if (!this.statusBarItem && this.container) { + this.statusBarItem = dom.append(this.container, $('.debug-statusbar-item')); + this.toDispose.push(dom.addDisposableListener(this.statusBarItem, 'click', () => { this.quickOpenService.show('debug ').done(undefined, errors.onUnexpectedError); })); - statusBarItem.title = nls.localize('selectAndStartDebug', "Select and start debug configuration"); - const a = dom.append(statusBarItem, $('a')); + this.statusBarItem.title = nls.localize('selectAndStartDebug', "Select and start debug configuration"); + const a = dom.append(this.statusBarItem, $('a')); this.icon = dom.append(a, $('.icon')); this.label = dom.append(a, $('span.label')); this.setLabel(); this.updateStyles(); } - - return this; } private setLabel(): void { - if (this.label && !this.hidden) { + if (this.label && this.statusBarItem) { const manager = this.debugService.getConfigurationManager(); const name = manager.selectedName || ''; this.label.textContent = manager.getLaunches().length > 1 ? `${name} (${manager.selectedLaunch.workspace.name})` : name; diff --git a/src/vs/workbench/parts/debug/browser/media/debug.contribution.css b/src/vs/workbench/parts/debug/browser/media/debug.contribution.css index 05d23a6ffd0..c0f23e9662f 100644 --- a/src/vs/workbench/parts/debug/browser/media/debug.contribution.css +++ b/src/vs/workbench/parts/debug/browser/media/debug.contribution.css @@ -109,9 +109,13 @@ padding: 0 5px 0 5px; } +.monaco-workbench .part.statusbar .debug-statusbar-item.hidden { + display: none; +} + .monaco-workbench .part.statusbar .debug-statusbar-item .icon { -webkit-mask: url('continue.svg') no-repeat 50% 50%; - -webkit-mask-size: 18px; + -webkit-mask-size: 16px; display: inline-block; padding-right: 2px; width: 16px; diff --git a/src/vs/workbench/parts/debug/common/debug.ts b/src/vs/workbench/parts/debug/common/debug.ts index b6c74071cb4..9e08b4ff90b 100644 --- a/src/vs/workbench/parts/debug/common/debug.ts +++ b/src/vs/workbench/parts/debug/common/debug.ts @@ -323,6 +323,7 @@ export interface IDebugConfiguration { openExplorerOnEnd: boolean; inlineValues: boolean; hideActionBar: boolean; + showInStatusBar: string; internalConsoleOptions: string; } diff --git a/src/vs/workbench/parts/debug/electron-browser/debug.contribution.ts b/src/vs/workbench/parts/debug/electron-browser/debug.contribution.ts index 2203a758fa5..fbce85bb550 100644 --- a/src/vs/workbench/parts/debug/electron-browser/debug.contribution.ts +++ b/src/vs/workbench/parts/debug/electron-browser/debug.contribution.ts @@ -188,6 +188,11 @@ configurationRegistry.registerConfiguration({ description: nls.localize({ comment: ['This is the description for a setting'], key: 'hideActionBar' }, "Controls if the floating debug action bar should be hidden"), default: false }, + 'debug.showInStatusBar': { + enum: ['never', 'always', 'onFirstSessionStart'], + description: nls.localize({ comment: ['This is the description for a setting'], key: 'showInStatusBar' }, "Controls when the debug status bar should be visible"), + default: 'onFirstSessionStart' + }, 'debug.internalConsoleOptions': INTERNAL_CONSOLE_OPTIONS_SCHEMA, 'debug.openDebug': { enum: ['neverOpen', 'openOnSessionStart', 'openOnFirstSessionStart'], From b70f2d4bb2fd121c0ef06db9aed46de4a862ddc4 Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 14 Nov 2017 23:18:55 +0100 Subject: [PATCH 04/19] debug: pass in name when checking if debug start is enabled fixes #38045 --- src/vs/workbench/parts/debug/browser/debugActions.ts | 9 ++++----- src/vs/workbench/parts/debug/browser/debugQuickOpen.ts | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/parts/debug/browser/debugActions.ts b/src/vs/workbench/parts/debug/browser/debugActions.ts index b936c028751..9d52ade5213 100644 --- a/src/vs/workbench/parts/debug/browser/debugActions.ts +++ b/src/vs/workbench/parts/debug/browser/debugActions.ts @@ -132,9 +132,8 @@ export class StartAction extends AbstractDebugAction { return false; } - public static isEnabled(debugService: IDebugService, contextService: IWorkspaceContextService) { + public static isEnabled(debugService: IDebugService, contextService: IWorkspaceContextService, configName: string) { const processes = debugService.getModel().getProcesses(); - const selectedName = debugService.getConfigurationManager().selectedName; const launch = debugService.getConfigurationManager().selectedLaunch; if (debugService.state === State.Initializing) { @@ -143,10 +142,10 @@ export class StartAction extends AbstractDebugAction { if (contextService && contextService.getWorkbenchState() === WorkbenchState.EMPTY && processes.length > 0) { return false; } - if (processes.some(p => p.getName(false) === selectedName && (!launch || p.session.root.uri.toString() === launch.workspace.uri.toString()))) { + if (processes.some(p => p.getName(false) === configName && (!launch || p.session.root.uri.toString() === launch.workspace.uri.toString()))) { return false; } - const compound = launch && launch.getCompound(selectedName); + const compound = launch && launch.getCompound(configName); if (compound && compound.configurations && processes.some(p => compound.configurations.indexOf(p.getName(false)) !== -1)) { return false; } @@ -156,7 +155,7 @@ export class StartAction extends AbstractDebugAction { // Disabled if the launch drop down shows the launch config that is already running. protected isEnabled(state: State): boolean { - return StartAction.isEnabled(this.debugService, this.contextService); + return StartAction.isEnabled(this.debugService, this.contextService, this.debugService.getConfigurationManager().selectedName); } } diff --git a/src/vs/workbench/parts/debug/browser/debugQuickOpen.ts b/src/vs/workbench/parts/debug/browser/debugQuickOpen.ts index 22b15c73359..cd90a6fd6fc 100644 --- a/src/vs/workbench/parts/debug/browser/debugQuickOpen.ts +++ b/src/vs/workbench/parts/debug/browser/debugQuickOpen.ts @@ -65,7 +65,7 @@ class StartDebugEntry extends Model.QuickOpenEntry { } public run(mode: QuickOpen.Mode, context: Model.IContext): boolean { - if (mode === QuickOpen.Mode.PREVIEW || !StartAction.isEnabled(this.debugService, this.contextService)) { + if (mode === QuickOpen.Mode.PREVIEW || !StartAction.isEnabled(this.debugService, this.contextService, this.configurationName)) { return false; } // Run selected debug configuration From cfb83449637eb31de626f8823585d0025519c286 Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 14 Nov 2017 23:41:01 +0100 Subject: [PATCH 05/19] layout: keep size before maximized in storage to be preserved across sessions fixes #37375 --- src/vs/workbench/browser/layout.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/browser/layout.ts b/src/vs/workbench/browser/layout.ts index 87c49af6222..aadf82d6a0b 100644 --- a/src/vs/workbench/browser/layout.ts +++ b/src/vs/workbench/browser/layout.ts @@ -52,6 +52,7 @@ export class WorkbenchLayout implements IVerticalSashLayoutProvider, IHorizontal private static sashXOneWidthSettingsKey = 'workbench.sidebar.width'; private static sashXTwoWidthSettingsKey = 'workbench.panel.width'; private static sashYHeightSettingsKey = 'workbench.panel.height'; + private static panelSizeBeforeMaximizedKey = 'workbench.panel.sizeBeforeMaximized'; private parent: Builder; private workbenchContainer: Builder; @@ -108,7 +109,7 @@ export class WorkbenchLayout implements IVerticalSashLayoutProvider, IHorizontal this.statusbar = parts.statusbar; this.quickopen = quickopen; this.toUnbind = []; - this.panelSizeBeforeMaximized = 0; + this.panelSizeBeforeMaximized = this.storageService.getInteger(WorkbenchLayout.panelSizeBeforeMaximizedKey, StorageScope.GLOBAL, 0); this.panelMaximized = false; this.sashXOne = new Sash(this.workbenchContainer.getHTMLElement(), this, { @@ -501,6 +502,7 @@ export class WorkbenchLayout implements IVerticalSashLayoutProvider, IHorizontal this.panelSizeBeforeMaximized = panelWidth; } } + this.storageService.store(WorkbenchLayout.panelSizeBeforeMaximizedKey, this.panelSizeBeforeMaximized, StorageScope.GLOBAL); const panelDimension = new Dimension(panelWidth, panelHeight); // Editor From 03e1d7ddd1e89d978fb0236a9f3fdbf3d1a9f29c Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 14 Nov 2017 15:48:40 -0800 Subject: [PATCH 06/19] Fix errant fuzzy widget and invisible count element on keybindings editor --- .../preferences/browser/preferencesEditor.ts | 4 +- .../preferences/browser/preferencesWidgets.ts | 76 +++++++++++-------- 2 files changed, 49 insertions(+), 31 deletions(-) diff --git a/src/vs/workbench/parts/preferences/browser/preferencesEditor.ts b/src/vs/workbench/parts/preferences/browser/preferencesEditor.ts index 1a3c4c20126..4ef60a4c0c5 100644 --- a/src/vs/workbench/parts/preferences/browser/preferencesEditor.ts +++ b/src/vs/workbench/parts/preferences/browser/preferencesEditor.ts @@ -143,7 +143,9 @@ export class PreferencesEditor extends BaseEditor { this.searchWidget = this._register(this.instantiationService.createInstance(SearchWidget, this.headerContainer, { ariaLabel: nls.localize('SearchSettingsWidget.AriaLabel', "Search settings"), placeholder: nls.localize('SearchSettingsWidget.Placeholder', "Search Settings"), - focusKey: this.focusSettingsContextKey + focusKey: this.focusSettingsContextKey, + showFuzzyToggle: true, + showResultCount: true })); this.searchWidget.setFuzzyToggleVisible(this.searchProvider.remoteSearchEnabled); this.searchWidget.fuzzyEnabled = this.memento['fuzzyEnabled']; diff --git a/src/vs/workbench/parts/preferences/browser/preferencesWidgets.ts b/src/vs/workbench/parts/preferences/browser/preferencesWidgets.ts index 1dc121e8af3..f9d05c9ee06 100644 --- a/src/vs/workbench/parts/preferences/browser/preferencesWidgets.ts +++ b/src/vs/workbench/parts/preferences/browser/preferencesWidgets.ts @@ -424,6 +424,8 @@ export class SettingsTargetsWidget extends Widget { export interface SearchOptions extends IInputOptions { focusKey?: IContextKey; + showFuzzyToggle?: boolean; + showResultCount?: boolean; } export class SearchWidget extends Widget { @@ -463,33 +465,37 @@ export class SearchWidget extends Widget { this.domNode = DOM.append(parent, DOM.$('div.settings-header-widget')); this.createSearchContainer(DOM.append(this.domNode, DOM.$('div.settings-search-container'))); this.controlsDiv = DOM.append(this.domNode, DOM.$('div.settings-search-controls')); - this.fuzzyToggle = this._register(new Checkbox({ - actionClassName: 'prefs-fuzzy-search-toggle', - isChecked: false, - onChange: () => { - this.inputBox.focus(); - this._onDidChange.fire(); - }, - title: localize('enableFuzzySearch', 'Enable experimental fuzzy search') - })); - DOM.append(this.controlsDiv, this.fuzzyToggle.domNode); - this._register(attachCheckboxStyler(this.fuzzyToggle, this.themeService)); + if (this.options.showFuzzyToggle) { + this.fuzzyToggle = this._register(new Checkbox({ + actionClassName: 'prefs-fuzzy-search-toggle', + isChecked: false, + onChange: () => { + this.inputBox.focus(); + this._onDidChange.fire(); + }, + title: localize('enableFuzzySearch', 'Enable experimental fuzzy search') + })); + DOM.append(this.controlsDiv, this.fuzzyToggle.domNode); + this._register(attachCheckboxStyler(this.fuzzyToggle, this.themeService)); + } - this.countElement = DOM.append(this.controlsDiv, DOM.$('.settings-count-widget')); - this._register(attachStylerCallback(this.themeService, { badgeBackground, contrastBorder }, colors => { - const background = colors.badgeBackground ? colors.badgeBackground.toString() : null; - const border = colors.contrastBorder ? colors.contrastBorder.toString() : null; + if (this.options.showResultCount) { + this.countElement = DOM.append(this.controlsDiv, DOM.$('.settings-count-widget')); + this._register(attachStylerCallback(this.themeService, { badgeBackground, contrastBorder }, colors => { + const background = colors.badgeBackground ? colors.badgeBackground.toString() : null; + const border = colors.contrastBorder ? colors.contrastBorder.toString() : null; - this.countElement.style.backgroundColor = background; + this.countElement.style.backgroundColor = background; - this.countElement.style.borderWidth = border ? '1px' : null; - this.countElement.style.borderStyle = border ? 'solid' : null; - this.countElement.style.borderColor = border; + this.countElement.style.borderWidth = border ? '1px' : null; + this.countElement.style.borderStyle = border ? 'solid' : null; + this.countElement.style.borderColor = border; + + this.styleCountElementForeground(); + })); + } - this.styleCountElementForeground(); - })); this.inputBox.inputElement.setAttribute('aria-live', 'assertive'); - const focusTracker = this._register(DOM.trackFocus(this.inputBox.inputElement)); this._register(focusTracker.addFocusListener(() => this._onFocus.fire())); @@ -514,11 +520,13 @@ export class SearchWidget extends Widget { } public showMessage(message: string, count: number): void { - this.countElement.textContent = message; - this.inputBox.inputElement.setAttribute('aria-label', message); - DOM.toggleClass(this.countElement, 'no-results', count === 0); - this.inputBox.inputElement.style.paddingRight = this.getControlsWidth() + 'px'; - this.styleCountElementForeground(); + if (this.countElement) { + this.countElement.textContent = message; + this.inputBox.inputElement.setAttribute('aria-label', message); + DOM.toggleClass(this.countElement, 'no-results', count === 0); + this.inputBox.inputElement.style.paddingRight = this.getControlsWidth() + 'px'; + this.styleCountElementForeground(); + } } public setFuzzyToggleVisible(visible: boolean): void { @@ -539,16 +547,24 @@ export class SearchWidget extends Widget { public layout(dimension: Dimension) { if (dimension.width < 400) { - DOM.addClass(this.countElement, 'hide'); + if (this.countElement) { + DOM.addClass(this.countElement, 'hide'); + } + this.inputBox.inputElement.style.paddingRight = '0px'; } else { - DOM.removeClass(this.countElement, 'hide'); + if (this.countElement) { + DOM.removeClass(this.countElement, 'hide'); + } + this.inputBox.inputElement.style.paddingRight = this.getControlsWidth() + 'px'; } } private getControlsWidth(): number { - return DOM.getTotalWidth(this.countElement) + DOM.getTotalWidth(this.fuzzyToggle.domNode) + 20; + const countWidth = this.countElement ? DOM.getTotalWidth(this.countElement) : 0; + const fuzzyToggleWidth = this.fuzzyToggle ? DOM.getTotalWidth(this.fuzzyToggle.domNode) : 0; + return countWidth + fuzzyToggleWidth + 20; } public focus() { From 3a7d517f65212de615f469587c2ace53275ce96e Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 14 Nov 2017 16:29:56 -0800 Subject: [PATCH 07/19] Use octicon lightbulb for fuzzy search toggle button --- .../parts/preferences/browser/media/preferences.css | 11 +++++------ .../parts/preferences/browser/preferencesWidgets.ts | 2 ++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/parts/preferences/browser/media/preferences.css b/src/vs/workbench/parts/preferences/browser/media/preferences.css index 4084078ea86..603cd871d5f 100644 --- a/src/vs/workbench/parts/preferences/browser/media/preferences.css +++ b/src/vs/workbench/parts/preferences/browser/media/preferences.css @@ -97,12 +97,11 @@ display: none; } -.vs .settings-header-widget > .settings-search-controls > .prefs-fuzzy-search-toggle { - background: url('regex.svg') center center no-repeat; -} - -.vs-dark .settings-header-widget > .settings-search-controls > .prefs-fuzzy-search-toggle { - background: url('regex-dark.svg') center center no-repeat; +.settings-header-widget > .settings-search-controls > .prefs-fuzzy-search-toggle > .octicon { + text-align: center; + vertical-align: top; + font-size: 16px; + width: 100%; } .settings-header-widget > .settings-search-container { diff --git a/src/vs/workbench/parts/preferences/browser/preferencesWidgets.ts b/src/vs/workbench/parts/preferences/browser/preferencesWidgets.ts index f9d05c9ee06..9b4b70cdde9 100644 --- a/src/vs/workbench/parts/preferences/browser/preferencesWidgets.ts +++ b/src/vs/workbench/parts/preferences/browser/preferencesWidgets.ts @@ -37,6 +37,7 @@ import { IMouseEvent } from 'vs/base/browser/mouseEvent'; import { MarkdownString } from 'vs/base/common/htmlContent'; import { ConfigurationTarget } from 'vs/platform/configuration/common/configuration'; import { IMarginData } from 'vs/editor/browser/controller/mouseTarget'; +import { render as renderOcticons } from 'vs/base/browser/ui/octiconLabel/octiconLabel'; export class SettingsHeaderWidget extends Widget implements IViewZone { @@ -475,6 +476,7 @@ export class SearchWidget extends Widget { }, title: localize('enableFuzzySearch', 'Enable experimental fuzzy search') })); + this.fuzzyToggle.domNode.innerHTML = renderOcticons('$(light-bulb)'); DOM.append(this.controlsDiv, this.fuzzyToggle.domNode); this._register(attachCheckboxStyler(this.fuzzyToggle, this.themeService)); } From ed9092c0a503f880b3f9d2c9ee0b0d4c20bfda8e Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Tue, 14 Nov 2017 16:51:38 -0800 Subject: [PATCH 08/19] Fit promise error in fileServices `detectMimeAndEncodingFromBuffer` returns a `TPromise | X`. Seems we need to use `TPromise.resolve`to wrap this instead Fixes #38360 --- src/vs/workbench/services/files/node/fileService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/services/files/node/fileService.ts b/src/vs/workbench/services/files/node/fileService.ts index eb6427ed3c5..a09a8e194a9 100644 --- a/src/vs/workbench/services/files/node/fileService.ts +++ b/src/vs/workbench/services/files/node/fileService.ts @@ -449,7 +449,7 @@ export class FileService implements IFileService { } else { // when receiving the first chunk of data we need to create the // decoding stream which is then used to drive the string stream. - Promise.resolve(detectMimeAndEncodingFromBuffer( + TPromise.as(detectMimeAndEncodingFromBuffer( { buffer: chunkBuffer, bytesRead }, options && options.autoGuessEncoding || this.configuredAutoGuessEncoding(resource) )).then(value => { @@ -468,7 +468,7 @@ export class FileService implements IFileService { handleChunk(bytesRead); } - }).catch(err => { + }).then(undefined, err => { // failed to get encoding finish(err); }); From f2daea90470fd607d73bf38f45f37003601b887d Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 15 Nov 2017 08:49:56 +0100 Subject: [PATCH 09/19] add type annotation --- src/vs/base/common/lifecycle.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/base/common/lifecycle.ts b/src/vs/base/common/lifecycle.ts index 191260756a6..79297c9556e 100644 --- a/src/vs/base/common/lifecycle.ts +++ b/src/vs/base/common/lifecycle.ts @@ -7,7 +7,7 @@ import { once } from 'vs/base/common/functional'; -export const empty: IDisposable = Object.freeze({ +export const empty: IDisposable = Object.freeze({ dispose() { } }); From 18e70eb636e20d51ae0f183d600662ead015427a Mon Sep 17 00:00:00 2001 From: Dirk Baeumer Date: Wed, 15 Nov 2017 09:15:46 +0100 Subject: [PATCH 10/19] Fixes #38344: Cannot create custom tasks when there are detected tasks --- .../tasks/electron-browser/task.contribution.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/parts/tasks/electron-browser/task.contribution.ts b/src/vs/workbench/parts/tasks/electron-browser/task.contribution.ts index 16084fe01e6..3712b2b4288 100644 --- a/src/vs/workbench/parts/tasks/electron-browser/task.contribution.ts +++ b/src/vs/workbench/parts/tasks/electron-browser/task.contribution.ts @@ -2192,12 +2192,19 @@ class TaskService extends EventEmitter implements ITaskService { let entries: EntryType[] = []; if (this.contextService.getWorkbenchState() === WorkbenchState.FOLDER) { let tasks = taskMap.all(); + let needsCreateOrOpen: boolean = true; if (tasks.length > 0) { tasks = tasks.sort((a, b) => a._label.localeCompare(b._label)); - entries = tasks.map(task => { return { label: task._label, task }; }); - } else { + for (let task of tasks) { + entries.push({ label: task._label, task }); + if (!ContributedTask.is(task)) { + needsCreateOrOpen = false; + } + } + } + if (needsCreateOrOpen) { let label = stats[0] !== void 0 ? openLabel : createLabel; - entries.push({ label, folder: this.contextService.getWorkspace().folders[0] }); + entries.push({ label, folder: this.contextService.getWorkspace().folders[0], separator: entries.length > 0 ? { border: true } : undefined }); } } else { let folders = this.contextService.getWorkspace().folders; From 9486025f8cbdd1107c3dea916b2ffcd64c2739cb Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Wed, 15 Nov 2017 09:22:51 +0100 Subject: [PATCH 11/19] perf - remove unused code --- src/vs/base/node/encoding.ts | 4 - src/vs/base/node/mime.ts | 69 +-------------- src/vs/base/node/processes.ts | 55 ------------ src/vs/base/node/stream.ts | 96 ++++++--------------- src/vs/base/test/node/mime/mime.test.ts | 42 +++++---- src/vs/base/test/node/stream/stream.test.ts | 20 ----- 6 files changed, 52 insertions(+), 234 deletions(-) diff --git a/src/vs/base/node/encoding.ts b/src/vs/base/node/encoding.ts index e7579c071c4..b8b6707cda7 100644 --- a/src/vs/base/node/encoding.ts +++ b/src/vs/base/node/encoding.ts @@ -42,10 +42,6 @@ export function decodeStream(encoding: string): NodeJS.ReadWriteStream { return iconv.decodeStream(toNodeEncoding(encoding)); } -export function encodeStream(encoding: string): NodeJS.ReadWriteStream { - return iconv.encodeStream(toNodeEncoding(encoding)); -} - function toNodeEncoding(enc: string): string { if (enc === UTF8_with_bom) { return UTF8; // iconv does not distinguish UTF 8 with or without BOM, so we need to help it diff --git a/src/vs/base/node/mime.ts b/src/vs/base/node/mime.ts index 1e71046cce6..1b915e78117 100644 --- a/src/vs/base/node/mime.ts +++ b/src/vs/base/node/mime.ts @@ -5,8 +5,6 @@ 'use strict'; -import streams = require('stream'); - import mime = require('vs/base/common/mime'); import { TPromise } from 'vs/base/common/winjs.base'; @@ -76,18 +74,6 @@ export interface DetectMimesOption { autoGuessEncoding?: boolean; } -function doDetectMimesFromStream(instream: streams.Readable, option?: DetectMimesOption): TPromise { - return stream.readExactlyByStream(instream, maxBufferLen(option)).then((readResult: stream.ReadResult) => { - return detectMimeAndEncodingFromBuffer(readResult, option && option.autoGuessEncoding); - }); -} - -function doDetectMimesFromFile(absolutePath: string, option?: DetectMimesOption): TPromise { - return stream.readExactlyByFile(absolutePath, maxBufferLen(option)).then((readResult: stream.ReadResult) => { - return detectMimeAndEncodingFromBuffer(readResult, option && option.autoGuessEncoding); - }); -} - export function detectMimeAndEncodingFromBuffer(readResult: stream.ReadResult, autoGuessEncoding?: false): IMimeAndEncoding; export function detectMimeAndEncodingFromBuffer(readResult: stream.ReadResult, autoGuessEncoding?: boolean): TPromise; export function detectMimeAndEncodingFromBuffer({ buffer, bytesRead }: stream.ReadResult, autoGuessEncoding?: boolean): TPromise | IMimeAndEncoding { @@ -117,57 +103,4 @@ export function detectMimeAndEncodingFromBuffer({ buffer, bytesRead }: stream.Re mimes: isText ? [mime.MIME_TEXT] : [mime.MIME_BINARY], encoding: enc }; -} - -function filterAndSortMimes(detectedMimes: string[], guessedMimes: string[]): string[] { - const mimes = detectedMimes; - - // Add extension based mime as first element as this is the desire of whoever created the file. - // Never care about application/octet-stream or application/unknown as guessed mime, as this is the fallback of the guess which is never accurate - const guessedMime = guessedMimes[0]; - if (guessedMime !== mime.MIME_BINARY && guessedMime !== mime.MIME_UNKNOWN) { - mimes.unshift(guessedMime); - } - - // Remove duplicate elements from array and sort unspecific mime to the end - const uniqueSortedMimes = mimes.filter((element, position) => { - return element && mimes.indexOf(element) === position; - }).sort((mimeA, mimeB) => { - if (mimeA === mime.MIME_BINARY) { return 1; } - if (mimeB === mime.MIME_BINARY) { return -1; } - if (mimeA === mime.MIME_TEXT) { return 1; } - if (mimeB === mime.MIME_TEXT) { return -1; } - - return 0; - }); - - return uniqueSortedMimes; -} - -/** - * Opens the given stream to detect its mime type. Returns an array of mime types sorted from most specific to unspecific. - * @param instream the readable stream to detect the mime types from. - * @param nameHint an additional hint that can be used to detect a mime from a file extension. - */ -export function detectMimesFromStream(instream: streams.Readable, nameHint: string, option?: DetectMimesOption): TPromise { - return doDetectMimesFromStream(instream, option).then(encoding => - handleMimeResult(nameHint, encoding) - ); -} - -/** - * Opens the given file to detect its mime type. Returns an array of mime types sorted from most specific to unspecific. - * @param absolutePath the absolute path of the file. - */ -export function detectMimesFromFile(absolutePath: string, option?: DetectMimesOption): TPromise { - return doDetectMimesFromFile(absolutePath, option).then(encoding => - handleMimeResult(absolutePath, encoding) - ); -} - -function handleMimeResult(nameHint: string, result: IMimeAndEncoding): IMimeAndEncoding { - const filterAndSortedMimes = filterAndSortMimes(result.mimes, mime.guessMimeTypes(nameHint)); - result.mimes = filterAndSortedMimes; - - return result; -} +} \ No newline at end of file diff --git a/src/vs/base/node/processes.ts b/src/vs/base/node/processes.ts index 1911add6b25..dd5b51cd0f9 100644 --- a/src/vs/base/node/processes.ts +++ b/src/vs/base/node/processes.ts @@ -9,7 +9,6 @@ import * as cp from 'child_process'; import ChildProcess = cp.ChildProcess; import exec = cp.exec; import spawn = cp.spawn; -import { PassThrough } from 'stream'; import { fork } from 'vs/base/node/stdFork'; import nls = require('vs/nls'); import { PPromise, TPromise, TValueCallback, TProgressCallback, ErrorCallback } from 'vs/base/common/winjs.base'; @@ -391,60 +390,6 @@ export class LineProcess extends AbstractProcess { } } -export class BufferProcess extends AbstractProcess { - - public constructor(executable: Executable); - public constructor(cmd: string, args: string[], shell: boolean, options: CommandOptions); - public constructor(module: string, args: string[], options: ForkOptions); - public constructor(arg1: string | Executable, arg2?: string[], arg3?: boolean | ForkOptions, arg4?: CommandOptions) { - super(arg1, arg2, arg3, arg4); - } - - protected handleExec(cc: TValueCallback, pp: TProgressCallback, error: Error, stdout: Buffer, stderr: Buffer): void { - pp({ data: stdout, source: Source.stdout }); - pp({ data: stderr, source: Source.stderr }); - cc({ terminated: this.terminateRequested, error: error }); - } - - protected handleSpawn(childProcess: ChildProcess, cc: TValueCallback, pp: TProgressCallback, ee: ErrorCallback, sync: boolean): void { - childProcess.stdout.on('data', (data: Buffer) => { - pp({ data: data, source: Source.stdout }); - }); - childProcess.stderr.on('data', (data: Buffer) => { - pp({ data: data, source: Source.stderr }); - }); - } -} - -export class StreamProcess extends AbstractProcess { - - public constructor(executable: Executable); - public constructor(cmd: string, args: string[], shell: boolean, options: CommandOptions); - public constructor(module: string, args: string[], options: ForkOptions); - public constructor(arg1: string | Executable, arg2?: string[], arg3?: boolean | ForkOptions, arg4?: CommandOptions) { - super(arg1, arg2, arg3, arg4); - } - - protected handleExec(cc: TValueCallback, pp: TProgressCallback, error: Error, stdout: Buffer, stderr: Buffer): void { - let stdoutStream = new PassThrough(); - stdoutStream.end(stdout); - let stderrStream = new PassThrough(); - stderrStream.end(stderr); - pp({ stdin: null, stdout: stdoutStream, stderr: stderrStream }); - cc({ terminated: this.terminateRequested, error: error }); - } - - protected handleSpawn(childProcess: ChildProcess, cc: TValueCallback, pp: TProgressCallback, ee: ErrorCallback, sync: boolean): void { - if (sync) { - process.nextTick(() => { - pp({ stdin: childProcess.stdin, stdout: childProcess.stdout, stderr: childProcess.stderr }); - }); - } else { - pp({ stdin: childProcess.stdin, stdout: childProcess.stdout, stderr: childProcess.stderr }); - } - } -} - export interface IQueuedSender { send: (msg: any) => void; } diff --git a/src/vs/base/node/stream.ts b/src/vs/base/node/stream.ts index 40fe0553574..897f9d36f5f 100644 --- a/src/vs/base/node/stream.ts +++ b/src/vs/base/node/stream.ts @@ -6,7 +6,6 @@ 'use strict'; import fs = require('fs'); -import stream = require('stream'); import { TPromise } from 'vs/base/common/winjs.base'; @@ -15,43 +14,6 @@ export interface ReadResult { bytesRead: number; } -/** - * Reads up to total bytes from the provided stream. - */ -export function readExactlyByStream(stream: stream.Readable, totalBytes: number): TPromise { - return new TPromise((complete, error) => { - let done = false; - let buffer = new Buffer(totalBytes); - let bytesRead = 0; - - stream.on('data', (data: NodeBuffer) => { - let bytesToRead = Math.min(totalBytes - bytesRead, data.length); - data.copy(buffer, bytesRead, 0, bytesToRead); - bytesRead += bytesToRead; - - if (bytesRead === totalBytes) { - (stream as any).destroy(); // Will trigger the close event eventually - } - }); - - stream.on('error', (e: Error) => { - if (!done) { - done = true; - error(e); - } - }); - - let onSuccess = () => { - if (!done) { - done = true; - complete({ buffer, bytesRead }); - } - }; - - stream.on('close', onSuccess); - }); -} - /** * Reads totalBytes from the provided file. */ @@ -63,7 +25,7 @@ export function readExactlyByFile(file: string, totalBytes: number): TPromise { + fs.close(fd, closeError => { if (closeError) { return error(closeError); } @@ -76,35 +38,30 @@ export function readExactlyByFile(file: string, totalBytes: number): TPromise { + const buffer = new Buffer(totalBytes); + let offset = 0; + + function readChunk(): void { + fs.read(fd, buffer, offset, totalBytes - offset, null, (err, bytesRead) => { if (err) { return end(err, null, 0); } - // Retry up to N times in case 0 bytes where read - if (moreBytesRead === 0) { - if (++zeroAttempts === 10) { - return end(null, buffer, bytesRead); - } - - return loop(); + if (bytesRead === 0) { + return end(null, buffer, offset); } - bytesRead += moreBytesRead; + offset += bytesRead; - if (bytesRead === totalBytes) { - return end(null, buffer, bytesRead); + if (offset === totalBytes) { + return end(null, buffer, offset); } - return loop(); + return readChunk(); }); } - loop(); + readChunk(); }); }); } @@ -126,7 +83,7 @@ export function readToMatchingString(file: string, matchingString: string, chunk } function end(err: Error, result: string): void { - fs.close(fd, (closeError: Error) => { + fs.close(fd, closeError => { if (closeError) { return error(closeError); } @@ -140,39 +97,34 @@ export function readToMatchingString(file: string, matchingString: string, chunk } let buffer = new Buffer(maximumBytesToRead); - let bytesRead = 0; - let zeroAttempts = 0; - function loop(): void { - fs.read(fd, buffer, bytesRead, chunkBytes, null, (err, moreBytesRead) => { + let offset = 0; + + function readChunk(): void { + fs.read(fd, buffer, offset, chunkBytes, null, (err, bytesRead) => { if (err) { return end(err, null); } - // Retry up to N times in case 0 bytes where read - if (moreBytesRead === 0) { - if (++zeroAttempts === 10) { - return end(null, null); - } - - return loop(); + if (bytesRead === 0) { + return end(null, null); } - bytesRead += moreBytesRead; + offset += bytesRead; const newLineIndex = buffer.indexOf(matchingString); if (newLineIndex >= 0) { return end(null, buffer.toString('utf8').substr(0, newLineIndex)); } - if (bytesRead >= maximumBytesToRead) { + if (offset >= maximumBytesToRead) { return end(new Error(`Could not find ${matchingString} in first ${maximumBytesToRead} bytes of ${file}`), null); } - return loop(); + return readChunk(); }); } - loop(); + readChunk(); }) ); } \ No newline at end of file diff --git a/src/vs/base/test/node/mime/mime.test.ts b/src/vs/base/test/node/mime/mime.test.ts index ef791aadb84..bb4a9bde51f 100644 --- a/src/vs/base/test/node/mime/mime.test.ts +++ b/src/vs/base/test/node/mime/mime.test.ts @@ -9,29 +9,34 @@ import assert = require('assert'); import mimeCommon = require('vs/base/common/mime'); import mime = require('vs/base/node/mime'); +import { readExactlyByFile } from 'vs/base/node/stream'; suite('Mime', () => { test('detectMimesFromFile (JSON saved as PNG)', function (done: (err?: any) => void) { const file = require.toUrl('./fixtures/some.json.png'); - mime.detectMimesFromFile(file).then(mimes => { + + readExactlyByFile(file, 512).then(buffer => { + const mimes = mime.detectMimeAndEncodingFromBuffer(buffer); assert.deepEqual(mimes.mimes, ['text/plain']); done(); - }, done); + }); }); test('detectMimesFromFile (PNG saved as TXT)', function (done: (err?: any) => void) { mimeCommon.registerTextMime({ id: 'text', mime: 'text/plain', extension: '.txt' }); const file = require.toUrl('./fixtures/some.png.txt'); - mime.detectMimesFromFile(file).then(mimes => { - assert.deepEqual(mimes.mimes, ['text/plain', 'application/octet-stream']); + readExactlyByFile(file, 512).then(buffer => { + const mimes = mime.detectMimeAndEncodingFromBuffer(buffer); + assert.deepEqual(mimes.mimes, ['application/octet-stream']); done(); }, done); }); test('detectMimesFromFile (XML saved as PNG)', function (done: (err?: any) => void) { const file = require.toUrl('./fixtures/some.xml.png'); - mime.detectMimesFromFile(file).then(mimes => { + readExactlyByFile(file, 512).then(buffer => { + const mimes = mime.detectMimeAndEncodingFromBuffer(buffer); assert.deepEqual(mimes.mimes, ['text/plain']); done(); }, done); @@ -39,15 +44,17 @@ suite('Mime', () => { test('detectMimesFromFile (QWOFF saved as TXT)', function (done: (err?: any) => void) { const file = require.toUrl('./fixtures/some.qwoff.txt'); - mime.detectMimesFromFile(file).then(mimes => { - assert.deepEqual(mimes.mimes, ['text/plain', 'application/octet-stream']); + readExactlyByFile(file, 512).then(buffer => { + const mimes = mime.detectMimeAndEncodingFromBuffer(buffer); + assert.deepEqual(mimes.mimes, ['application/octet-stream']); done(); }, done); }); test('detectMimesFromFile (CSS saved as QWOFF)', function (done: (err?: any) => void) { const file = require.toUrl('./fixtures/some.css.qwoff'); - mime.detectMimesFromFile(file).then(mimes => { + readExactlyByFile(file, 512).then(buffer => { + const mimes = mime.detectMimeAndEncodingFromBuffer(buffer); assert.deepEqual(mimes.mimes, ['text/plain']); done(); }, done); @@ -55,7 +62,8 @@ suite('Mime', () => { test('detectMimesFromFile (PDF)', function (done: () => void) { const file = require.toUrl('./fixtures/some.pdf'); - mime.detectMimesFromFile(file).then(mimes => { + readExactlyByFile(file, 512).then(buffer => { + const mimes = mime.detectMimeAndEncodingFromBuffer(buffer); assert.deepEqual(mimes.mimes, ['application/octet-stream']); done(); }, done); @@ -63,17 +71,21 @@ suite('Mime', () => { test('autoGuessEncoding (ShiftJIS)', function (done: () => void) { const file = require.toUrl('./fixtures/some.shiftjis.txt'); - mime.detectMimesFromFile(file, { autoGuessEncoding: true }).then(mimes => { - assert.equal(mimes.encoding, 'shiftjis'); - done(); + readExactlyByFile(file, 512 * 8).then(buffer => { + mime.detectMimeAndEncodingFromBuffer(buffer, true).then(mimes => { + assert.equal(mimes.encoding, 'shiftjis'); + done(); + }); }, done); }); test('autoGuessEncoding (CP1252)', function (done: () => void) { const file = require.toUrl('./fixtures/some.cp1252.txt'); - mime.detectMimesFromFile(file, { autoGuessEncoding: true }).then(mimes => { - assert.equal(mimes.encoding, 'windows1252'); - done(); + readExactlyByFile(file, 512 * 8).then(buffer => { + mime.detectMimeAndEncodingFromBuffer(buffer, true).then(mimes => { + assert.equal(mimes.encoding, 'windows1252'); + done(); + }); }, done); }); }); diff --git a/src/vs/base/test/node/stream/stream.test.ts b/src/vs/base/test/node/stream/stream.test.ts index 2cb3eca07e2..4dc7f9fb8c6 100644 --- a/src/vs/base/test/node/stream/stream.test.ts +++ b/src/vs/base/test/node/stream/stream.test.ts @@ -6,7 +6,6 @@ 'use strict'; import assert = require('assert'); -import fs = require('fs'); import stream = require('vs/base/node/stream'); @@ -30,25 +29,6 @@ suite('Stream', () => { }, done); }); - test('readExactlyByStream - ANSI', function (done: (err?: any) => void) { - const file = require.toUrl('./fixtures/file.css'); - - stream.readExactlyByStream(fs.createReadStream(file), 10).then(({ buffer, bytesRead }) => { - assert.equal(bytesRead, 10); - assert.equal(buffer.toString(), '/*--------'); - done(); - }, done); - }); - - test('readExactlyByStream - empty', function (done: (err?: any) => void) { - const file = require.toUrl('./fixtures/empty.txt'); - - stream.readExactlyByStream(fs.createReadStream(file), 10).then(({ bytesRead }) => { - assert.equal(bytesRead, 0); - done(); - }, done); - }); - test('readToMatchingString - ANSI', function (done: (err?: any) => void) { const file = require.toUrl('./fixtures/file.css'); From 5a318c23511797396ad71314a89803d039273175 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Wed, 15 Nov 2017 10:06:33 +0100 Subject: [PATCH 12/19] perf - remove duplicated mkdirp method --- src/vs/base/node/extfs.ts | 98 +++++++++---------- src/vs/base/node/pfs.ts | 29 +----- src/vs/base/test/node/config.test.ts | 7 +- src/vs/base/test/node/extfs/extfs.test.ts | 34 ++++--- src/vs/base/test/node/pfs.test.ts | 30 ++++-- .../test/node/configurationService.test.ts | 7 +- .../electron-main/auto-updater.win32.ts | 3 +- .../node/configurationEditingService.test.ts | 24 ++--- .../test/node/configurationService.test.ts | 24 +---- .../test/node/keybindingEditing.test.ts | 15 +-- 10 files changed, 112 insertions(+), 159 deletions(-) diff --git a/src/vs/base/node/extfs.ts b/src/vs/base/node/extfs.ts index c41a1992540..e50ad6b102c 100644 --- a/src/vs/base/node/extfs.ts +++ b/src/vs/base/node/extfs.ts @@ -12,6 +12,8 @@ import * as flow from 'vs/base/node/flow'; import * as fs from 'fs'; import * as paths from 'path'; +import { TPromise } from 'vs/base/common/winjs.base'; +import { nfcall } from 'vs/base/common/async'; const loop = flow.loop; @@ -41,80 +43,72 @@ export function readdir(path: string, callback: (error: Error, files: string[]) return fs.readdir(path, callback); } -export function mkdirp(path: string, mode: number, callback: (error: Error) => void): void { - fs.exists(path, exists => { - if (exists) { - return isDirectory(path, (err: Error, itIs?: boolean) => { - if (err) { - return callback(err); - } - - if (!itIs) { - return callback(new Error('"' + path + '" is not a directory.')); - } - - callback(null); - }); - } - - mkdirp(paths.dirname(path), mode, (err: Error) => { - if (err) { callback(err); return; } - - if (mode) { - fs.mkdir(path, mode, error => { - if (error) { - return callback(error); - } - - fs.chmod(path, mode, callback); // we need to explicitly chmod because of https://github.com/nodejs/node/issues/1104 - }); - } else { - fs.mkdir(path, null, callback); - } - }); - }); -} - -function isDirectory(path: string, callback: (error: Error, isDirectory?: boolean) => void): void { - fs.stat(path, (error, stat) => { - if (error) { return callback(error); } - - callback(null, stat.isDirectory()); - }); -} - export function copy(source: string, target: string, callback: (error: Error) => void, copiedSources?: { [path: string]: boolean }): void { if (!copiedSources) { copiedSources = Object.create(null); } fs.stat(source, (error, stat) => { - if (error) { return callback(error); } - if (!stat.isDirectory()) { return pipeFs(source, target, stat.mode & 511, callback); } + if (error) { + return callback(error); + } + + if (!stat.isDirectory()) { + return pipeFs(source, target, stat.mode & 511, callback); + } if (copiedSources[source]) { return callback(null); // escape when there are cycles (can happen with symlinks) - } else { - copiedSources[source] = true; // remember as copied } - mkdirp(target, stat.mode & 511, err => { + copiedSources[source] = true; // remember as copied + + const proceed = function () { readdir(source, (err, files) => { loop(files, (file: string, clb: (error: Error, result: string[]) => void) => { copy(paths.join(source, file), paths.join(target, file), (error: Error) => clb(error, void 0), copiedSources); }, callback); }); + }; + + mkdirp(target, stat.mode & 511).done(proceed, proceed); + }); +} + +export function mkdirp(path: string, mode?: number): TPromise { + const mkdir = () => nfcall(fs.mkdir, path, mode) + .then(null, (err: NodeJS.ErrnoException) => { + if (err.code === 'EEXIST') { + return nfcall(fs.stat, path) + .then((stat: fs.Stats) => stat.isDirectory + ? null + : TPromise.wrapError(new Error(`'${path}' exists and is not a directory.`))); + } + + return TPromise.wrapError(err); }); + + // is root? + if (path === paths.dirname(path)) { + return TPromise.as(true); + } + + return mkdir().then(null, (err: NodeJS.ErrnoException) => { + if (err.code === 'ENOENT') { + return mkdirp(paths.dirname(path), mode).then(mkdir); + } + + return TPromise.wrapError(err); }); } function pipeFs(source: string, target: string, mode: number, callback: (error: Error) => void): void { let callbackHandled = false; - let readStream = fs.createReadStream(source); - let writeStream = fs.createWriteStream(target, { mode: mode }); + const readStream = fs.createReadStream(source); + const writeStream = fs.createWriteStream(target, { mode: mode }); - let onError = (error: Error) => { + const onError = (error: Error) => { if (!callbackHandled) { callbackHandled = true; callback(error); @@ -163,7 +157,7 @@ export function del(path: string, tmpFolder: string, callback: (error: Error) => return rmRecursive(path, callback); } - let pathInTemp = paths.join(tmpFolder, uuid.generateUuid()); + const pathInTemp = paths.join(tmpFolder, uuid.generateUuid()); fs.rename(path, pathInTemp, (error: Error) => { if (error) { return rmRecursive(path, callback); // if rename fails, delete without tmp dir @@ -200,7 +194,7 @@ function rmRecursive(path: string, callback: (error: Error) => void): void { if (err || !stat) { callback(err); } else if (!stat.isDirectory() || stat.isSymbolicLink() /* !!! never recurse into links when deleting !!! */) { - let mode = stat.mode; + const mode = stat.mode; if (!(mode & 128)) { // 128 === 0200 fs.chmod(path, mode | 128, (err: Error) => { // 128 === 0200 if (err) { diff --git a/src/vs/base/node/pfs.ts b/src/vs/base/node/pfs.ts index e75dd48d1b5..b949384de1e 100644 --- a/src/vs/base/node/pfs.ts +++ b/src/vs/base/node/pfs.ts @@ -7,7 +7,7 @@ import { TPromise } from 'vs/base/common/winjs.base'; import * as extfs from 'vs/base/node/extfs'; -import { dirname, join } from 'path'; +import { join } from 'path'; import { nfcall, Queue } from 'vs/base/common/async'; import * as fs from 'fs'; import * as os from 'os'; @@ -26,32 +26,7 @@ export function chmod(path: string, mode: number): TPromise { return nfcall(fs.chmod, path, mode); } -export function mkdirp(path: string, mode?: number): TPromise { - const mkdir = () => nfcall(fs.mkdir, path, mode) - .then(null, (err: NodeJS.ErrnoException) => { - if (err.code === 'EEXIST') { - return nfcall(fs.stat, path) - .then((stat: fs.Stats) => stat.isDirectory - ? null - : TPromise.wrapError(new Error(`'${path}' exists and is not a directory.`))); - } - - return TPromise.wrapError(err); - }); - - // is root? - if (path === dirname(path)) { - return TPromise.as(true); - } - - return mkdir().then(null, (err: NodeJS.ErrnoException) => { - if (err.code === 'ENOENT') { - return mkdirp(dirname(path), mode).then(mkdir); - } - - return TPromise.wrapError(err); - }); -} +export import mkdirp = extfs.mkdirp; export function rimraf(path: string): TPromise { return lstat(path).then(stat => { diff --git a/src/vs/base/test/node/config.test.ts b/src/vs/base/test/node/config.test.ts index 35867706551..2fc621eefea 100644 --- a/src/vs/base/test/node/config.test.ts +++ b/src/vs/base/test/node/config.test.ts @@ -14,6 +14,7 @@ import extfs = require('vs/base/node/extfs'); import uuid = require('vs/base/common/uuid'); import { ConfigWatcher } from 'vs/base/node/config'; import { onError } from 'vs/base/test/common/utils'; +import { mkdirp } from 'vs/base/node/pfs'; suite('Config', () => { @@ -23,9 +24,9 @@ suite('Config', () => { const newDir = path.join(parentDir, 'config', id); const testFile = path.join(newDir, 'config.json'); - extfs.mkdirp(newDir, 493, error => { - callback(error, testFile, (callback) => extfs.del(parentDir, os.tmpdir(), () => { }, callback)); - }); + const onMkdirp = error => callback(error, testFile, (callback) => extfs.del(parentDir, os.tmpdir(), () => { }, callback)); + + mkdirp(newDir, 493).done(() => onMkdirp(null), error => onMkdirp(error)); } test('defaults', function () { diff --git a/src/vs/base/test/node/extfs/extfs.test.ts b/src/vs/base/test/node/extfs/extfs.test.ts index 21fbc552dd8..fb3308f9a4a 100644 --- a/src/vs/base/test/node/extfs/extfs.test.ts +++ b/src/vs/base/test/node/extfs/extfs.test.ts @@ -18,6 +18,10 @@ import { onError } from 'vs/base/test/common/utils'; const ignore = () => { }; +const mkdirp = (path: string, mode: number, callback: (error) => void) => { + extfs.mkdirp(path, mode).done(() => callback(null), error => callback(error)); +}; + suite('Extfs', () => { test('mkdirp', function (done: () => void) { @@ -25,7 +29,7 @@ suite('Extfs', () => { const parentDir = path.join(os.tmpdir(), 'vsctests', id); const newDir = path.join(parentDir, 'extfs', id); - extfs.mkdirp(newDir, 493, (error) => { + mkdirp(newDir, 493, error => { if (error) { return onError(error, done); } @@ -51,7 +55,7 @@ suite('Extfs', () => { const parentDir = path.join(os.tmpdir(), 'vsctests', id); const newDir = path.join(parentDir, 'extfs', id); - extfs.mkdirp(newDir, 493, (error) => { + mkdirp(newDir, 493, error => { if (error) { return onError(error, done); } @@ -71,7 +75,7 @@ suite('Extfs', () => { const parentDir = path.join(os.tmpdir(), 'vsctests', id); const newDir = path.join(parentDir, 'extfs', id); - extfs.mkdirp(newDir, 493, (error) => { + mkdirp(newDir, 493, error => { if (error) { return onError(error, done); } @@ -97,7 +101,7 @@ suite('Extfs', () => { const targetDir = path.join(parentDir, id); const targetDir2 = path.join(parentDir, id2); - extfs.copy(sourceDir, targetDir, (error) => { + extfs.copy(sourceDir, targetDir, error => { if (error) { return onError(error, done); } @@ -109,7 +113,7 @@ suite('Extfs', () => { assert.ok(fs.statSync(path.join(targetDir, 'examples')).isDirectory()); assert.ok(fs.existsSync(path.join(targetDir, 'examples', 'small.jxs'))); - extfs.mv(targetDir, targetDir2, (error) => { + extfs.mv(targetDir, targetDir2, error => { if (error) { return onError(error, done); } @@ -122,7 +126,7 @@ suite('Extfs', () => { assert.ok(fs.statSync(path.join(targetDir2, 'examples')).isDirectory()); assert.ok(fs.existsSync(path.join(targetDir2, 'examples', 'small.jxs'))); - extfs.mv(path.join(targetDir2, 'index.html'), path.join(targetDir2, 'index_moved.html'), (error) => { + extfs.mv(path.join(targetDir2, 'index.html'), path.join(targetDir2, 'index_moved.html'), error => { if (error) { return onError(error, done); } @@ -130,11 +134,11 @@ suite('Extfs', () => { assert.ok(!fs.existsSync(path.join(targetDir2, 'index.html'))); assert.ok(fs.existsSync(path.join(targetDir2, 'index_moved.html'))); - extfs.del(parentDir, os.tmpdir(), (error) => { + extfs.del(parentDir, os.tmpdir(), error => { if (error) { return onError(error, done); } - }, (error) => { + }, error => { if (error) { return onError(error, done); } @@ -152,7 +156,7 @@ suite('Extfs', () => { const parentDir = path.join(os.tmpdir(), 'vsctests', id); const newDir = path.join(parentDir, 'extfs', id, 'öäü'); - extfs.mkdirp(newDir, 493, (error) => { + mkdirp(newDir, 493, error => { if (error) { return onError(error, done); } @@ -176,14 +180,14 @@ suite('Extfs', () => { const newDir = path.join(parentDir, 'extfs', id); const testFile = path.join(newDir, 'flushed.txt'); - extfs.mkdirp(newDir, 493, (error) => { + mkdirp(newDir, 493, error => { if (error) { return onError(error, done); } assert.ok(fs.existsSync(newDir)); - extfs.writeFileAndFlush(testFile, 'Hello World', null, (error) => { + extfs.writeFileAndFlush(testFile, 'Hello World', null, error => { if (error) { return onError(error, done); } @@ -192,7 +196,7 @@ suite('Extfs', () => { const largeString = (new Array(100 * 1024)).join('Large String\n'); - extfs.writeFileAndFlush(testFile, largeString, null, (error) => { + extfs.writeFileAndFlush(testFile, largeString, null, error => { if (error) { return onError(error, done); } @@ -210,7 +214,7 @@ suite('Extfs', () => { const parentDir = path.join(os.tmpdir(), 'vsctests', id); const newDir = path.join(parentDir, 'extfs', id); - extfs.mkdirp(newDir, 493, (error) => { + mkdirp(newDir, 493, error => { // assume case insensitive file system if (process.platform === 'win32' || process.platform === 'darwin') { @@ -239,7 +243,7 @@ suite('Extfs', () => { const parentDir = path.join(os.tmpdir(), 'vsctests', id); const newDir = path.join(parentDir, 'extfs', id); - extfs.mkdirp(newDir, 493, (error) => { + mkdirp(newDir, 493, error => { extfs.realpath(newDir, (error, realpath) => { assert.ok(realpath); @@ -255,7 +259,7 @@ suite('Extfs', () => { const parentDir = path.join(os.tmpdir(), 'vsctests', id); const newDir = path.join(parentDir, 'extfs', id); - extfs.mkdirp(newDir, 493, (error) => { + mkdirp(newDir, 493, error => { let realpath: string; try { realpath = extfs.realpathSync(newDir); diff --git a/src/vs/base/test/node/pfs.test.ts b/src/vs/base/test/node/pfs.test.ts index 8e6c07a766d..1f167ac7954 100644 --- a/src/vs/base/test/node/pfs.test.ts +++ b/src/vs/base/test/node/pfs.test.ts @@ -25,7 +25,7 @@ suite('PFS', () => { const newDir = path.join(parentDir, 'pfs', id); const testFile = path.join(newDir, 'writefile.txt'); - extfs.mkdirp(newDir, 493, (error) => { + const onMkdirp = error => { if (error) { return onError(error, done); } @@ -37,7 +37,9 @@ suite('PFS', () => { extfs.del(parentDir, os.tmpdir(), () => { }, done); }, error => onError(error, done)); - }); + }; + + pfs.mkdirp(newDir, 493).done(() => onMkdirp(null), error => onMkdirp(error)); }); test('writeFile - parallel write on different files works', function (done: () => void) { @@ -50,7 +52,7 @@ suite('PFS', () => { const testFile4 = path.join(newDir, 'writefile4.txt'); const testFile5 = path.join(newDir, 'writefile5.txt'); - extfs.mkdirp(newDir, 493, (error) => { + const onMkdirp = error => { if (error) { return onError(error, done); } @@ -72,7 +74,9 @@ suite('PFS', () => { extfs.del(parentDir, os.tmpdir(), () => { }, done); }, error => onError(error, done)); - }); + }; + + pfs.mkdirp(newDir, 493).done(() => onMkdirp(null), error => onMkdirp(error)); }); test('writeFile - parallel write on same files works and is sequentalized', function (done: () => void) { @@ -81,7 +85,7 @@ suite('PFS', () => { const newDir = path.join(parentDir, 'pfs', id); const testFile = path.join(newDir, 'writefile.txt'); - extfs.mkdirp(newDir, 493, (error) => { + const onMkdirp = error => { if (error) { return onError(error, done); } @@ -99,7 +103,9 @@ suite('PFS', () => { extfs.del(parentDir, os.tmpdir(), () => { }, done); }, error => onError(error, done)); - }); + }; + + pfs.mkdirp(newDir, 493).done(() => onMkdirp(null), error => onMkdirp(error)); }); test('rimraf - simple', function (done: () => void) { @@ -107,7 +113,7 @@ suite('PFS', () => { const parentDir = path.join(os.tmpdir(), 'vsctests', id); const newDir = path.join(parentDir, 'extfs', id); - extfs.mkdirp(newDir, 493, (error) => { + const onMkdirp = error => { if (error) { return onError(error, done); } @@ -119,7 +125,9 @@ suite('PFS', () => { assert.ok(!fs.existsSync(newDir)); done(); }, error => onError(error, done)); - }); // 493 = 0755 + }; + + pfs.mkdirp(newDir, 493).done(() => onMkdirp(null), error => onMkdirp(error)); }); test('rimraf - recursive folder structure', function (done: () => void) { @@ -127,7 +135,7 @@ suite('PFS', () => { const parentDir = path.join(os.tmpdir(), 'vsctests', id); const newDir = path.join(parentDir, 'extfs', id); - extfs.mkdirp(newDir, 493, (error) => { + const onMkdirp = error => { if (error) { return onError(error, done); } @@ -142,6 +150,8 @@ suite('PFS', () => { assert.ok(!fs.existsSync(newDir)); done(); }, error => onError(error, done)); - }); // 493 = 0755 + }; + + pfs.mkdirp(newDir, 493).done(() => onMkdirp(null), error => onMkdirp(error)); }); }); \ No newline at end of file diff --git a/src/vs/platform/configuration/test/node/configurationService.test.ts b/src/vs/platform/configuration/test/node/configurationService.test.ts index 60394c19de8..55964b1e41c 100644 --- a/src/vs/platform/configuration/test/node/configurationService.test.ts +++ b/src/vs/platform/configuration/test/node/configurationService.test.ts @@ -18,6 +18,7 @@ import { EnvironmentService } from 'vs/platform/environment/node/environmentServ import extfs = require('vs/base/node/extfs'); import uuid = require('vs/base/common/uuid'); import { IConfigurationRegistry, Extensions as ConfigurationExtensions } from 'vs/platform/configuration/common/configurationRegistry'; +import { mkdirp } from 'vs/base/node/pfs'; class SettingsTestEnvironmentService extends EnvironmentService { @@ -36,9 +37,9 @@ suite('ConfigurationService - Node', () => { const newDir = path.join(parentDir, 'config', id); const testFile = path.join(newDir, 'config.json'); - extfs.mkdirp(newDir, 493, (error) => { - callback(testFile, (callback) => extfs.del(parentDir, os.tmpdir(), () => { }, callback)); - }); + const onMkdirp = error => callback(testFile, (callback) => extfs.del(parentDir, os.tmpdir(), () => { }, callback)); + + mkdirp(newDir, 493).done(() => onMkdirp(null), error => onMkdirp(error)); } test('simple', (done: () => void) => { diff --git a/src/vs/platform/update/electron-main/auto-updater.win32.ts b/src/vs/platform/update/electron-main/auto-updater.win32.ts index 4bc3db6a823..c88880617f3 100644 --- a/src/vs/platform/update/electron-main/auto-updater.win32.ts +++ b/src/vs/platform/update/electron-main/auto-updater.win32.ts @@ -11,7 +11,6 @@ import { checksum } from 'vs/base/node/crypto'; import { EventEmitter } from 'events'; import { tmpdir } from 'os'; import { spawn } from 'child_process'; -import { mkdirp } from 'vs/base/node/extfs'; import { isString } from 'vs/base/common/types'; import { Promise, TPromise } from 'vs/base/common/winjs.base'; import { download, asJson } from 'vs/base/node/request'; @@ -42,7 +41,7 @@ export class Win32AutoUpdaterImpl extends EventEmitter implements IAutoUpdater { get cachePath(): TPromise { const result = path.join(tmpdir(), `vscode-update-${process.arch}`); - return new TPromise((c, e) => mkdirp(result, null, err => err ? e(err) : c(result))); + return pfs.mkdirp(result, null).then(() => result); } setFeedURL(url: string): void { diff --git a/src/vs/workbench/services/configuration/test/node/configurationEditingService.test.ts b/src/vs/workbench/services/configuration/test/node/configurationEditingService.test.ts index f458e603870..ffc77dc40f7 100644 --- a/src/vs/workbench/services/configuration/test/node/configurationEditingService.test.ts +++ b/src/vs/workbench/services/configuration/test/node/configurationEditingService.test.ts @@ -34,6 +34,7 @@ import { TextModelResolverService } from 'vs/workbench/services/textmodelResolve import { IChoiceService } from 'vs/platform/message/common/message'; import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; import { IWindowConfiguration } from 'vs/platform/windows/common/windows'; +import { mkdirp } from 'vs/base/node/pfs'; class SettingsTestEnvironmentService extends EnvironmentService { @@ -80,21 +81,14 @@ suite('ConfigurationEditingService', () => { .then(() => setUpServices()); }); - function setUpWorkspace(): TPromise { - return new TPromise((c, e) => { - const id = uuid.generateUuid(); - parentDir = path.join(os.tmpdir(), 'vsctests', id); - workspaceDir = path.join(parentDir, 'workspaceconfig', id); - globalSettingsFile = path.join(workspaceDir, 'config.json'); - workspaceSettingsDir = path.join(workspaceDir, '.vscode'); - extfs.mkdirp(workspaceSettingsDir, 493, (error) => { - if (error) { - e(error); - } else { - c(null); - } - }); - }); + function setUpWorkspace(): TPromise { + const id = uuid.generateUuid(); + parentDir = path.join(os.tmpdir(), 'vsctests', id); + workspaceDir = path.join(parentDir, 'workspaceconfig', id); + globalSettingsFile = path.join(workspaceDir, 'config.json'); + workspaceSettingsDir = path.join(workspaceDir, '.vscode'); + + return mkdirp(workspaceSettingsDir, 493); } function setUpServices(noWorkspace: boolean = false): TPromise { diff --git a/src/vs/workbench/services/configuration/test/node/configurationService.test.ts b/src/vs/workbench/services/configuration/test/node/configurationService.test.ts index d68c32161f0..eaa5fa0c337 100644 --- a/src/vs/workbench/services/configuration/test/node/configurationService.test.ts +++ b/src/vs/workbench/services/configuration/test/node/configurationService.test.ts @@ -34,6 +34,7 @@ import { IJSONEditingService } from 'vs/workbench/services/configuration/common/ import { JSONEditingService } from 'vs/workbench/services/configuration/node/jsonEditingService'; import { IWorkspaceConfigurationService } from 'vs/workbench/services/configuration/common/configuration'; import { IWindowConfiguration } from 'vs/platform/windows/common/windows'; +import { mkdirp } from 'vs/base/node/pfs'; class SettingsTestEnvironmentService extends EnvironmentService { @@ -53,15 +54,7 @@ function setUpFolderWorkspace(folderName: string): TPromise<{ parentDir: string, function setUpFolder(folderName: string, parentDir: string): TPromise { const folderDir = path.join(parentDir, folderName); const workspaceSettingsDir = path.join(folderDir, '.vscode'); - return new TPromise((c, e) => { - extfs.mkdirp(workspaceSettingsDir, 493, (error) => { - if (error) { - e(error); - return null; - } - c(folderDir); - }); - }); + return mkdirp(workspaceSettingsDir, 493).then(() => folderDir); } function setUpWorkspace(folders: string[]): TPromise<{ parentDir: string, configPath: string }> { @@ -69,7 +62,7 @@ function setUpWorkspace(folders: string[]): TPromise<{ parentDir: string, config const id = uuid.generateUuid(); const parentDir = path.join(os.tmpdir(), 'vsctests', id); - return createDir(parentDir) + return mkdirp(parentDir, 493) .then(() => { const configPath = path.join(parentDir, 'vsctests.code-workspace'); const workspace = { folders: folders.map(path => ({ path })) }; @@ -81,17 +74,6 @@ function setUpWorkspace(folders: string[]): TPromise<{ parentDir: string, config } -function createDir(dir: string): TPromise { - return new TPromise((c, e) => { - extfs.mkdirp(dir, 493, (error) => { - if (error) { - e(error); - return null; - } - c(null); - }); - }); -} suite('WorkspaceContextService - Folder', () => { diff --git a/src/vs/workbench/services/keybinding/test/node/keybindingEditing.test.ts b/src/vs/workbench/services/keybinding/test/node/keybindingEditing.test.ts index 5e382d10aec..6b9bbd618b5 100644 --- a/src/vs/workbench/services/keybinding/test/node/keybindingEditing.test.ts +++ b/src/vs/workbench/services/keybinding/test/node/keybindingEditing.test.ts @@ -43,6 +43,7 @@ import { IUserFriendlyKeybinding } from 'vs/platform/keybinding/common/keybindin import { ResolvedKeybindingItem } from 'vs/platform/keybinding/common/resolvedKeybindingItem'; import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; import { IHashService } from 'vs/workbench/services/hash/common/hashService'; +import { mkdirp } from 'vs/base/node/pfs'; interface Modifiers { metaKey?: boolean; @@ -87,17 +88,9 @@ suite('Keybindings Editing', () => { }); }); - function setUpWorkspace(): TPromise { - return new TPromise((c, e) => { - testDir = path.join(os.tmpdir(), 'vsctests', uuid.generateUuid()); - extfs.mkdirp(testDir, 493, (error) => { - if (error) { - e(error); - } else { - c(null); - } - }); - }); + function setUpWorkspace(): TPromise { + testDir = path.join(os.tmpdir(), 'vsctests', uuid.generateUuid()); + return mkdirp(testDir, 493); } teardown(() => { From c7c988c5af378cf4fb4de62066f3783fb5f2e57b Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 15 Nov 2017 10:28:27 +0100 Subject: [PATCH 13/19] fix #38294 --- .../workbench/api/node/extHostLanguageFeatures.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/api/node/extHostLanguageFeatures.ts b/src/vs/workbench/api/node/extHostLanguageFeatures.ts index 7ad835147e1..fe8a177ca07 100644 --- a/src/vs/workbench/api/node/extHostLanguageFeatures.ts +++ b/src/vs/workbench/api/node/extHostLanguageFeatures.ts @@ -408,11 +408,17 @@ class NavigateTypeAdapter { return asWinJsPromise(token => this._provider.provideWorkspaceSymbols(search, token)).then(value => { if (!isFalsyOrEmpty(value)) { for (const item of value) { - if (item) { - const symbol = IdObject.mixin(TypeConverters.fromSymbolInformation(item)); - this._symbolCache[symbol._id] = item; - result.symbols.push(symbol); + if (!item) { + // drop + continue; } + if (!item.name) { + console.warn('INVALID SymbolInformation, lacks name', item); + continue; + } + const symbol = IdObject.mixin(TypeConverters.fromSymbolInformation(item)); + this._symbolCache[symbol._id] = item; + result.symbols.push(symbol); } } }).then(() => { From d08a44a960d62510b4a25c83621d1362accf2d85 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Wed, 15 Nov 2017 10:49:47 +0100 Subject: [PATCH 14/19] Fix #37482 --- .../preferences/browser/preferencesEditor.ts | 144 ++++++++++++------ .../parts/preferences/common/preferences.ts | 2 + .../preferences/common/preferencesModels.ts | 8 +- 3 files changed, 109 insertions(+), 45 deletions(-) diff --git a/src/vs/workbench/parts/preferences/browser/preferencesEditor.ts b/src/vs/workbench/parts/preferences/browser/preferencesEditor.ts index 4ef60a4c0c5..5db65ebc7e5 100644 --- a/src/vs/workbench/parts/preferences/browser/preferencesEditor.ts +++ b/src/vs/workbench/parts/preferences/browser/preferencesEditor.ts @@ -30,7 +30,7 @@ import { import { SettingsEditorModel, DefaultSettingsEditorModel } from 'vs/workbench/parts/preferences/common/preferencesModels'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { SearchWidget, SettingsTargetsWidget } from 'vs/workbench/parts/preferences/browser/preferencesWidgets'; -import { PreferencesSearchProvider, PreferencesSearchModel } from 'vs/workbench/parts/preferences/browser/preferencesSearch'; +import { PreferencesSearchProvider } from 'vs/workbench/parts/preferences/browser/preferencesSearch'; import { ContextKeyExpr, IContextKeyService, IContextKey } from 'vs/platform/contextkey/common/contextkey'; import { registerEditorContribution, Command, IEditorContributionCtor } from 'vs/editor/browser/editorExtensions'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; @@ -161,7 +161,7 @@ export class PreferencesEditor extends BaseEditor { this.sideBySidePreferencesWidget = this._register(this.instantiationService.createInstance(SideBySidePreferencesWidget, editorsContainer)); this._register(this.sideBySidePreferencesWidget.onFocus(() => this.lastFocusedWidget = this.sideBySidePreferencesWidget)); - this.preferencesRenderers = this._register(new PreferencesRenderers()); + this.preferencesRenderers = this._register(new PreferencesRenderers(this.searchProvider)); this._register(this.workspaceContextService.onDidChangeWorkspaceFolders(() => this.onWorkspaceFoldersChanged())); this._register(this.workspaceContextService.onDidChangeWorkbenchState(() => this.onWorkbenchStateChanged())); @@ -169,6 +169,8 @@ export class PreferencesEditor extends BaseEditor { this.searchWidget.fuzzyEnabled = true; this.filterPreferences(); })); + + this._register(this.preferencesRenderers.onDidFilterResultsCountChange(count => this.showSearchResultsMessage(count))); } public clearSearchResults(): void { @@ -331,11 +333,9 @@ export class PreferencesEditor extends BaseEditor { private filterPreferences(): TPromise { this.memento['fuzzyEnabled'] = this.searchWidget.fuzzyEnabled; const filter = this.searchWidget.getValue().trim(); - return this.preferencesRenderers.filterPreferences(filter, this.searchProvider, this.searchWidget.fuzzyEnabled).then(result => { - const count = result.count; - const message = filter ? this.showSearchResultsMessage(count) : nls.localize('totalSettingsMessage', "Total {0} Settings", count); - this.searchWidget.showMessage(message, count); - if (count === 0) { + return this.preferencesRenderers.filterPreferences({ filter, fuzzy: this.searchWidget.fuzzyEnabled }).then(result => { + this.showSearchResultsMessage(result.count); + if (result.count === 0) { this.latestEmptyFilters.push(filter); } @@ -343,10 +343,18 @@ export class PreferencesEditor extends BaseEditor { }, onUnexpectedError); } - private showSearchResultsMessage(count: number): string { - return count === 0 ? nls.localize('noSettingsFound', "No Results") : - count === 1 ? nls.localize('oneSettingFound', "1 Setting matched") : - nls.localize('settingsFound', "{0} Settings matched", count); + private showSearchResultsMessage(count: number): void { + if (this.searchWidget.getValue()) { + if (count === 0) { + this.searchWidget.showMessage(nls.localize('noSettingsFound', "No Results"), count); + } else if (count === 1) { + this.searchWidget.showMessage(nls.localize('oneSettingFound', "1 Setting matched"), count); + } else { + this.searchWidget.showMessage(nls.localize('settingsFound', "{0} Settings matched", count), count); + } + } else { + this.searchWidget.showMessage(nls.localize('totalSettingsMessage', "Total {0} Settings", count), count); + } } private reportFilteringUsed(filter: string, metadata?: IFilterMetadata): void { @@ -414,32 +422,52 @@ class SettingsNavigator implements INavigator { } } +interface ISearchCriteria { + filter: string; + fuzzy: boolean; +} + class PreferencesRenderers extends Disposable { private _defaultPreferencesRenderer: IPreferencesRenderer; + private _defaultPreferencesRendererDisposables: IDisposable[] = []; + + private _defaultPreferencesFilterResult: IFilterResult; + private _editablePreferencesFilterResult: IFilterResult; + private _editablePreferencesRenderer: IPreferencesRenderer; + private _editablePreferencesRendererDisposables: IDisposable[] = []; + private _settingsNavigator: SettingsNavigator; private _filtersInProgress: TPromise[]; + private _searchCriteria: ISearchCriteria; - private _disposables: IDisposable[] = []; - - private _onTriggeredFuzzy: Emitter = new Emitter(); + private _onTriggeredFuzzy: Emitter = this._register(new Emitter()); public onTriggeredFuzzy: Event = this._onTriggeredFuzzy.event; - public get defaultPreferencesRenderer(): IPreferencesRenderer { + private _onDidFilterResultsCountChange: Emitter = this._register(new Emitter()); + public onDidFilterResultsCountChange: Event = this._onDidFilterResultsCountChange.event; + + constructor( + private searchProvider: PreferencesSearchProvider + ) { + super(); + } + + get defaultPreferencesRenderer(): IPreferencesRenderer { return this._defaultPreferencesRenderer; } - public set defaultPreferencesRenderer(defaultPreferencesRenderer: IPreferencesRenderer) { + set defaultPreferencesRenderer(defaultPreferencesRenderer: IPreferencesRenderer) { if (this._defaultPreferencesRenderer !== defaultPreferencesRenderer) { this._defaultPreferencesRenderer = defaultPreferencesRenderer; - this._disposables = dispose(this._disposables); + this._defaultPreferencesRendererDisposables = dispose(this._defaultPreferencesRendererDisposables); if (this._defaultPreferencesRenderer) { - this._defaultPreferencesRenderer.onUpdatePreference(({ key, value, source, index }) => this._updatePreference(key, value, source, index, this._editablePreferencesRenderer), this, this._disposables); - this._defaultPreferencesRenderer.onFocusPreference(preference => this._focusPreference(preference, this._editablePreferencesRenderer), this, this._disposables); - this._defaultPreferencesRenderer.onClearFocusPreference(preference => this._clearFocus(preference, this._editablePreferencesRenderer), this, this._disposables); + this._defaultPreferencesRenderer.onUpdatePreference(({ key, value, source, index }) => this._updatePreference(key, value, source, index, this._editablePreferencesRenderer), this, this._defaultPreferencesRendererDisposables); + this._defaultPreferencesRenderer.onFocusPreference(preference => this._focusPreference(preference, this._editablePreferencesRenderer), this, this._defaultPreferencesRendererDisposables); + this._defaultPreferencesRenderer.onClearFocusPreference(preference => this._clearFocus(preference, this._editablePreferencesRenderer), this, this._defaultPreferencesRendererDisposables); if (this._defaultPreferencesRenderer.onTriggeredFuzzy) { this._register(this._defaultPreferencesRenderer.onTriggeredFuzzy(() => this._onTriggeredFuzzy.fire())); } @@ -447,37 +475,39 @@ class PreferencesRenderers extends Disposable { } } - public set editablePreferencesRenderer(editableSettingsRenderer: IPreferencesRenderer) { - this._editablePreferencesRenderer = editableSettingsRenderer; + set editablePreferencesRenderer(editableSettingsRenderer: IPreferencesRenderer) { + if (this._editablePreferencesRenderer !== editableSettingsRenderer) { + this._editablePreferencesRenderer = editableSettingsRenderer; + this._editablePreferencesRendererDisposables = dispose(this._editablePreferencesRendererDisposables); + if (this._editablePreferencesRenderer) { + (this._editablePreferencesRenderer.preferencesModel).onDidChangeGroups(() => { + this._filterEditablePreferences() + .then(() => { + const count = this.consolidateAndUpdate(); + this._onDidFilterResultsCountChange.fire(count); + }); + }, this, this._editablePreferencesRendererDisposables); + } + } } - public filterPreferences(filter: string, searchProvider: PreferencesSearchProvider, fuzzy: boolean): TPromise<{ count: number, metadata: IFilterMetadata }> { + filterPreferences(criteria: ISearchCriteria): TPromise<{ count: number, metadata: IFilterMetadata }> { + this._searchCriteria = criteria; + if (this._filtersInProgress) { // Resolved/rejected promises have no .cancel() this._filtersInProgress.forEach(p => p.cancel && p.cancel()); } - const searchModel = searchProvider.startSearch(filter, fuzzy); - this._filtersInProgress = [ - this._filterPreferences(searchModel, searchProvider, this._defaultPreferencesRenderer), - this._filterPreferences(searchModel, searchProvider, this._editablePreferencesRenderer)]; + this._filtersInProgress = [this._filterDefaultPreferences(), this._filterEditablePreferences()]; - return TPromise.join(this._filtersInProgress).then(filterResults => { - this._filtersInProgress = null; - const defaultPreferencesFilterResult = filterResults[0]; - const editablePreferencesFilterResult = filterResults[1]; - - const defaultPreferencesFilteredGroups = defaultPreferencesFilterResult ? defaultPreferencesFilterResult.filteredGroups : this._getAllPreferences(this._defaultPreferencesRenderer); - const editablePreferencesFilteredGroups = editablePreferencesFilterResult ? editablePreferencesFilterResult.filteredGroups : this._getAllPreferences(this._editablePreferencesRenderer); - const consolidatedSettings = this._consolidateSettings(editablePreferencesFilteredGroups, defaultPreferencesFilteredGroups); - - this._settingsNavigator = new SettingsNavigator(filter ? consolidatedSettings : []); - - return { count: consolidatedSettings.length, metadata: defaultPreferencesFilterResult && defaultPreferencesFilterResult.metadata }; + return TPromise.join(this._filtersInProgress).then(() => { + const count = this.consolidateAndUpdate(); + return { count, metadata: this._defaultPreferencesFilterResult && this._defaultPreferencesFilterResult.metadata }; }); } - public focusNextPreference(forward: boolean = true) { + focusNextPreference(forward: boolean = true) { if (!this._settingsNavigator) { return; } @@ -487,21 +517,46 @@ class PreferencesRenderers extends Disposable { this._focusPreference(setting, this._editablePreferencesRenderer); } + private _filterDefaultPreferences(): TPromise { + if (this._searchCriteria && this._defaultPreferencesRenderer) { + return this._filterPreferences(this._searchCriteria, this._defaultPreferencesRenderer) + .then(filterResult => { this._defaultPreferencesFilterResult = filterResult; }); + } + return TPromise.wrap(null); + } + + private _filterEditablePreferences(): TPromise { + if (this._searchCriteria && this._editablePreferencesRenderer) { + return this._filterPreferences({ filter: this._searchCriteria.filter, fuzzy: false }, this._editablePreferencesRenderer) + .then(filterResult => { this._editablePreferencesFilterResult = filterResult; }); + } + return TPromise.wrap(null); + } + private _getAllPreferences(preferencesRenderer: IPreferencesRenderer): ISettingsGroup[] { return preferencesRenderer ? (preferencesRenderer.preferencesModel).settingsGroups : []; } - private _filterPreferences(searchModel: PreferencesSearchModel, searchProvider: PreferencesSearchProvider, preferencesRenderer: IPreferencesRenderer): TPromise { + private _filterPreferences(searchCriteria: ISearchCriteria, preferencesRenderer: IPreferencesRenderer): TPromise { if (preferencesRenderer) { + const searchModel = this.searchProvider.startSearch(searchCriteria.filter, searchCriteria.fuzzy); const prefSearchP = searchModel.filterPreferences(preferencesRenderer.preferencesModel); return prefSearchP.then(filterResult => { - preferencesRenderer.filterPreferences(filterResult, searchProvider.remoteSearchEnabled); + preferencesRenderer.filterPreferences(filterResult, this.searchProvider.remoteSearchEnabled); return filterResult; }); } + return TPromise.as(null); + } - return TPromise.wrap(null); + private consolidateAndUpdate(): number { + const defaultPreferencesFilteredGroups = this._defaultPreferencesFilterResult ? this._defaultPreferencesFilterResult.filteredGroups : this._getAllPreferences(this._defaultPreferencesRenderer); + const editablePreferencesFilteredGroups = this._editablePreferencesFilterResult ? this._editablePreferencesFilterResult.filteredGroups : this._getAllPreferences(this._editablePreferencesRenderer); + const consolidatedSettings = this._consolidateSettings(editablePreferencesFilteredGroups, defaultPreferencesFilteredGroups); + + this._settingsNavigator = new SettingsNavigator(this._searchCriteria.filter ? consolidatedSettings : []); + return consolidatedSettings.length; } private _focusPreference(preference: ISetting, preferencesRenderer: IPreferencesRenderer): void { @@ -539,7 +594,8 @@ class PreferencesRenderers extends Disposable { } public dispose(): void { - dispose(this._disposables); + dispose(this._defaultPreferencesRendererDisposables); + dispose(this._editablePreferencesRendererDisposables); super.dispose(); } } diff --git a/src/vs/workbench/parts/preferences/common/preferences.ts b/src/vs/workbench/parts/preferences/common/preferences.ts index 1490704ca4a..95ce9e9b8ec 100644 --- a/src/vs/workbench/parts/preferences/common/preferences.ts +++ b/src/vs/workbench/parts/preferences/common/preferences.ts @@ -15,6 +15,7 @@ import { IRange } from 'vs/editor/common/core/range'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; import { join } from 'vs/base/common/paths'; import { ConfigurationTarget } from 'vs/platform/configuration/common/configuration'; +import Event from 'vs/base/common/event'; export interface IWorkbenchSettingsConfiguration { workbench: { @@ -84,6 +85,7 @@ export type IGroupFilter = (group: ISettingsGroup) => boolean; export type ISettingFilter = (setting: ISetting) => IRange[]; export interface ISettingsEditorModel extends IPreferencesEditorModel { + readonly onDidChangeGroups: Event; settingsGroups: ISettingsGroup[]; groupsTerms: string[]; filterSettings(filter: string, groupFilter: IGroupFilter, settingFilter: ISettingFilter, mostRelevantSettings?: string[]): IFilterResult; diff --git a/src/vs/workbench/parts/preferences/common/preferencesModels.ts b/src/vs/workbench/parts/preferences/common/preferencesModels.ts index 4c650a8c81b..17d666feae1 100644 --- a/src/vs/workbench/parts/preferences/common/preferencesModels.ts +++ b/src/vs/workbench/parts/preferences/common/preferencesModels.ts @@ -122,11 +122,17 @@ export class SettingsEditorModel extends AbstractSettingsModel implements ISetti protected settingsModel: IModel; private queue: Queue; + private _onDidChangeGroups: Emitter = this._register(new Emitter()); + readonly onDidChangeGroups: Event = this._onDidChangeGroups.event; + constructor(reference: IReference, private _configurationTarget: ConfigurationTarget, @ITextFileService protected textFileService: ITextFileService) { super(); this.settingsModel = reference.object.textEditorModel; this._register(this.onDispose(() => reference.dispose())); - this._register(this.settingsModel.onDidChangeContent(() => this._settingsGroups = null)); + this._register(this.settingsModel.onDidChangeContent(() => { + this._settingsGroups = null; + this._onDidChangeGroups.fire(); + })); this.queue = new Queue(); } From 68c6182b83f7e4ae829fa6f9f86cccf22ea156f3 Mon Sep 17 00:00:00 2001 From: Martin Aeschlimann Date: Wed, 15 Nov 2017 11:15:41 +0100 Subject: [PATCH 15/19] clean up test scripts, bring back html server tests --- scripts/{test-mocha.bat => node-electron.bat} | 9 +-------- scripts/{test-mocha.sh => node-electron.sh} | 10 ++-------- scripts/test-int-mocha.bat | 1 - scripts/test-int-mocha.sh | 1 - scripts/test-integration.bat | 9 +++++++-- scripts/test-integration.sh | 11 ++++++++--- test/mocha.opts | 4 +--- 7 files changed, 19 insertions(+), 26 deletions(-) rename scripts/{test-mocha.bat => node-electron.bat} (61%) rename scripts/{test-mocha.sh => node-electron.sh} (81%) delete mode 100644 scripts/test-int-mocha.bat delete mode 100755 scripts/test-int-mocha.sh diff --git a/scripts/test-mocha.bat b/scripts/node-electron.bat similarity index 61% rename from scripts/test-mocha.bat rename to scripts/node-electron.bat index e1aa4bef47f..4ddb95b3cae 100644 --- a/scripts/test-mocha.bat +++ b/scripts/node-electron.bat @@ -10,15 +10,8 @@ set NAMESHORT=%NAMESHORT: "=% set NAMESHORT=%NAMESHORT:"=%.exe set CODE=".build\electron\%NAMESHORT%" -rem TFS Builds -if not "%BUILD_BUILDID%" == "" ( - %CODE% .\node_modules\mocha\bin\_mocha %* -) +%CODE% %* -rem Otherwise -if "%BUILD_BUILDID%" == "" ( - %CODE% .\node_modules\mocha\bin\_mocha --reporter dot %* -) popd endlocal diff --git a/scripts/test-mocha.sh b/scripts/node-electron.sh similarity index 81% rename from scripts/test-mocha.sh rename to scripts/node-electron.sh index 9aa16fa3241..a1c2cf469b0 100755 --- a/scripts/test-mocha.sh +++ b/scripts/node-electron.sh @@ -20,23 +20,17 @@ fi INTENDED_VERSION="v`node -p "require('./package.json').electronVersion"`" INSTALLED_VERSION=$(cat .build/electron/version 2> /dev/null) -# Node modules -test -d node_modules || ./scripts/npm.sh install # Get electron (test -f "$CODE" && [ $INTENDED_VERSION == $INSTALLED_VERSION ]) || ./node_modules/.bin/gulp electron -# Build -test -d out || ./node_modules/.bin/gulp compile - -# Unit Tests export VSCODE_DEV=1 if [[ "$OSTYPE" == "darwin"* ]]; then cd $ROOT ; ulimit -n 4096 ; ELECTRON_RUN_AS_NODE=1 \ "$CODE" \ - node_modules/mocha/bin/_mocha "$@" + "$@" else cd $ROOT ; ELECTRON_RUN_AS_NODE=1 \ "$CODE" \ - node_modules/mocha/bin/_mocha "$@" + "$@" fi diff --git a/scripts/test-int-mocha.bat b/scripts/test-int-mocha.bat deleted file mode 100644 index 482aadc5bd5..00000000000 --- a/scripts/test-int-mocha.bat +++ /dev/null @@ -1 +0,0 @@ -.\scripts\test.bat --runGlob **\*.integrationTest.js -g integration %* diff --git a/scripts/test-int-mocha.sh b/scripts/test-int-mocha.sh deleted file mode 100755 index c0a8913886a..00000000000 --- a/scripts/test-int-mocha.sh +++ /dev/null @@ -1 +0,0 @@ -./scripts/test.sh --runGlob **/*.integrationTest.js -g integration "$@" \ No newline at end of file diff --git a/scripts/test-integration.bat b/scripts/test-integration.bat index 0a2b92e00d7..3c807be78ae 100644 --- a/scripts/test-integration.bat +++ b/scripts/test-integration.bat @@ -8,12 +8,17 @@ if not "%APPVEYOR%" == "" ( ) set VSCODEUSERDATADIR=%TMP%\vscodeuserfolder-%RANDOM%-%TIME:~6,5% -:: Integration Tests +:: Tests in the extension host .\scripts\code.bat %~dp0\..\extensions\vscode-api-tests\testWorkspace --extensionDevelopmentPath=%~dp0\..\extensions\vscode-api-tests --extensionTestsPath=%~dp0\..\extensions\vscode-api-tests\out --disableExtensions --user-data-dir=%VSCODEUSERDATADIR% .\scripts\code.bat %~dp0\..\extensions\vscode-colorize-tests\test --extensionDevelopmentPath=%~dp0\..\extensions\vscode-colorize-tests --extensionTestsPath=%~dp0\..\extensions\vscode-colorize-tests\out --user-data-dir=%VSCODEUSERDATADIR% -.\scripts\test-int-mocha.bat .\scripts\code.bat $%~dp0\..\extensions\emmet\test-fixtures --extensionDevelopmentPath=%~dp0\..\extensions\emmet --extensionTestsPath=%~dp0\..\extensions\emmet\out\test --disableExtensions --user-data-dir=%VSCODEUSERDATADIR% +:: Integration & performance tests in AMD +.\scripts\test.bat --runGlob **\*.integrationTest.js %* + +:: Tests in commonJS (language servers tests...) +.\scripts\mocha-electron.bat .\extensions\html\server\out\test\ + rmdir /s /q %VSCODEUSERDATADIR% popd diff --git a/scripts/test-integration.sh b/scripts/test-integration.sh index 0a9eea1b077..911b411015f 100755 --- a/scripts/test-integration.sh +++ b/scripts/test-integration.sh @@ -12,10 +12,15 @@ fi cd $ROOT -# Integration Tests +# Tests in the extension host ./scripts/code.sh $ROOT/extensions/vscode-api-tests/testWorkspace --extensionDevelopmentPath=$ROOT/extensions/vscode-api-tests --extensionTestsPath=$ROOT/extensions/vscode-api-tests/out --disableExtensions --user-data-dir=$VSCODEUSERDATADIR --skip-getting-started ./scripts/code.sh $ROOT/extensions/vscode-colorize-tests/test --extensionDevelopmentPath=$ROOT/extensions/vscode-colorize-tests --extensionTestsPath=$ROOT/extensions/vscode-colorize-tests/out --user-data-dir=$VSCODEUSERDATADIR --skip-getting-started -./scripts/test-int-mocha.sh -./scripts/code.sh $ROOT/extensions/emmet/test-fixtures --extensionDevelopmentPath=$ROOT/extensions/emmet --extensionTestsPath=$ROOT/extensions/emmet/out/test --disableExtensions --user-data-dir=$VSCODEUSERDATADIR --skip-getting-started +#./scripts/code.sh $ROOT/extensions/emmet/test-fixtures --extensionDevelopmentPath=$ROOT/extensions/emmet --extensionTestsPath=$ROOT/extensions/emmet/out/test --disableExtensions --user-data-dir=$VSCODEUSERDATADIR --skip-getting-started + +# Integration tests in AMD +./scripts/test.sh --runGlob **/*.integrationTest.js "$@" + +# Tests in commonJS (language server tests...) +./scripts/node-electron.sh ./node_modules/mocha/bin/_mocha ./extensions/html/server/out/test/ rm -r $VSCODEUSERDATADIR diff --git a/test/mocha.opts b/test/mocha.opts index d6b09ce6e07..52e4348c3ed 100644 --- a/test/mocha.opts +++ b/test/mocha.opts @@ -1,4 +1,2 @@ ---delay --ui tdd ---timeout 10000 -test/all.js +--timeout 10000 \ No newline at end of file From 81427242c5be6557be601a3f216b9a5718f98cbb Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Wed, 15 Nov 2017 11:29:53 +0100 Subject: [PATCH 16/19] perf - prevent 2 page layouts in one method (for #34745) --- .../workbench/browser/parts/editor/tabsTitleControl.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts b/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts index 6d1714379a5..c67400b8958 100644 --- a/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts +++ b/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts @@ -549,6 +549,14 @@ export class TabsTitleControl extends TitleControl { const visibleContainerWidth = this.tabsContainer.offsetWidth; const totalContainerWidth = this.tabsContainer.scrollWidth; + let activeTabPosX: number; + let activeTabWidth: number; + + if (!this.blockRevealActiveTab) { + activeTabPosX = this.activeTab.offsetLeft; + activeTabWidth = this.activeTab.offsetWidth; + } + // Update scrollbar this.scrollbar.setScrollDimensions({ width: visibleContainerWidth, @@ -563,8 +571,6 @@ export class TabsTitleControl extends TitleControl { // Reveal the active one const containerScrollPosX = this.scrollbar.getScrollPosition().scrollLeft; - const activeTabPosX = this.activeTab.offsetLeft; - const activeTabWidth = this.activeTab.offsetWidth; const activeTabFits = activeTabWidth <= visibleContainerWidth; // Tab is overflowing to the right: Scroll minimally until the element is fully visible to the right From f54d16c464ee292a55de7f1f241e2e163830b08e Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Wed, 15 Nov 2017 11:38:47 +0100 Subject: [PATCH 17/19] perf - schedule tabs layout at next animation frame (for #34745) --- .../browser/parts/editor/tabsTitleControl.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts b/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts index c67400b8958..d2e91ffa84d 100644 --- a/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts +++ b/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts @@ -45,6 +45,7 @@ import { activeContrastBorder, contrastBorder } from 'vs/platform/theme/common/c import { IFileService } from 'vs/platform/files/common/files'; import { IWorkspacesService } from 'vs/platform/workspaces/common/workspaces'; import { Dimension } from 'vs/base/browser/builder'; +import { scheduleAtNextAnimationFrame } from 'vs/base/browser/dom'; interface IEditorInputLabel { name: string; @@ -64,6 +65,7 @@ export class TabsTitleControl extends TitleControl { private tabDisposeables: IDisposable[]; private blockRevealActiveTab: boolean; private dimension: Dimension; + private layoutScheduled: IDisposable; constructor( @IContextMenuService contextMenuService: IContextMenuService, @@ -546,6 +548,18 @@ export class TabsTitleControl extends TitleControl { this.dimension = dimension; + // The layout of tabs can be an expensive operation because we access DOM properties + // that can result in the browser doing a full page layout to validate them. To buffer + // this a little bit we try at least to schedule this work on the next animation frame. + if (!this.layoutScheduled) { + this.layoutScheduled = scheduleAtNextAnimationFrame(() => { + this.doLayout(this.dimension); + this.layoutScheduled = void 0; + }); + } + } + + private doLayout(dimension: Dimension): void { const visibleContainerWidth = this.tabsContainer.offsetWidth; const totalContainerWidth = this.tabsContainer.scrollWidth; @@ -864,6 +878,12 @@ export class TabsTitleControl extends TitleControl { return !isCopy || source.id === target.id; } + + public dispose(): void { + super.dispose(); + + this.layoutScheduled = dispose(this.layoutScheduled); + } } class TabActionRunner extends ActionRunner { From d5d1e76155342eefc6b8fdc3de0a9c7167bdc67a Mon Sep 17 00:00:00 2001 From: Martin Aeschlimann Date: Wed, 15 Nov 2017 11:48:41 +0100 Subject: [PATCH 18/19] fix test-integration --- scripts/test-integration.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/test-integration.sh b/scripts/test-integration.sh index 911b411015f..6159dc88aa9 100755 --- a/scripts/test-integration.sh +++ b/scripts/test-integration.sh @@ -14,8 +14,8 @@ cd $ROOT # Tests in the extension host ./scripts/code.sh $ROOT/extensions/vscode-api-tests/testWorkspace --extensionDevelopmentPath=$ROOT/extensions/vscode-api-tests --extensionTestsPath=$ROOT/extensions/vscode-api-tests/out --disableExtensions --user-data-dir=$VSCODEUSERDATADIR --skip-getting-started -./scripts/code.sh $ROOT/extensions/vscode-colorize-tests/test --extensionDevelopmentPath=$ROOT/extensions/vscode-colorize-tests --extensionTestsPath=$ROOT/extensions/vscode-colorize-tests/out --user-data-dir=$VSCODEUSERDATADIR --skip-getting-started -#./scripts/code.sh $ROOT/extensions/emmet/test-fixtures --extensionDevelopmentPath=$ROOT/extensions/emmet --extensionTestsPath=$ROOT/extensions/emmet/out/test --disableExtensions --user-data-dir=$VSCODEUSERDATADIR --skip-getting-started +./scripts/code.sh $ROOT/extensions/vscode-colorize-tests/test --extensionDevelopmentPath=$ROOT/extensions/vscode-colorize-tests --extensionTestsPath=$ROOT/extensions/vscode-colorize-tests/out --disableExtensions --user-data-dir=$VSCODEUSERDATADIR --skip-getting-started +./scripts/code.sh $ROOT/extensions/emmet/test-fixtures --extensionDevelopmentPath=$ROOT/extensions/emmet --extensionTestsPath=$ROOT/extensions/emmet/out/test --disableExtensions --user-data-dir=$VSCODEUSERDATADIR --skip-getting-started # Integration tests in AMD ./scripts/test.sh --runGlob **/*.integrationTest.js "$@" From dd6dbe86b4cedf5278f756849da0315d3e2a54ae Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Wed, 15 Nov 2017 12:00:07 +0100 Subject: [PATCH 19/19] Trigger default settings change only after updating the model --- .../parts/preferences/browser/preferencesService.ts | 1 + .../workbench/parts/preferences/common/preferencesModels.ts | 6 +----- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/parts/preferences/browser/preferencesService.ts b/src/vs/workbench/parts/preferences/browser/preferencesService.ts index 19bb214dc23..e6eba1092eb 100644 --- a/src/vs/workbench/parts/preferences/browser/preferencesService.ts +++ b/src/vs/workbench/parts/preferences/browser/preferencesService.ts @@ -123,6 +123,7 @@ export class PreferencesService extends Disposable implements IPreferencesServic } defaultSettings = this.getDefaultSettings(scope); this.modelService.updateModel(model, defaultSettings.parse()); + defaultSettings._onDidChange.fire(); } }); diff --git a/src/vs/workbench/parts/preferences/common/preferencesModels.ts b/src/vs/workbench/parts/preferences/common/preferencesModels.ts index 17d666feae1..064fac4acdf 100644 --- a/src/vs/workbench/parts/preferences/common/preferencesModels.ts +++ b/src/vs/workbench/parts/preferences/common/preferencesModels.ts @@ -511,7 +511,7 @@ export class DefaultSettings extends Disposable { private _content: string; private _settingsByName: Map; - private _onDidChange: Emitter = this._register(new Emitter()); + readonly _onDidChange: Emitter = this._register(new Emitter()); readonly onDidChange: Event = this._onDidChange.event; constructor( @@ -536,16 +536,12 @@ export class DefaultSettings extends Disposable { } parse(): string { - const currentContent = this._content; const configurations = Registry.as(Extensions.Configuration).getConfigurations().slice(); const settingsGroups = this.removeEmptySettingsGroups(configurations.sort(this.compareConfigurationNodes).reduce((result, config, index, array) => this.parseConfig(config, result, array), [])); this.initAllSettingsMap(settingsGroups); const mostCommonlyUsed = this.getMostCommonlyUsedSettings(settingsGroups); this._allSettingsGroups = [mostCommonlyUsed, ...settingsGroups]; this._content = this.toContent(mostCommonlyUsed, settingsGroups); - if (this._content !== currentContent) { - this._onDidChange.fire(); - } return this._content; }