Merge pull request #10657 from Microsoft/joh/suggest

some suggest refactorings

src/vs/editor/browser/editor.all.ts

@@ -35,7 +35,7 @@ import 'vs/editor/contrib/smartSelect/common/smartSelect';
 import 'vs/editor/contrib/smartSelect/common/jumpToBracket';
 import 'vs/editor/contrib/snippet/common/snippet';
 import 'vs/editor/contrib/snippet/browser/snippet';
-import 'vs/editor/contrib/suggest/browser/suggest';
+import 'vs/editor/contrib/suggest/browser/suggestController';
 import 'vs/editor/contrib/suggest/browser/tabCompletion';
 import 'vs/editor/contrib/toggleTabFocusMode/common/toggleTabFocusMode';
 import 'vs/editor/contrib/toggleWordWrap/common/toggleWordWrap';

src/vs/editor/contrib/suggest/browser/suggest.ts → src/vs/editor/contrib/suggest/browser/suggestController.ts

@@ -7,16 +7,13 @@
 import * as nls from 'vs/nls';
 import { KeyCode, KeyMod } from 'vs/base/common/keyCodes';
 import { IDisposable, dispose } from 'vs/base/common/lifecycle';
-import { isFalsyOrEmpty } from 'vs/base/common/arrays';
-import { forEach } from 'vs/base/common/collections';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey';
 import { ICommonCodeEditor, IEditorContribution, EditorContextKeys, ModeContextKeys } from 'vs/editor/common/editorCommon';
 import { editorAction, ServicesAccessor, EditorAction, EditorCommand, CommonEditorRegistry } from 'vs/editor/common/editorCommonExtensions';
-import { ISuggestSupport, SuggestRegistry } from 'vs/editor/common/modes';
 import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
 import { EditorBrowserRegistry } from 'vs/editor/browser/editorBrowserExtensions';
-import { getSnippetController } from 'vs/editor/contrib/snippet/common/snippet';
+import { getSnippetController, CodeSnippet } from 'vs/editor/contrib/snippet/common/snippet';
 import { Context as SuggestContext } from 'vs/editor/contrib/suggest/common/suggest';
 import { SuggestModel } from '../common/suggestModel';
 import { CompletionItem } from '../common/completionModel';
@@ -31,7 +28,6 @@ export class SuggestController implements IEditorContribution {
 
 	private model: SuggestModel;
 	private widget: SuggestWidget;
-	private triggerCharacterListeners: IDisposable[];
 	private toDispose: IDisposable[] = [];
 
 	constructor(
@@ -39,24 +35,12 @@ export class SuggestController implements IEditorContribution {
 		@IInstantiationService instantiationService: IInstantiationService
 	) {
 		this.model = new SuggestModel(this.editor);
-		this.widget = instantiationService.createInstance(SuggestWidget, this.editor);
-
-		this.toDispose.push(this.model.onDidTrigger(e => this.widget.showTriggered(e)));
-		this.toDispose.push(this.model.onDidSuggest(e => this.widget.showSuggestions(e)));
-		this.toDispose.push(this.model.onDidCancel(e => this.widget.showDidCancel(e)));
+		this.toDispose.push(this.model.onDidTrigger(e => this.widget.showTriggered(e.auto)));
+		this.toDispose.push(this.model.onDidSuggest(e => this.widget.showSuggestions(e.completionModel, e.isFrozen, e.auto)));
+		this.toDispose.push(this.model.onDidCancel(e => !e.retrigger && this.widget.hideWidget()));
 
+		this.widget = instantiationService.createInstance(SuggestWidget, this.editor);
 		this.toDispose.push(this.widget.onDidSelect(this.onDidSelectItem, this));
-
-		this.triggerCharacterListeners = [];
-
-		this.toDispose.push(editor.onDidChangeConfiguration(() => this.update()));
-		this.toDispose.push(editor.onDidChangeModel(() => this.update()));
-		this.toDispose.push(editor.onDidChangeModelMode(() => this.update()));
-		this.toDispose.push(SuggestRegistry.onDidChange(this.update, this));
-
-		this.toDispose.push(this.model.onDidAccept(e => getSnippetController(this.editor).run(e.snippet, e.overwriteBefore, e.overwriteAfter)));
-
-		this.update();
 	}
 
 	getId(): string {
@@ -65,8 +49,6 @@ export class SuggestController implements IEditorContribution {
 
 	dispose(): void {
 		this.toDispose = dispose(this.toDispose);
-		this.triggerCharacterListeners = dispose(this.triggerCharacterListeners);
-
 		if (this.widget) {
 			this.widget.dispose();
 			this.widget = null;
@@ -78,45 +60,16 @@ export class SuggestController implements IEditorContribution {
 	}
 
 	private onDidSelectItem(item: CompletionItem): void {
-		if (!item) {
-			this.model.cancel();
-			return;
-		}
-		const {overwriteBefore, overwriteAfter} = item.suggestion;
-		this.model.accept(item.suggestion, overwriteBefore, overwriteAfter);
-	}
-
-	private update(): void {
-
-		this.triggerCharacterListeners = dispose(this.triggerCharacterListeners);
+		if (item) {
+			const {insertText, overwriteBefore, overwriteAfter} = item.suggestion;
+			const columnDelta = this.editor.getPosition().column - this.model.getTriggerPosition().column;
 
-		if (this.editor.getConfiguration().readOnly
-			|| !this.editor.getModel()
-			|| !this.editor.getConfiguration().contribInfo.suggestOnTriggerCharacters) {
-
-			return;
+			getSnippetController(this.editor).run(new CodeSnippet(insertText),
+				overwriteBefore + columnDelta,
+				overwriteAfter);
 		}
 
-		const supportsByTriggerCharacter: { [ch: string]: ISuggestSupport[] } = Object.create(null);
-		for (const support of SuggestRegistry.all(this.editor.getModel())) {
-			if (isFalsyOrEmpty(support.triggerCharacters)) {
-				continue;
-			}
-			for (const ch of support.triggerCharacters) {
-				const array = supportsByTriggerCharacter[ch];
-				if (!array) {
-					supportsByTriggerCharacter[ch] = [support];
-				} else {
-					array.push(support);
-				}
-			}
-		}
-
-		forEach(supportsByTriggerCharacter, entry => {
-			this.triggerCharacterListeners.push(this.editor.addTypingListener(entry.key, () => {
-				this.model.trigger(true, false, entry.value);
-			}));
-		});
+		this.model.cancel();
 	}
 
 	triggerSuggest(): void {
@@ -133,7 +86,7 @@ export class SuggestController implements IEditorContribution {
 
 	cancelSuggestWidget(): void {
 		if (this.widget) {
-			this.widget.cancel();
+			this.widget.hideDetailsOrHideWidget();
 		}
 	}
 

src/vs/editor/contrib/suggest/browser/suggestWidget.ts

@@ -24,7 +24,6 @@ import { IConfigurationChangedEvent } from 'vs/editor/common/editorCommon';
 import { ContentWidgetPositionPreference, ICodeEditor, IContentWidget, IContentWidgetPosition } from 'vs/editor/browser/editorBrowser';
 import { Context as SuggestContext } from '../common/suggest';
 import { CompletionItem, CompletionModel } from '../common/completionModel';
-import { ICancelEvent, ISuggestEvent, ITriggerEvent } from '../common/suggestModel';
 import { alert } from 'vs/base/browser/ui/aria/aria';
 import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 
@@ -532,12 +531,12 @@ export class SuggestWidget implements IContentWidget, IDisposable {
 		return this.onDidSelectEmitter.event;
 	}
 
-	showTriggered(e: ITriggerEvent) {
+	showTriggered(auto: boolean) {
 		if (this.state !== State.Hidden) {
 			return;
 		}
 
-		this.isAuto = !!e.auto;
+		this.isAuto = !!auto;
 
 		if (!this.isAuto) {
 			this.loadingTimeout = setTimeout(() => {
@@ -547,15 +546,15 @@ export class SuggestWidget implements IContentWidget, IDisposable {
 		}
 	}
 
-	showSuggestions(e: ISuggestEvent): void {
+	showSuggestions(completionModel: CompletionModel, isFrozen: boolean, isAuto: boolean): void {
 		if (this.loadingTimeout) {
 			clearTimeout(this.loadingTimeout);
 			this.loadingTimeout = null;
 		}
 
-		this.completionModel = e.completionModel;
+		this.completionModel = completionModel;
 
-		if (e.isFrozen && this.state !== State.Empty) {
+		if (isFrozen && this.state !== State.Empty) {
 			this.setState(State.Frozen);
 			return;
 		}
@@ -566,7 +565,7 @@ export class SuggestWidget implements IContentWidget, IDisposable {
 		this.suggestWidgetMultipleSuggestions.set(visibleCount > 1);
 
 		if (isEmpty) {
-			if (e.auto) {
+			if (isAuto) {
 				this.setState(State.Hidden);
 			} else {
 				this.setState(State.Empty);
@@ -595,22 +594,11 @@ export class SuggestWidget implements IContentWidget, IDisposable {
 				suggestionCount: visibleCount,
 				snippetCount,
 				textCount,
-				wasAutomaticallyTriggered: !!e.auto
+				wasAutomaticallyTriggered: !!isAuto
 			});
 		}
 	}
 
-	showDidCancel(e: ICancelEvent) {
-		if (this.loadingTimeout) {
-			clearTimeout(this.loadingTimeout);
-			this.loadingTimeout = null;
-		}
-
-		if (!e.retrigger) {
-			this.setState(State.Hidden);
-		}
-	}
-
 	selectNextPage(): boolean {
 		switch (this.state) {
 			case State.Hidden:
@@ -715,11 +703,16 @@ export class SuggestWidget implements IContentWidget, IDisposable {
 		removeClass(this.element, 'visible');
 	}
 
-	cancel(): void {
+	hideWidget(): void {
+		clearTimeout(this.loadingTimeout);
+		this.setState(State.Hidden);
+	}
+
+	hideDetailsOrHideWidget(): void {
 		if (this.state === State.Details) {
 			this.toggleDetails();
 		} else {
-			this.showDidCancel({ retrigger: false });
+			this.hideWidget();
 		}
 	}
 

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

@@ -101,7 +101,7 @@ export function provideSuggestionItems(model: IReadOnlyModel, position: Position
 		};
 	});
 
-	return sequence(factory).then(() => result.sort(createSuggesionComparator(snippetConfig)));
+	return sequence(factory).then(() => result.sort(getSuggestionComparator(snippetConfig)));
 }
 
 function fixOverwriteBeforeAfter(suggestion: ISuggestion, container: ISuggestResult): void {
@@ -131,7 +131,7 @@ function createSuggesionFilter(snippetConfig: SnippetConfig): (candidate: ISugge
 	}
 }
 
-function createSuggesionComparator(snippetConfig: SnippetConfig): (a: ISuggestionItem, b: ISuggestionItem) => number {
+export function getSuggestionComparator(snippetConfig: SnippetConfig): (a: ISuggestionItem, b: ISuggestionItem) => number {
 
 	function defaultComparator(a: ISuggestionItem, b: ISuggestionItem): number {
 

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

@@ -5,13 +5,14 @@
 'use strict';
 
 import {onUnexpectedError} from 'vs/base/common/errors';
+import { isFalsyOrEmpty } from 'vs/base/common/arrays';
+import { forEach } from 'vs/base/common/collections';
 import Event, { Emitter } from 'vs/base/common/event';
 import {IDisposable, dispose} from 'vs/base/common/lifecycle';
 import {startsWith} from 'vs/base/common/strings';
 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 {ISuggestSupport, SuggestRegistry} from 'vs/editor/common/modes';
 import {ISuggestionItem, provideSuggestionItems} from './suggest';
 import {CompletionModel} from './completionModel';
 
@@ -29,12 +30,6 @@ export interface ISuggestEvent {
 	auto: boolean;
 }
 
-export interface IAcceptEvent {
-	snippet: CodeSnippet;
-	overwriteBefore: number;
-	overwriteAfter: number;
-}
-
 class Context {
 
 	lineNumber: number;
@@ -147,8 +142,9 @@ enum State {
 
 export class SuggestModel implements IDisposable {
 
-	private toDispose: IDisposable[];
-	private autoSuggestDelay: number;
+	private toDispose: IDisposable[] = [];
+	private quickSuggestDelay: number;
+	private triggerCharacterListeners: IDisposable[] = [];
 
 	private triggerAutoSuggestPromise: TPromise<void>;
 	private state: State;
@@ -156,7 +152,7 @@ export class SuggestModel implements IDisposable {
 	private requestPromise: TPromise<void>;
 	private context: Context;
 
-	private raw: ISuggestionItem[];
+	private suggestionItems: ISuggestionItem[];
 	private completionModel: CompletionModel;
 	private incomplete: boolean;
 
@@ -169,27 +165,93 @@ export class SuggestModel implements IDisposable {
 	private _onDidSuggest: Emitter<ISuggestEvent> = new Emitter();
 	get onDidSuggest(): Event<ISuggestEvent> { return this._onDidSuggest.event; }
 
-	private _onDidAccept: Emitter<IAcceptEvent> = new Emitter();
-	get onDidAccept(): Event<IAcceptEvent> { return this._onDidAccept.event; }
-
 	constructor(private editor: ICommonCodeEditor) {
 		this.state = State.Idle;
 		this.triggerAutoSuggestPromise = null;
 		this.requestPromise = null;
-		this.raw = null;
+		this.suggestionItems = null;
 		this.completionModel = null;
 		this.incomplete = false;
 		this.context = null;
 
-		this.toDispose = [];
-		this.toDispose.push(this.editor.onDidChangeConfiguration(() => this.onEditorConfigurationChange()));
-		this.toDispose.push(this.editor.onDidChangeCursorSelection(e => this.onCursorChange(e)));
-		this.toDispose.push(this.editor.onDidChangeModel(() => this.cancel()));
-		this.toDispose.push(SuggestRegistry.onDidChange(this.onSuggestRegistryChange, this));
-		this.onEditorConfigurationChange();
+		// wire up various listeners
+		this.toDispose.push(this.editor.onDidChangeModel(() => {
+			this.updateTriggerCharacters();
+			this.cancel();
+		}));
+		this.toDispose.push(editor.onDidChangeModelMode(() => {
+			this.updateTriggerCharacters();
+			this.cancel();
+		}));
+		this.toDispose.push(this.editor.onDidChangeConfiguration(() => {
+			this.updateTriggerCharacters();
+			this.updateQuickSuggest();
+		}));
+		this.toDispose.push(SuggestRegistry.onDidChange(() => {
+			this.updateTriggerCharacters();
+			this.updateActiveSuggestSession();
+		}));
+		this.toDispose.push(this.editor.onDidChangeCursorSelection(e => {
+			this.onCursorChange(e);
+		}));
+
+		this.updateTriggerCharacters();
+		this.updateQuickSuggest();
+	}
+
+	dispose(): void {
+		dispose([this._onDidCancel, this._onDidSuggest, this._onDidTrigger]);
+		this.toDispose = dispose(this.toDispose);
+		this.triggerCharacterListeners = dispose(this.triggerCharacterListeners);
+		this.cancel();
+	}
+
+	// --- handle configuration & precondition changes
+
+	private updateQuickSuggest(): void {
+		this.quickSuggestDelay = this.editor.getConfiguration().contribInfo.quickSuggestionsDelay;
+
+		if (isNaN(this.quickSuggestDelay) || (!this.quickSuggestDelay && this.quickSuggestDelay !== 0) || this.quickSuggestDelay < 0) {
+			this.quickSuggestDelay = 10;
+		}
+	}
+
+	private updateTriggerCharacters(): void {
+
+		this.triggerCharacterListeners = dispose(this.triggerCharacterListeners);
+
+		if (this.editor.getConfiguration().readOnly
+			|| !this.editor.getModel()
+			|| !this.editor.getConfiguration().contribInfo.suggestOnTriggerCharacters) {
+
+			return;
+		}
+
+		const supportsByTriggerCharacter: { [ch: string]: ISuggestSupport[] } = Object.create(null);
+		for (const support of SuggestRegistry.all(this.editor.getModel())) {
+			if (isFalsyOrEmpty(support.triggerCharacters)) {
+				continue;
+			}
+			for (const ch of support.triggerCharacters) {
+				const array = supportsByTriggerCharacter[ch];
+				if (!array) {
+					supportsByTriggerCharacter[ch] = [support];
+				} else {
+					array.push(support);
+				}
+			}
+		}
+
+		forEach(supportsByTriggerCharacter, entry => {
+			this.triggerCharacterListeners.push(this.editor.addTypingListener(entry.key, () => {
+				this.trigger(true, false, entry.value);
+			}));
+		});
 	}
 
-	cancel(silent: boolean = false, retrigger: boolean = false): boolean {
+	// --- trigger/retrigger/cancel suggest
+
+	cancel(retrigger: boolean = false): boolean {
 		const actuallyCanceled = this.state !== State.Idle;
 
 		if (this.triggerAutoSuggestPromise) {
@@ -203,20 +265,23 @@ export class SuggestModel implements IDisposable {
 		}
 
 		this.state = State.Idle;
-		this.raw = null;
+		this.suggestionItems = null;
 		this.completionModel = null;
 		this.incomplete = false;
 		this.context = null;
 
-		if (!silent) {
-			this._onDidCancel.fire({ retrigger });
-		}
-
+		this._onDidCancel.fire({ retrigger });
 		return actuallyCanceled;
 	}
 
-	private isAutoSuggest(): boolean {
-		return this.state === State.Auto;
+	private updateActiveSuggestSession(): void {
+		if (this.state !== State.Idle) {
+			if (!SuggestRegistry.has(this.editor.getModel())) {
+				this.cancel();
+			} else {
+				this.trigger(this.state === State.Auto, true);
+			}
+		}
 	}
 
 	private onCursorChange(e: ICursorSelectionChangedEvent): void {
@@ -253,33 +318,20 @@ export class SuggestModel implements IDisposable {
 			this.cancel();
 
 			if (ctx.shouldAutoTrigger()) {
-				this.triggerAutoSuggestPromise = TPromise.timeout(this.autoSuggestDelay);
+				this.triggerAutoSuggestPromise = TPromise.timeout(this.quickSuggestDelay);
 				this.triggerAutoSuggestPromise.then(() => {
 					this.triggerAutoSuggestPromise = null;
 					this.trigger(true);
 				});
 			}
 
-		} else if (this.raw && this.incomplete) {
+		} else if (this.suggestionItems && this.incomplete) {
 			this.trigger(this.state === State.Auto, true);
 		} else {
 			this.onNewContext(ctx);
 		}
 	}
 
-	private onSuggestRegistryChange(): void {
-		if (this.state === State.Idle) {
-			return;
-		}
-
-		if (!SuggestRegistry.has(this.editor.getModel())) {
-			this.cancel();
-			return;
-		}
-
-		this.trigger(this.state === State.Auto, true);
-	}
-
 	public trigger(auto: boolean, retrigger: boolean = false, onlyFrom?: ISuggestSupport[]): void {
 
 		const model = this.editor.getModel();
@@ -295,7 +347,7 @@ export class SuggestModel implements IDisposable {
 		}
 
 		// Cancel previous requests, change state & update UI
-		this.cancel(false, retrigger);
+		this.cancel(retrigger);
 		this.state = auto ? State.Auto : State.Manual;
 		this._onDidTrigger.fire({ auto: this.isAutoSuggest() });
 
@@ -303,7 +355,7 @@ export class SuggestModel implements IDisposable {
 		this.context = ctx;
 
 		this.requestPromise = provideSuggestionItems(model, this.editor.getPosition(),
-			this.editor.getConfiguration().contribInfo.snippetSuggestions, onlyFrom).then(all => {
+			this.editor.getConfiguration().contribInfo.snippetSuggestions, onlyFrom).then(items => {
 
 			this.requestPromise = null;
 
@@ -311,19 +363,27 @@ export class SuggestModel implements IDisposable {
 				return;
 			}
 
-			this.raw = all;
-			this.incomplete = all.some(result => result.container.incomplete);
-
 			const model = this.editor.getModel();
-
 			if (!model) {
 				return;
 			}
 
+			this.suggestionItems = items;
+			this.incomplete = items.some(result => result.container.incomplete);
 			this.onNewContext(new Context(model, this.editor.getPosition(), auto));
+
 		}).then(null, onUnexpectedError);
 	}
 
+	private isAutoSuggest(): boolean {
+		return this.state === State.Auto;
+	}
+
+	public getTriggerPosition(): IPosition {
+		const {lineNumber, column} = this.context;
+		return { lineNumber, column };
+	}
+
 	private onNewContext(ctx: Context): void {
 		if (this.context && this.context.isDifferentContext(ctx)) {
 			if (this.context.shouldRetrigger(ctx)) {
@@ -335,11 +395,11 @@ export class SuggestModel implements IDisposable {
 			return;
 		}
 
-		if (this.raw) {
+		if (this.suggestionItems) {
 			let auto = this.isAutoSuggest();
 
 			let isFrozen = false;
-			if (this.completionModel && this.completionModel.raw === this.raw) {
+			if (this.completionModel && this.completionModel.raw === this.suggestionItems) {
 				const oldLineContext = this.completionModel.lineContext;
 				this.completionModel.lineContext = {
 					leadingLineContent: ctx.lineContentBefore,
@@ -353,7 +413,7 @@ export class SuggestModel implements IDisposable {
 					isFrozen = true;
 				}
 			} else {
-				this.completionModel = new CompletionModel(this.raw, ctx.lineContentBefore);
+				this.completionModel = new CompletionModel(this.suggestionItems, ctx.lineContentBefore);
 			}
 
 			this._onDidSuggest.fire({
@@ -363,32 +423,4 @@ export class SuggestModel implements IDisposable {
 			});
 		}
 	}
-
-	accept(suggestion: ISuggestion, overwriteBefore: number, overwriteAfter: number): boolean {
-		if (this.raw === null) {
-			return false;
-		}
-
-		this._onDidAccept.fire({
-			snippet: new CodeSnippet(suggestion.insertText),
-			overwriteBefore: overwriteBefore + (this.editor.getPosition().column - this.context.column),
-			overwriteAfter
-		});
-
-		this.cancel();
-		return true;
-	}
-
-	private onEditorConfigurationChange(): void {
-		this.autoSuggestDelay = this.editor.getConfiguration().contribInfo.quickSuggestionsDelay;
-
-		if (isNaN(this.autoSuggestDelay) || (!this.autoSuggestDelay && this.autoSuggestDelay !== 0) || this.autoSuggestDelay < 0) {
-			this.autoSuggestDelay = 10;
-		}
-	}
-
-	dispose(): void {
-		this.cancel(true);
-		this.toDispose = dispose(this.toDispose);
-	}
 }