move sorting and snippet selection into low level function, #9286

src/vs/editor/contrib/suggest/common/completionModel.ts

@@ -11,67 +11,10 @@ import {TPromise} from 'vs/base/common/winjs.base';
 import {IReadOnlyModel} from 'vs/editor/common/editorCommon';
 import {IFilter, IMatch, fuzzyContiguousFilter} from 'vs/base/common/filters';
 import {ISuggestResult, ISuggestSupport, ISuggestion} from 'vs/editor/common/modes';
-import {ISuggestResult2} from './suggest';
+import {ISuggestionItem} from './suggest';
 import {asWinJsPromise} from 'vs/base/common/async';
 import {Position} from 'vs/editor/common/core/position';
 
-export interface CompletionItemComparator {
-	(a: CompletionItem, b: CompletionItem): number;
-}
-
-export namespace CompletionItemComparator {
-
-	export function fromConfig(order: 'top' | 'bottom' | string): CompletionItemComparator {
-		return order === 'top'
-			? CompletionItemComparator.snippetUpComparator
-			: order === 'bottom'
-				? CompletionItemComparator.snippetDownComparator
-				: CompletionItemComparator.defaultComparator;
-	}
-
-	export function defaultComparator(item: CompletionItem, otherItem: CompletionItem): number {
-		const suggestion = item.suggestion;
-		const otherSuggestion = otherItem.suggestion;
-
-		if (typeof suggestion.sortText === 'string' && typeof otherSuggestion.sortText === 'string') {
-			const one = suggestion.sortText.toLowerCase();
-			const other = otherSuggestion.sortText.toLowerCase();
-
-			if (one < other) {
-				return -1;
-			} else if (one > other) {
-				return 1;
-			}
-		}
-
-		return suggestion.label.toLowerCase() < otherSuggestion.label.toLowerCase() ? -1 : 1;
-	}
-
-	export function snippetUpComparator(a: CompletionItem, b: CompletionItem): number {
-		if (a.suggestion.type !== b.suggestion.type) {
-			if (a.suggestion.type === 'snippet') {
-				return -1;
-			} else if (b.suggestion.type === 'snippet') {
-				return 1;
-			}
-		} else {
-			return defaultComparator(a, b);
-		}
-	}
-
-	export function snippetDownComparator(a: CompletionItem, b: CompletionItem): number {
-		if (a.suggestion.type !== b.suggestion.type) {
-			if (a.suggestion.type === 'snippet') {
-				return 1;
-			} else if (b.suggestion.type === 'snippet') {
-				return -1;
-			}
-		} else {
-			return defaultComparator(a, b);
-		}
-	}
-}
-
 export class CompletionItem {
 
 	suggestion: ISuggestion;
@@ -81,11 +24,11 @@ export class CompletionItem {
 
 	private _support: ISuggestSupport;
 
-	constructor(suggestion: ISuggestion, container: ISuggestResult2) {
-		this._support = container.support;
-		this.suggestion = suggestion;
-		this.container = container;
-		this.filter = container.support && container.support.filter || fuzzyContiguousFilter;
+	constructor(item: ISuggestionItem) {
+		this.suggestion = item.suggestion;
+		this.container = item.container;
+		this.filter = item.support && item.support.filter || fuzzyContiguousFilter;
+		this._support = item.support;
 	}
 
 	resolveDetails(model:IReadOnlyModel, position:Position): TPromise<ISuggestion> {
@@ -110,22 +53,18 @@ export class LineContext {
 
 export class CompletionModel {
 
-	public raw: ISuggestResult2[];
+	public raw: ISuggestionItem[];
 
 	private _lineContext: LineContext;
 	private _items: CompletionItem[] = [];
 	private _filteredItems: CompletionItem[] = undefined;
 
-	constructor(raw: ISuggestResult2[], leadingLineContent: string, comparator: CompletionItemComparator, ignoreSnippets: boolean) {
+	constructor(raw: ISuggestionItem[], leadingLineContent: string) {
+		this.raw = raw;
 		this._lineContext = { leadingLineContent, characterCountDelta: 0 };
-		for (let container of raw) {
-			for (let suggestion of container.suggestions) {
-				if (!ignoreSnippets || suggestion.type !== 'snippet') {
-					this._items.push(new CompletionItem(suggestion, container));
-				}
-			}
+		for (const item of raw) {
+			this._items.push(new CompletionItem(item));
 		}
-		this._items.sort(comparator);
 	}
 
 	get lineContext(): LineContext {

src/vs/editor/contrib/suggest/common/suggest.ts

@@ -10,7 +10,7 @@ import {onUnexpectedError} from 'vs/base/common/errors';
 import {TPromise} from 'vs/base/common/winjs.base';
 import {IReadOnlyModel} from 'vs/editor/common/editorCommon';
 import {CommonEditorRegistry} from 'vs/editor/common/editorCommonExtensions';
-import {ISuggestResult, ISuggestSupport, SuggestRegistry} from 'vs/editor/common/modes';
+import {ISuggestResult, ISuggestSupport, ISuggestion, SuggestRegistry} from 'vs/editor/common/modes';
 import {SnippetsRegistry} from 'vs/editor/common/modes/supports';
 import {Position} from 'vs/editor/common/core/position';
 
@@ -20,63 +20,132 @@ export const Context = {
 	AcceptOnKey: 'suggestionSupportsAcceptOnKey'
 };
 
-export interface ISuggestResult2 extends ISuggestResult {
-	support?: ISuggestSupport;
+export interface ISuggestionItem {
+	suggestion: ISuggestion;
+	container: ISuggestResult;
+	support: ISuggestSupport;
 }
 
-export function provideCompletionItems(model: IReadOnlyModel, position: Position, groups?: ISuggestSupport[][]): TPromise<ISuggestResult2[]> {
+export interface ISuggestOptions {
+	groups?: ISuggestSupport[][];
+	snippetConfig?: 'top' | 'bottom' | 'inline' | 'none' | 'only';
+}
 
-	if (!groups) {
-		groups = SuggestRegistry.orderedGroups(model);
-	}
+export function provideSuggestionItems(model: IReadOnlyModel, position: Position, options: ISuggestOptions = {}): TPromise<ISuggestionItem[]> {
 
-	const result: ISuggestResult2[] = [];
+	const result: ISuggestionItem[] = [];
+	const suggestFilter = createSuggesionFilter(options);
+	const suggestCompare = createSuggesionComparator(options);
 
-	const factory = groups.map((supports, index) => {
-		return () => {
+	// add suggestions from snippet registry
+	const snippets = SnippetsRegistry.getSnippets(model, position);
+	fillInSuggestResult(result, snippets, undefined, suggestFilter);
 
-			// stop as soon as a group produced a result
-			if (result.length > 0) {
+
+	// add suggestions from contributed providers - providers are ordered in groups of
+	// equal score and once a group produces a result the process stops
+	let hasResult = false;
+	const factory = (options.groups || SuggestRegistry.orderedGroups(model)).map(supports => {
+		return () => {
+			// stop when we have a result
+			if (hasResult) {
 				return;
 			}
-
 			// for each support in the group ask for suggestions
-			return TPromise.join(supports.map(support => {
-				return asWinJsPromise((token) => {
-					return support.provideCompletionItems(model, position, token);
-				}).then(values => {
-
-					if (!values) {
-						return;
+			return TPromise.join(supports.map(support => asWinJsPromise(token => support.provideCompletionItems(model, position, token)).then(values => {
+				if (!isFalsyOrEmpty(values)) {
+					for (let suggestResult of values) {
+						hasResult = fillInSuggestResult(result, suggestResult, support, suggestFilter) || hasResult;
 					}
+				}
+			}, onUnexpectedError)));
+		};
+	});
 
-					for (let suggestResult of values) {
+	return sequence(factory).then(() => result.sort(suggestCompare));
+}
 
-						if (!suggestResult || isFalsyOrEmpty(suggestResult.suggestions)) {
-							continue;
-						}
+function fillInSuggestResult(bucket: ISuggestionItem[], result: ISuggestResult, support: ISuggestSupport, acceptFn: (c: ISuggestion) => boolean): boolean {
+	if (!result) {
+		return false;
+	}
+	if (!result.suggestions) {
+		return false;
+	}
+	const len = bucket.length;
+	for (const suggestion of result.suggestions) {
+		if (acceptFn(suggestion)) {
+			bucket.push({
+				support,
+				suggestion,
+				container: result,
+			});
+		}
+	}
+	return len !== bucket.length;
+}
 
-						result.push({
-							support,
-							currentWord: suggestResult.currentWord,
-							incomplete: suggestResult.incomplete,
-							suggestions: suggestResult.suggestions
-						});
-					}
+function createSuggesionFilter(options: ISuggestOptions): (candidate: ISuggestion) => boolean {
+	if (options.snippetConfig === 'only') {
+		return suggestion => suggestion.type === 'snippet';
+	} else if (options.snippetConfig === 'none') {
+		return suggestion => suggestion.type !== 'snippet';
+	} else {
+		return _ => true;
+	}
+}
 
-				}, onUnexpectedError);
-			}));
-		};
-	});
+function createSuggesionComparator(options: ISuggestOptions): (a: ISuggestionItem, b: ISuggestionItem) => number {
 
-	return sequence(factory).then(() => {
-		// add snippets to the first group
-		const snippets = SnippetsRegistry.getSnippets(model, position);
-		result.push(snippets);
-		return result;
-	});
+	function defaultComparator(a: ISuggestionItem, b: ISuggestionItem): number {
+
+		if (typeof a.suggestion.sortText === 'string' && typeof b.suggestion.sortText === 'string') {
+			const one = a.suggestion.sortText.toLowerCase();
+			const other = b.suggestion.sortText.toLowerCase();
+
+			if (one < other) {
+				return -1;
+			} else if (one > other) {
+				return 1;
+			}
+		}
+
+		return a.suggestion.label.toLowerCase() < b.suggestion.label.toLowerCase() ? -1 : 1;
+	}
+
+	function snippetUpComparator(a: ISuggestionItem, b: ISuggestionItem): number {
+		if (a.suggestion.type !== b.suggestion.type) {
+			if (a.suggestion.type === 'snippet') {
+				return -1;
+			} else if (b.suggestion.type === 'snippet') {
+				return 1;
+			}
+		} else {
+			return defaultComparator(a, b);
+		}
+	}
+
+	function snippetDownComparator(a: ISuggestionItem, b: ISuggestionItem): number {
+		if (a.suggestion.type !== b.suggestion.type) {
+			if (a.suggestion.type === 'snippet') {
+				return 1;
+			} else if (b.suggestion.type === 'snippet') {
+				return -1;
+			}
+		} else {
+			return defaultComparator(a, b);
+		}
+	}
+
+	if (options.snippetConfig === 'top') {
+		return snippetUpComparator;
+	} else if (options.snippetConfig === 'bottom') {
+		return snippetDownComparator;
+	} else {
+		return defaultComparator;
+	}
 }
 
 CommonEditorRegistry.registerDefaultLanguageCommand('_executeCompletionItemProvider', (model, position, args) => {
-	return provideCompletionItems(model, position);
+	return provideSuggestionItems(model, position);
 });

src/vs/editor/contrib/suggest/common/suggestModel.ts

@@ -12,8 +12,8 @@ import {TPromise} from 'vs/base/common/winjs.base';
 import {ICommonCodeEditor, ICursorSelectionChangedEvent, CursorChangeReason, IModel, IPosition} from 'vs/editor/common/editorCommon';
 import {ISuggestSupport, ISuggestion, SuggestRegistry} from 'vs/editor/common/modes';
 import {CodeSnippet} from 'vs/editor/contrib/snippet/common/snippet';
-import {ISuggestResult2, provideCompletionItems} from './suggest';
-import {CompletionModel, CompletionItemComparator} from './completionModel';
+import {ISuggestionItem, provideSuggestionItems} from './suggest';
+import {CompletionModel} from './completionModel';
 import {Position} from 'vs/editor/common/core/position';
 
 export interface ICancelEvent {
@@ -168,7 +168,7 @@ export class SuggestModel implements IDisposable {
 	private requestPromise: TPromise<void>;
 	private context: Context;
 
-	private raw: ISuggestResult2[];
+	private raw: ISuggestionItem[];
 	private completionModel: CompletionModel;
 	private incomplete: boolean;
 
@@ -330,7 +330,7 @@ export class SuggestModel implements IDisposable {
 
 		const position = this.editor.getPosition();
 
-		this.requestPromise = provideCompletionItems(model, position, groups).then(all => {
+		this.requestPromise = provideSuggestionItems(model, position, groups).then(all => {
 			this.requestPromise = null;
 
 			if (this.state === State.Idle) {
@@ -338,7 +338,7 @@ export class SuggestModel implements IDisposable {
 			}
 
 			this.raw = all;
-			this.incomplete = all.some(result => result.incomplete);
+			this.incomplete = all.some(result => result.container.incomplete);
 
 			const model = this.editor.getModel();
 
@@ -379,10 +379,7 @@ export class SuggestModel implements IDisposable {
 					isFrozen = true;
 				}
 			} else {
-				const {contribInfo} = this.editor.getConfiguration();
-				this.completionModel = new CompletionModel(this.raw, ctx.lineContentBefore,
-					CompletionItemComparator.fromConfig(contribInfo.snippetOrder),
-					!contribInfo.snippetSuggestions);
+				this.completionModel = new CompletionModel(this.raw, ctx.lineContentBefore);
 			}
 
 			this._onDidSuggest.fire({

src/vs/editor/contrib/suggest/test/common/completionModel.test.ts

-/*---------------------------------------------------------------------------------------------
- *  Copyright (c) Microsoft Corporation. All rights reserved.
- *  Licensed under the MIT License. See License.txt in the project root for license information.
- *--------------------------------------------------------------------------------------------*/
-'use strict';
-
-import * as assert from 'assert';
-import {ISuggestResult2} from 'vs/editor/contrib/suggest/common/suggest';
-import {CompletionModel, CompletionItemComparator} from 'vs/editor/contrib/suggest/common/completionModel';
-
-
-const mixedSuggestions = <ISuggestResult2>{
-	currentWord: '',
-	incomplete: false,
-	support: undefined,
-	suggestions: [{
-		type: 'snippet',
-		label: 'zzz',
-		codeSnippet: 'zzz'
-	}, {
-		type: 'snippet',
-		label: 'aaa',
-		codeSnippet: 'aaa'
-	}, {
-		type: 'property',
-		label: 'fff',
-		codeSnippet: 'fff'
-	}]
-};
-
-
-suite('CompletionModel', function() {
-
-	test('sort - normal', function() {
-		const model = new CompletionModel([mixedSuggestions], '', CompletionItemComparator.defaultComparator, false);
-		assert.equal(model.items.length, 3);
-
-		const [one, two, three] = model.items;
-		assert.equal(one.suggestion.label, 'aaa');
-		assert.equal(two.suggestion.label, 'fff');
-		assert.equal(three.suggestion.label, 'zzz');
-	});
-
-	test('sort - snippet up', function() {
-		const model = new CompletionModel([mixedSuggestions], '', CompletionItemComparator.snippetUpComparator, false);
-		assert.equal(model.items.length, 3);
-
-		const [one, two, three] = model.items;
-		assert.equal(one.suggestion.label, 'aaa');
-		assert.equal(two.suggestion.label, 'zzz');
-		assert.equal(three.suggestion.label, 'fff');
-	});
-
-	test('sort - snippet down', function() {
-		const model = new CompletionModel([mixedSuggestions], '', CompletionItemComparator.snippetDownComparator, false);
-		assert.equal(model.items.length, 3);
-
-		const [one, two, three] = model.items;
-		assert.equal(one.suggestion.label, 'fff');
-		assert.equal(two.suggestion.label, 'aaa');
-		assert.equal(three.suggestion.label, 'zzz');
-	});
-
-	test('ignore snippets', function() {
-
-		const model = new CompletionModel([mixedSuggestions], '', CompletionItemComparator.defaultComparator, true);
-		assert.equal(model.items.length, 1);
-
-		const [one] = model.items;
-		assert.equal(one.suggestion.label, 'fff');
-	});
-});
\ No newline at end of file

src/vs/workbench/api/node/extHostApiCommands.ts

@@ -329,18 +329,15 @@ class ExtHostApiCommands {
 			position: position && typeConverters.fromPosition(position),
 			triggerCharacter
 		};
-		return this._commands.executeCommand<modes.ISuggestResult[]>('_executeCompletionItemProvider', args).then(value => {
-			if (value) {
+		return this._commands.executeCommand<{ suggestion: modes.ISuggestion; container: modes.ISuggestResult }[]>('_executeCompletionItemProvider', args).then(values => {
+			if (values) {
 				let items: types.CompletionItem[] = [];
 				let incomplete: boolean;
-				for (let suggestions of value) {
-					incomplete = suggestions.incomplete || incomplete;
-					for (let suggestion of suggestions.suggestions) {
-						const item = typeConverters.Suggest.to(suggestions, position, suggestion);
-						items.push(item);
-					}
+				for (const item of values) {
+					incomplete = item.container.incomplete || incomplete;
+					items.push(typeConverters.Suggest.to(item.container, position, item.suggestion));
 				}
-				return new types.CompletionList(<any>items, incomplete);
+				return new types.CompletionList(items, incomplete);
 			}
 		});
 	}

src/vs/workbench/test/node/api/extHostLanguageFeatures.test.ts

@@ -33,7 +33,7 @@ import {getCodeActions} from 'vs/editor/contrib/quickFix/common/quickFix';
 import {getNavigateToItems} from 'vs/workbench/parts/search/common/search';
 import {rename} from 'vs/editor/contrib/rename/common/rename';
 import {provideSignatureHelp} from 'vs/editor/contrib/parameterHints/common/parameterHints';
-import {provideCompletionItems} from 'vs/editor/contrib/suggest/common/suggest';
+import {provideSuggestionItems} from 'vs/editor/contrib/suggest/common/suggest';
 import {getDocumentFormattingEdits, getDocumentRangeFormattingEdits, getOnTypeFormattingEdits} from 'vs/editor/contrib/format/common/format';
 import {asWinJsPromise} from 'vs/base/common/async';
 import {MainContext, ExtHostContext} from 'vs/workbench/api/node/extHost.protocol';
@@ -760,11 +760,9 @@ suite('ExtHostLanguageFeatures', function() {
 		}, []));
 
 		return threadService.sync().then(() => {
-			return provideCompletionItems(model, new EditorPosition(1, 1)).then(value => {
-				assert.ok(value.length >= 1); // check for min because snippets and others contribute
-				let [first] = value;
-				assert.equal(first.suggestions.length, 1);
-				assert.equal(first.suggestions[0].codeSnippet, 'testing2');
+			return provideSuggestionItems(model, new EditorPosition(1, 1), { snippetConfig: 'none' }).then(value => {
+				assert.equal(value.length, 1);
+				assert.equal(value[0].suggestion.codeSnippet, 'testing2');
 			});
 		});
 	});
@@ -784,11 +782,9 @@ suite('ExtHostLanguageFeatures', function() {
 		}, []));
 
 		return threadService.sync().then(() => {
-			return provideCompletionItems(model, new EditorPosition(1, 1)).then(value => {
-				assert.ok(value.length >= 1);
-				let [first] = value;
-				assert.equal(first.suggestions.length, 1);
-				assert.equal(first.suggestions[0].codeSnippet, 'weak-selector');
+			return provideSuggestionItems(model, new EditorPosition(1, 1), { snippetConfig: 'none' }).then(value => {
+				assert.equal(value.length, 1);
+				assert.equal(value[0].suggestion.codeSnippet, 'weak-selector');
 			});
 		});
 	});
@@ -808,12 +804,10 @@ suite('ExtHostLanguageFeatures', function() {
 		}, []));
 
 		return threadService.sync().then(() => {
-			return provideCompletionItems(model, new EditorPosition(1, 1)).then(value => {
-				assert.ok(value.length >= 2);
-				let [first, second] = value;
-				assert.equal(first.suggestions.length, 1);
-				assert.equal(first.suggestions[0].codeSnippet, 'strong-2'); // last wins
-				assert.equal(second.suggestions[0].codeSnippet, 'strong-1');
+			return provideSuggestionItems(model, new EditorPosition(1, 1), { snippetConfig: 'none' }).then(value => {
+				assert.equal(value.length, 2);
+				assert.equal(value[0].suggestion.codeSnippet, 'strong-1'); // sort by label
+				assert.equal(value[1].suggestion.codeSnippet, 'strong-2');
 			});
 		});
 	});
@@ -835,8 +829,8 @@ suite('ExtHostLanguageFeatures', function() {
 
 		return threadService.sync().then(() => {
 
-			return provideCompletionItems(model, new EditorPosition(1, 1)).then(value => {
-				assert.equal(value[0].incomplete, undefined);
+			return provideSuggestionItems(model, new EditorPosition(1, 1), { snippetConfig: 'none' }).then(value => {
+				assert.equal(value[0].container.incomplete, undefined);
 			});
 		});
 	});
@@ -851,8 +845,8 @@ suite('ExtHostLanguageFeatures', function() {
 
 		return threadService.sync().then(() => {
 
-			provideCompletionItems(model, new EditorPosition(1, 1)).then(value => {
-				assert.equal(value[0].incomplete, true);
+			provideSuggestionItems(model, new EditorPosition(1, 1), { snippetConfig: 'none' }).then(value => {
+				assert.equal(value[0].container.incomplete, true);
 			});
 		});
 	});