quick access - drop default handler

src/vs/editor/standalone/browser/quickAccess/standaloneHelpQuickAccess.ts

@@ -8,8 +8,8 @@ import { IQuickAccessRegistry, Extensions } from 'vs/platform/quickinput/common/
 import { QuickHelpNLS } from 'vs/editor/common/standaloneStrings';
 import { HelpQuickAccessProvider } from 'vs/platform/quickinput/browser/helpQuickAccess';
 
-Registry.as<IQuickAccessRegistry>(Extensions.Quickaccess).defaultProvider = {
+Registry.as<IQuickAccessRegistry>(Extensions.Quickaccess).registerQuickAccessProvider({
 	ctor: HelpQuickAccessProvider,
 	prefix: '',
 	helpEntries: [{ description: QuickHelpNLS.helpQuickAccessActionLabel, needsEditor: true }]
-};
+});

src/vs/platform/quickinput/browser/quickAccess.ts

@@ -37,11 +37,11 @@ export class QuickAccessController extends Disposable implements IQuickAccessCon
 		// Create a picker for the provider to use with the initial value
 		// and adjust the filtering to exclude the prefix from filtering
 		const picker = disposables.add(this.quickInputService.createQuickPick());
-		picker.placeholder = descriptor.placeholder;
+		picker.placeholder = descriptor?.placeholder;
 		picker.value = value;
 		picker.valueSelection = [value.length, value.length];
-		picker.contextKey = descriptor.contextKey;
-		picker.filterValue = (value: string) => value.substring(descriptor.prefix.length);
+		picker.contextKey = descriptor?.contextKey;
+		picker.filterValue = (value: string) => value.substring(descriptor ? descriptor.prefix.length : 0);
 
 		// Remember as last active picker and clean up once picker get's disposed
 		this.lastActivePicker = picker;
@@ -72,8 +72,10 @@ export class QuickAccessController extends Disposable implements IQuickAccessCon
 			}
 		}));
 
-		// Ask provider to fill the picker as needed
-		disposables.add(provider.provide(picker, cts.token));
+		// Ask provider to fill the picker as needed if we have one
+		if (provider) {
+			disposables.add(provider.provide(picker, cts.token));
+		}
 
 		// Finally, show the picker. This is important because a provider
 		// may not call this and then our disposables would leak that rely
@@ -81,8 +83,11 @@ export class QuickAccessController extends Disposable implements IQuickAccessCon
 		picker.show();
 	}
 
-	private getOrInstantiateProvider(value: string): [IQuickAccessProvider, IQuickAccessProviderDescriptor] {
-		const providerDescriptor = this.registry.getQuickAccessProvider(value) || this.registry.defaultProvider;
+	private getOrInstantiateProvider(value: string): [IQuickAccessProvider | undefined, IQuickAccessProviderDescriptor | undefined] {
+		const providerDescriptor = this.registry.getQuickAccessProvider(value);
+		if (!providerDescriptor) {
+			return [undefined, undefined];
+		}
 
 		let provider = this.mapProviderToDescriptor.get(providerDescriptor);
 		if (!provider) {

src/vs/platform/quickinput/common/quickAccess.ts

@@ -6,9 +6,8 @@
 import { IQuickPick, IQuickPickItem } from 'vs/platform/quickinput/common/quickInput';
 import { CancellationToken } from 'vs/base/common/cancellation';
 import { Registry } from 'vs/platform/registry/common/platform';
-import { first } from 'vs/base/common/arrays';
+import { first, coalesce } from 'vs/base/common/arrays';
 import { startsWith } from 'vs/base/common/strings';
-import { assertIsDefined } from 'vs/base/common/types';
 import { IDisposable, toDisposable } from 'vs/base/common/lifecycle';
 
 export interface IQuickAccessController {
@@ -92,11 +91,6 @@ export const Extensions = {
 
 export interface IQuickAccessRegistry {
 
-	/**
-	 * The default provider to use when no other provider matches.
-	 */
-	defaultProvider: IQuickAccessProviderDescriptor;
-
 	/**
 	 * Registers a quick access provider to the platform.
 	 */
@@ -113,29 +107,53 @@ export interface IQuickAccessRegistry {
 	getQuickAccessProvider(prefix: string): IQuickAccessProviderDescriptor | undefined;
 }
 
-class QuickAccessRegistry implements IQuickAccessRegistry {
+export class QuickAccessRegistry implements IQuickAccessRegistry {
 	private providers: IQuickAccessProviderDescriptor[] = [];
-
-	private _defaultProvider: IQuickAccessProviderDescriptor | undefined = undefined;
-	get defaultProvider(): IQuickAccessProviderDescriptor { return assertIsDefined(this._defaultProvider); }
-	set defaultProvider(provider: IQuickAccessProviderDescriptor) { this._defaultProvider = provider; }
+	private defaultProvider: IQuickAccessProviderDescriptor | undefined = undefined;
 
 	registerQuickAccessProvider(provider: IQuickAccessProviderDescriptor): IDisposable {
-		this.providers.push(provider);
+
+		// Extract the default provider when no prefix is present
+		if (provider.prefix.length === 0) {
+			this.defaultProvider = provider;
+		} else {
+			this.providers.push(provider);
+		}
 
 		// sort the providers by decreasing prefix length, such that longer
 		// prefixes take priority: 'ext' vs 'ext install' - the latter should win
 		this.providers.sort((providerA, providerB) => providerB.prefix.length - providerA.prefix.length);
 
-		return toDisposable(() => this.providers.splice(this.providers.indexOf(provider), 1));
+		return toDisposable(() => {
+			this.providers.splice(this.providers.indexOf(provider), 1);
+
+			if (this.defaultProvider === provider) {
+				this.defaultProvider = undefined;
+			}
+		});
 	}
 
 	getQuickAccessProviders(): IQuickAccessProviderDescriptor[] {
-		return [this.defaultProvider, ...this.providers];
+		return coalesce([this.defaultProvider, ...this.providers]);
 	}
 
 	getQuickAccessProvider(prefix: string): IQuickAccessProviderDescriptor | undefined {
-		return prefix ? (first(this.providers, provider => startsWith(prefix, provider.prefix)) || undefined) : undefined;
+		const result = prefix ? (first(this.providers, provider => startsWith(prefix, provider.prefix)) || undefined) : undefined;
+
+		return result || this.defaultProvider;
+	}
+
+	clear(): Function {
+		const providers = [...this.providers];
+		const defaultProvider = this.defaultProvider;
+
+		this.providers = [];
+		this.defaultProvider = undefined;
+
+		return () => {
+			this.providers = providers;
+			this.defaultProvider = defaultProvider;
+		};
 	}
 }
 

src/vs/workbench/contrib/quickaccess/browser/quickAccess.contribution.ts

@@ -15,13 +15,6 @@ import { KeybindingsRegistry, KeybindingWeight } from 'vs/platform/keybinding/co
 
 const registry = Registry.as<IQuickAccessRegistry>(Extensions.Quickaccess);
 
-registry.defaultProvider = {
-	ctor: HelpQuickAccessProvider,
-	prefix: '',
-	placeholder: localize('defaultAccessPlaceholder', "Type the name of a file to open."),
-	helpEntries: [{ description: localize('gotoFileQuickAccess', "Go to File"), needsEditor: false }]
-};
-
 registry.registerQuickAccessProvider({
 	ctor: HelpQuickAccessProvider,
 	prefix: HelpQuickAccessProvider.PREFIX,

src/vs/workbench/test/browser/quickAccess.test.ts

@@ -5,7 +5,7 @@
 
 import * as assert from 'assert';
 import { Registry } from 'vs/platform/registry/common/platform';
-import { IQuickAccessRegistry, Extensions, IQuickAccessProvider } from 'vs/platform/quickinput/common/quickAccess';
+import { IQuickAccessRegistry, Extensions, IQuickAccessProvider, QuickAccessRegistry } from 'vs/platform/quickinput/common/quickAccess';
 import { IQuickPick, IQuickPickItem, IQuickInputService } from 'vs/platform/quickinput/common/quickInput';
 import { CancellationToken } from 'vs/base/common/cancellation';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
@@ -18,6 +18,10 @@ suite('QuickAccess', () => {
 	let instantiationService: IInstantiationService;
 	let accessor: TestServiceAccessor;
 
+	let providerDefaultCalled = false;
+	let providerDefaultCanceled = false;
+	let providerDefaultDisposed = false;
+
 	let provider1Called = false;
 	let provider1Canceled = false;
 	let provider1Disposed = false;
@@ -30,9 +34,21 @@ suite('QuickAccess', () => {
 	let provider3Canceled = false;
 	let provider3Disposed = false;
 
-	let provider4Called = false;
-	let provider4Canceled = false;
-	let provider4Disposed = false;
+	class TestProviderDefault implements IQuickAccessProvider {
+
+		constructor(@IQuickInputService private readonly quickInputService: IQuickInputService, disposables: DisposableStore) { }
+
+		provide(picker: IQuickPick<IQuickPickItem>, token: CancellationToken): IDisposable {
+			assert.ok(picker);
+			providerDefaultCalled = true;
+			token.onCancellationRequested(() => providerDefaultCanceled = true);
+
+			// bring up provider #3
+			setTimeout(() => this.quickInputService.quickAccess.show(providerDescriptor3.prefix));
+
+			return toDisposable(() => providerDefaultDisposed = true);
+		}
+	}
 
 	class TestProvider1 implements IQuickAccessProvider {
 		provide(picker: IQuickPick<IQuickPickItem>, token: CancellationToken): IDisposable {
@@ -55,39 +71,22 @@ suite('QuickAccess', () => {
 	}
 
 	class TestProvider3 implements IQuickAccessProvider {
-
-		constructor(@IQuickInputService private readonly quickInputService: IQuickInputService, disposables: DisposableStore) { }
-
 		provide(picker: IQuickPick<IQuickPickItem>, token: CancellationToken): IDisposable {
 			assert.ok(picker);
 			provider3Called = true;
 			token.onCancellationRequested(() => provider3Canceled = true);
 
-			// bring up provider #4
-			setTimeout(() => this.quickInputService.quickAccess.show(providerDescriptor4.prefix));
-
-			return toDisposable(() => provider3Disposed = true);
-		}
-	}
-
-	class TestProvider4 implements IQuickAccessProvider {
-		provide(picker: IQuickPick<IQuickPickItem>, token: CancellationToken): IDisposable {
-			assert.ok(picker);
-			provider4Called = true;
-			token.onCancellationRequested(() => provider4Canceled = true);
-
 			// hide without picking
 			setTimeout(() => picker.hide());
 
-			return toDisposable(() => provider4Disposed = true);
+			return toDisposable(() => provider3Disposed = true);
 		}
 	}
 
-	const defaultProviderDescriptor = { ctor: TestProvider1, prefix: '', helpEntries: [] };
+	const providerDescriptorDefault = { ctor: TestProviderDefault, prefix: '', helpEntries: [] };
 	const providerDescriptor1 = { ctor: TestProvider1, prefix: 'test', helpEntries: [] };
 	const providerDescriptor2 = { ctor: TestProvider2, prefix: 'test something', helpEntries: [] };
-	const providerDescriptor3 = { ctor: TestProvider3, prefix: 'default', helpEntries: [] };
-	const providerDescriptor4 = { ctor: TestProvider4, prefix: 'changed', helpEntries: [] };
+	const providerDescriptor3 = { ctor: TestProvider3, prefix: 'changed', helpEntries: [] };
 
 	setup(() => {
 		instantiationService = workbenchInstantiationService();
@@ -96,91 +95,101 @@ suite('QuickAccess', () => {
 
 	test('registry', () => {
 		const registry = (Registry.as<IQuickAccessRegistry>(Extensions.Quickaccess));
-		registry.defaultProvider = defaultProviderDescriptor;
+		const restore = (registry as QuickAccessRegistry).clear();
 
-		const initialSize = registry.getQuickAccessProviders().length;
+		assert.ok(!registry.getQuickAccessProvider('test'));
 
-		const disposable = registry.registerQuickAccessProvider(providerDescriptor1);
+		const disposables = new DisposableStore();
+
+		disposables.add(registry.registerQuickAccessProvider(providerDescriptorDefault));
+		assert(registry.getQuickAccessProvider('') === providerDescriptorDefault);
+		assert(registry.getQuickAccessProvider('test') === providerDescriptorDefault);
 
+		const disposable = disposables.add(registry.registerQuickAccessProvider(providerDescriptor1));
 		assert(registry.getQuickAccessProvider('test') === providerDescriptor1);
 
 		const providers = registry.getQuickAccessProviders();
 		assert(providers.some(provider => provider.prefix === 'test'));
 
 		disposable.dispose();
+		assert(registry.getQuickAccessProvider('test') === providerDescriptorDefault);
+
+		disposables.dispose();
 		assert.ok(!registry.getQuickAccessProvider('test'));
-		assert.equal(registry.getQuickAccessProviders().length - initialSize, 0);
+
+		restore();
 	});
 
 	test('provider', async () => {
 		const registry = (Registry.as<IQuickAccessRegistry>(Extensions.Quickaccess));
-		const defaultProvider = registry.defaultProvider;
+		const restore = (registry as QuickAccessRegistry).clear();
 
 		const disposables = new DisposableStore();
 
+		disposables.add(registry.registerQuickAccessProvider(providerDescriptorDefault));
 		disposables.add(registry.registerQuickAccessProvider(providerDescriptor1));
 		disposables.add(registry.registerQuickAccessProvider(providerDescriptor2));
-		disposables.add(registry.registerQuickAccessProvider(providerDescriptor4));
-		registry.defaultProvider = providerDescriptor3;
+		disposables.add(registry.registerQuickAccessProvider(providerDescriptor3));
 
 		accessor.quickInputService.quickAccess.show('test');
+		assert.equal(providerDefaultCalled, false);
 		assert.equal(provider1Called, true);
 		assert.equal(provider2Called, false);
 		assert.equal(provider3Called, false);
-		assert.equal(provider4Called, false);
+		assert.equal(providerDefaultCanceled, false);
 		assert.equal(provider1Canceled, false);
 		assert.equal(provider2Canceled, false);
 		assert.equal(provider3Canceled, false);
-		assert.equal(provider4Canceled, false);
+		assert.equal(providerDefaultDisposed, false);
 		assert.equal(provider1Disposed, false);
 		assert.equal(provider2Disposed, false);
 		assert.equal(provider3Disposed, false);
-		assert.equal(provider4Disposed, false);
 		provider1Called = false;
 
 		accessor.quickInputService.quickAccess.show('test something');
+		assert.equal(providerDefaultCalled, false);
 		assert.equal(provider1Called, false);
 		assert.equal(provider2Called, true);
 		assert.equal(provider3Called, false);
-		assert.equal(provider4Called, false);
+		assert.equal(providerDefaultCanceled, false);
 		assert.equal(provider1Canceled, true);
 		assert.equal(provider2Canceled, false);
 		assert.equal(provider3Canceled, false);
-		assert.equal(provider4Canceled, false);
+		assert.equal(providerDefaultDisposed, false);
 		assert.equal(provider1Disposed, true);
 		assert.equal(provider2Disposed, false);
 		assert.equal(provider3Disposed, false);
-		assert.equal(provider4Disposed, false);
 		provider2Called = false;
 		provider1Canceled = false;
 		provider1Disposed = false;
 
 		accessor.quickInputService.quickAccess.show('usedefault');
+		assert.equal(providerDefaultCalled, true);
 		assert.equal(provider1Called, false);
 		assert.equal(provider2Called, false);
-		assert.equal(provider3Called, true);
-		assert.equal(provider4Called, false);
+		assert.equal(provider3Called, false);
+		assert.equal(providerDefaultCanceled, false);
 		assert.equal(provider1Canceled, false);
 		assert.equal(provider2Canceled, true);
 		assert.equal(provider3Canceled, false);
-		assert.equal(provider4Canceled, false);
+		assert.equal(providerDefaultDisposed, false);
 		assert.equal(provider1Disposed, false);
 		assert.equal(provider2Disposed, true);
 		assert.equal(provider3Disposed, false);
-		assert.equal(provider4Disposed, false);
 
 		await timeout(1);
 
-		assert.equal(provider3Canceled, true);
-		assert.equal(provider3Disposed, true);
-		assert.equal(provider4Called, true);
+		assert.equal(providerDefaultCanceled, true);
+		assert.equal(providerDefaultDisposed, true);
+		assert.equal(provider3Called, true);
 
 		await timeout(1);
 
-		assert.equal(provider4Canceled, true);
-		assert.equal(provider4Disposed, true);
+		assert.equal(provider3Canceled, true);
+		assert.equal(provider3Disposed, true);
 
 		disposables.dispose();
-		registry.defaultProvider = defaultProvider;
+
+		restore();
 	});
 });