Merge multiple triggers for parameter hints instead of always using latest

mjbvz · mjbvz · commit a64ca747fbda · 2019-10-18T14:36:21.000-07:00
Fixes microsoft#82825
diff --git a/src/vs/editor/contrib/parameterHints/parameterHintsModel.ts b/src/vs/editor/contrib/parameterHints/parameterHintsModel.ts
@@ -55,6 +55,7 @@ export class ParameterHintsModel extends Disposable {
 	private readonly editor: ICodeEditor;
 	private triggerOnType = false;
 	private _state: ParameterHintState.State = ParameterHintState.Default;
+	private _pendingTriggers: TriggerContext[] = [];
 	private readonly _lastSignatureHelpResult = this._register(new MutableDisposable<modes.SignatureHelpResult>());
 	private triggerChars = new CharacterSet();
 	private retriggerChars = new CharacterSet();
@@ -109,13 +110,12 @@ export class ParameterHintsModel extends Disposable {
 		}
 
 		const triggerId = ++this.triggerId;
-		this.throttledDelayer.trigger(
-			() => this.doTrigger({
-				triggerKind: context.triggerKind,
-				triggerCharacter: context.triggerCharacter,
-				isRetrigger: this.state.type === ParameterHintState.Type.Active || this.state.type === ParameterHintState.Type.Pending,
-				activeSignatureHelp: this.state.type === ParameterHintState.Type.Active ? this.state.hints : undefined
-			}, triggerId), delay).then(undefined, onUnexpectedError);
+
+		this._pendingTriggers.push(context);
+		this.throttledDelayer.trigger(() => {
+			return this.doTrigger(triggerId);
+		}, delay)
+			.catch(onUnexpectedError);
 	}
 
 	public next(): void {
@@ -165,9 +165,26 @@ export class ParameterHintsModel extends Disposable {
 		this._onChangedHints.fire(this.state.hints);
 	}
 
-	private doTrigger(triggerContext: modes.SignatureHelpContext, triggerId: number): Promise<boolean> {
+	private doTrigger(triggerId: number): Promise<boolean> {
+		const isRetrigger = this.state.type === ParameterHintState.Type.Active || this.state.type === ParameterHintState.Type.Pending;
+		const activeSignatureHelp = this.state.type === ParameterHintState.Type.Active ? this.state.hints : undefined;
+
 		this.cancel(true);
 
+		if (this._pendingTriggers.length === 0) {
+			return Promise.resolve(false);
+		}
+
+		const context: TriggerContext = this._pendingTriggers.reduce(mergeTriggerContexts);
+		this._pendingTriggers = [];
+
+		const triggerContext = {
+			triggerKind: context.triggerKind,
+			triggerCharacter: context.triggerCharacter,
+			isRetrigger: isRetrigger,
+			activeSignatureHelp: activeSignatureHelp
+		};
+
 		if (!this.editor.hasModel()) {
 			return Promise.resolve(false);
 		}
@@ -284,3 +301,19 @@ export class ParameterHintsModel extends Disposable {
 		super.dispose();
 	}
 }
+
+function mergeTriggerContexts(previous: TriggerContext, current: TriggerContext) {
+	switch (current.triggerKind) {
+		case modes.SignatureHelpTriggerKind.Invoke:
+			// Invoke overrides previous triggers.
+			return current;
+
+		case modes.SignatureHelpTriggerKind.ContentChange:
+			// Ignore content changes triggers
+			return previous;
+
+		case modes.SignatureHelpTriggerKind.TriggerCharacter:
+		default:
+			return current;
+	}
+}
diff --git a/src/vs/editor/contrib/parameterHints/test/parameterHintsModel.test.ts b/src/vs/editor/contrib/parameterHints/test/parameterHintsModel.test.ts
@@ -425,6 +425,43 @@ suite('ParameterHintsModel', () => {
 		assert.strictEqual(secondHint.activeSignature, 1);
 		assert.strictEqual(secondHint.signatures[0].parameters[0].label, paramterLabel);
 	});
+
+	test('Quick typing should use the first trigger character', async () => {
+		const editor = createMockEditor('');
+		const model = new ParameterHintsModel(editor, 50);
+		disposables.add(model);
+
+		const triggerCharacter = 'a';
+
+		let invokeCount = 0;
+		disposables.add(modes.SignatureHelpProviderRegistry.register(mockFileSelector, new class implements modes.SignatureHelpProvider {
+			signatureHelpTriggerCharacters = [triggerCharacter];
+			signatureHelpRetriggerCharacters = [];
+
+			provideSignatureHelp(_model: ITextModel, _position: Position, _token: CancellationToken, context: modes.SignatureHelpContext): modes.SignatureHelpResult | Promise<modes.SignatureHelpResult> {
+				try {
+					++invokeCount;
+
+					if (invokeCount === 1) {
+						assert.strictEqual(context.triggerKind, modes.SignatureHelpTriggerKind.TriggerCharacter);
+						assert.strictEqual(context.triggerCharacter, triggerCharacter);
+					} else {
+						assert.fail('Unexpected invoke');
+					}
+
+					return emptySigHelpResult;
+				} catch (err) {
+					console.error(err);
+					throw err;
+				}
+			}
+		}));
+
+		editor.trigger('keyboard', Handler.Type, { text: triggerCharacter });
+		editor.trigger('keyboard', Handler.Type, { text: 'x' });
+
+		await getNextHint(model);
+	});
 });
 
 function getNextHint(model: ParameterHintsModel) {
