Add isRetrigger field to signature help context

#54972 Instead of having a generic `Retrigger` reason, add a new field on the `SignatureHelpContext` that tracks if this was a retrigger or not. This allows retrigger for all the different trigger reasons, including invoke. Replace the old `Retriggger` trigger reason with `ContentChange` which tracks cursor movement and text updates.
microsoft · Oct 25, 2018 · f413197 · f413197
1 parent 9aa969c
commit f413197
Show file tree

Hide file tree

Showing 11 changed files with 69 additions and 41 deletions.
diff --git a/extensions/typescript-language-features/src/features/signatureHelp.ts b/extensions/typescript-language-features/src/features/signatureHelp.ts
@@ -73,16 +73,20 @@ class TypeScriptSignatureHelpProvider implements vscode.SignatureHelpProvider {
 
 function toTsTriggerReason(context: vscode.SignatureHelpContext): Proto.SignatureHelpTriggerReason {
 	switch (context.triggerReason) {
-		case vscode.SignatureHelpTriggerReason.Retrigger:
-			return { kind: 'retrigger' };
-
 		case vscode.SignatureHelpTriggerReason.TriggerCharacter:
 			if (context.triggerCharacter) {
-				return { kind: 'characterTyped', triggerCharacter: context.triggerCharacter as any };
+				if (context.isRetrigger) {
+					return { kind: 'retrigger', triggerCharacter: context.triggerCharacter as any };
+				} else {
+					return { kind: 'characterTyped', triggerCharacter: context.triggerCharacter as any };
+				}
 			} else {
 				return { kind: 'invoked' };
 			}
 
+		case vscode.SignatureHelpTriggerReason.ContentChange:
+			return context.isRetrigger ? { kind: 'retrigger' } : { kind: 'invoked' };
+
 		case vscode.SignatureHelpTriggerReason.Invoke:
 		default:
 			return { kind: 'invoked' };

diff --git a/src/vs/editor/common/modes.ts b/src/vs/editor/common/modes.ts
@@ -618,12 +618,13 @@ export interface SignatureHelp {
 export enum SignatureHelpTriggerReason {
 	Invoke = 1,
 	TriggerCharacter = 2,
-	Retrigger = 3,
+	ContentChange = 3,
 }
 
 export interface SignatureHelpContext {
-	triggerReason: SignatureHelpTriggerReason;
-	triggerCharacter?: string;
+	readonly triggerReason: SignatureHelpTriggerReason;
+	readonly triggerCharacter?: string;
+	readonly isRetrigger: boolean;
 }
 
 /**

diff --git a/src/vs/editor/common/standalone/standaloneEnums.ts b/src/vs/editor/common/standalone/standaloneEnums.ts
@@ -597,7 +597,7 @@ export enum CompletionTriggerKind {
 export enum SignatureHelpTriggerReason {
 	Invoke = 1,
 	TriggerCharacter = 2,
-	Retrigger = 3
+	ContentChange = 3
 }
 
 /**

diff --git a/src/vs/editor/contrib/parameterHints/parameterHints.ts b/src/vs/editor/contrib/parameterHints/parameterHints.ts
@@ -12,7 +12,7 @@ import { EditorContextKeys } from 'vs/editor/common/editorContextKeys';
 import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey';
 import { registerEditorAction, registerEditorContribution, ServicesAccessor, EditorAction, EditorCommand, registerEditorCommand } from 'vs/editor/browser/editorExtensions';
 import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
-import { ParameterHintsWidget } from './parameterHintsWidget';
+import { ParameterHintsWidget, TriggerContext } from './parameterHintsWidget';
 import { Context } from 'vs/editor/contrib/parameterHints/provideSignatureHelp';
 import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry';
 import * as modes from 'vs/editor/common/modes';
@@ -49,7 +49,7 @@ class ParameterHintsController implements IEditorContribution {
 		this.widget.next();
 	}
 
-	trigger(context: modes.SignatureHelpContext): void {
+	trigger(context: TriggerContext): void {
 		this.widget.trigger(context);
 	}
 
@@ -77,7 +77,9 @@ export class TriggerParameterHintsAction extends EditorAction {
 	public run(accessor: ServicesAccessor, editor: ICodeEditor): void {
 		let controller = ParameterHintsController.get(editor);
 		if (controller) {
-			controller.trigger({ triggerReason: modes.SignatureHelpTriggerReason.Invoke });
+			controller.trigger({
+				triggerReason: modes.SignatureHelpTriggerReason.Invoke
+			});
 		}
 	}
 }

diff --git a/src/vs/editor/contrib/parameterHints/parameterHintsWidget.ts b/src/vs/editor/contrib/parameterHints/parameterHintsWidget.ts
@@ -28,6 +28,11 @@ import { MarkdownRenderer } from 'vs/editor/contrib/markdown/markdownRenderer';
 
 const $ = dom.$;
 
+export interface TriggerContext {
+	readonly triggerReason: modes.SignatureHelpTriggerReason;
+	readonly triggerCharacter?: string;
+}
+
 export interface IHintEvent {
 	hints: modes.SignatureHelp;
 }
@@ -95,13 +100,20 @@ export class ParameterHintsModel extends Disposable {
 		}
 	}
 
-	trigger(context: modes.SignatureHelpContext, delay?: number): void {
+	trigger(context: TriggerContext, delay?: number): void {
 		if (!modes.SignatureHelpProviderRegistry.has(this.editor.getModel())) {
 			return;
 		}
 
+		const wasTriggered = this.isTriggered;
 		this.cancel(true);
-		this.triggerContext = context;
+
+		this.triggerContext = {
+			triggerReason: context.triggerReason,
+			triggerCharacter: context.triggerCharacter,
+			isRetrigger: wasTriggered
+		};
+
 		return this.throttledDelayer.schedule(delay);
 	}
 
@@ -112,7 +124,7 @@ export class ParameterHintsModel extends Disposable {
 
 		this.pending = true;
 
-		const triggerContext = this.triggerContext || { triggerReason: modes.SignatureHelpTriggerReason.Invoke };
+		const triggerContext = this.triggerContext || { triggerReason: modes.SignatureHelpTriggerReason.Invoke, isRetrigger: false };
 		this.triggerContext = undefined;
 
 		this.provideSignatureHelpRequest = createCancelablePromise(token =>
@@ -180,15 +192,10 @@ export class ParameterHintsModel extends Disposable {
 		const lastCharIndex = text.length - 1;
 		const triggerCharCode = text.charCodeAt(lastCharIndex);
 
-		if (this.isTriggered && this.retriggerChars.has(triggerCharCode)) {
-			this.trigger({
-				triggerReason: modes.SignatureHelpTriggerReason.Retrigger,
-				triggerCharacter: text.charAt(lastCharIndex)
-			});
-		} else if (this.triggerChars.has(triggerCharCode)) {
+		if (this.triggerChars.has(triggerCharCode) || this.isTriggered && this.retriggerChars.has(triggerCharCode)) {
 			this.trigger({
 				triggerReason: modes.SignatureHelpTriggerReason.TriggerCharacter,
-				triggerCharacter: text.charAt(lastCharIndex)
+				triggerCharacter: text.charAt(lastCharIndex),
 			});
 		}
 	}
@@ -197,13 +204,13 @@ export class ParameterHintsModel extends Disposable {
 		if (e.source === 'mouse') {
 			this.cancel();
 		} else if (this.isTriggered) {
-			this.trigger({ triggerReason: modes.SignatureHelpTriggerReason.Retrigger });
+			this.trigger({ triggerReason: modes.SignatureHelpTriggerReason.ContentChange });
 		}
 	}
 
 	private onModelContentChange(): void {
 		if (this.isTriggered) {
-			this.trigger({ triggerReason: modes.SignatureHelpTriggerReason.Retrigger });
+			this.trigger({ triggerReason: modes.SignatureHelpTriggerReason.ContentChange });
 		}
 	}
 
@@ -571,7 +578,7 @@ export class ParameterHintsWidget implements IContentWidget, IDisposable {
 		return ParameterHintsWidget.ID;
 	}
 
-	trigger(context: modes.SignatureHelpContext): void {
+	trigger(context: TriggerContext): void {
 		this.model.trigger(context, 0);
 	}
 

diff --git a/src/vs/editor/contrib/parameterHints/provideSignatureHelp.ts b/src/vs/editor/contrib/parameterHints/provideSignatureHelp.ts
@@ -27,4 +27,7 @@ export function provideSignatureHelp(model: ITextModel, position: Position, cont
 }
 
 registerDefaultLanguageCommand('_executeSignatureHelpProvider', (model, position) =>
-	provideSignatureHelp(model, position, { triggerReason: modes.SignatureHelpTriggerReason.Invoke }, CancellationToken.None));
+	provideSignatureHelp(model, position, {
+		triggerReason: modes.SignatureHelpTriggerReason.Invoke,
+		isRetrigger: false
+	}, CancellationToken.None));
diff --git a/src/vs/editor/contrib/parameterHints/test/parameterHintsModel.test.ts b/src/vs/editor/contrib/parameterHints/test/parameterHintsModel.test.ts
@@ -93,7 +93,8 @@ suite('ParameterHintsModel', () => {
 					editor.trigger('keyboard', Handler.Type, { text: triggerChar });
 				} else {
 					assert.strictEqual(invokeCount, 2);
-					assert.strictEqual(context.triggerReason, modes.SignatureHelpTriggerReason.Retrigger);
+					assert.strictEqual(context.triggerReason, modes.SignatureHelpTriggerReason.TriggerCharacter);
+					assert.ok(context.isRetrigger);
 					assert.strictEqual(context.triggerCharacter, triggerChar);
 					done();
 				}
@@ -149,7 +150,8 @@ suite('ParameterHintsModel', () => {
 
 			provideSignatureHelp(_model: ITextModel, _position: Position, _token: CancellationToken, context: modes.SignatureHelpContext): modes.SignatureHelp | Thenable<modes.SignatureHelp> {
 				++invokeCount;
-				assert.strictEqual(context.triggerReason, modes.SignatureHelpTriggerReason.Retrigger);
+				assert.strictEqual(context.triggerReason, modes.SignatureHelpTriggerReason.TriggerCharacter);
+				assert.ok(context.isRetrigger);
 				assert.strictEqual(context.triggerCharacter, 'c');
 
 				// Give some time to allow for later triggers
@@ -185,7 +187,8 @@ suite('ParameterHintsModel', () => {
 					// retrigger after delay for widget to show up
 					setTimeout(() => editor.trigger('keyboard', Handler.Type, { text: 'b' }), 50);
 				} else if (invokeCount === 2) {
-					assert.strictEqual(context.triggerReason, modes.SignatureHelpTriggerReason.Retrigger);
+					assert.strictEqual(context.triggerReason, modes.SignatureHelpTriggerReason.TriggerCharacter);
+					assert.ok(context.isRetrigger);
 					assert.strictEqual(context.triggerCharacter, 'b');
 					done();
 				} else {
@@ -272,7 +275,8 @@ suite('ParameterHintsModel', () => {
 					// retrigger after delay for widget to show up
 					setTimeout(() => editor.trigger('keyboard', Handler.Type, { text: retriggerChar }), 50);
 				} else if (invokeCount === 2) {
-					assert.strictEqual(context.triggerReason, modes.SignatureHelpTriggerReason.Retrigger);
+					assert.strictEqual(context.triggerReason, modes.SignatureHelpTriggerReason.TriggerCharacter);
+					assert.ok(context.isRetrigger);
 					assert.strictEqual(context.triggerCharacter, retriggerChar);
 					done();
 				} else {

diff --git a/src/vs/monaco.d.ts b/src/vs/monaco.d.ts
@@ -4921,12 +4921,13 @@ declare namespace monaco.languages {
 	export enum SignatureHelpTriggerReason {
 		Invoke = 1,
 		TriggerCharacter = 2,
-		Retrigger = 3
+		ContentChange = 3
 	}
 
 	export interface SignatureHelpContext {
-		triggerReason: SignatureHelpTriggerReason;
-		triggerCharacter?: string;
+		readonly triggerReason: SignatureHelpTriggerReason;
+		readonly triggerCharacter?: string;
+		readonly isRetrigger: boolean;
 	}
 
 	/**

diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts
@@ -1094,12 +1094,9 @@ declare module 'vscode' {
 		TriggerCharacter = 2,
 
 		/**
-		 * Signature help was retriggered.
-		 *
-		 * Retriggers occur when the signature help is already active and can be caused by typing a trigger character
-		 * or by a cursor move.
+		 * Signature help was triggered by the cursor moving or by the document content changing.
 		 */
-		Retrigger = 3,
+		ContentChange = 3,
 	}
 
 	/**
@@ -1115,9 +1112,18 @@ declare module 'vscode' {
 		/**
 		 * Character that caused signature help to be requested.
 		 *
-		 * This is `undefined` for manual triggers or retriggers for a cursor move.
+		 * This is `undefined` when signature help is not triggered by typing, such as when invoking signature help
+		 * or when moving the cursor.
 		 */
 		readonly triggerCharacter?: string;
+
+		/**
+		 * Whether or not signature help was previously showing when triggered.
+		 *
+		 * Retriggers occur when the signature help is already active and can be caused by typing a trigger character
+		 * or by a cursor move.
+		 */
+		readonly isRetrigger: boolean;
 	}
 
 	export interface SignatureHelpProvider {

diff --git a/src/vs/workbench/api/node/extHostTypes.ts b/src/vs/workbench/api/node/extHostTypes.ts
@@ -1115,7 +1115,7 @@ export class SignatureHelp {
 export enum SignatureHelpTriggerReason {
 	Invoke = 1,
 	TriggerCharacter = 2,
-	Retrigger = 3,
+	ContentChange = 3,
 }
 
 export enum CompletionTriggerKind {

diff --git a/src/vs/workbench/test/electron-browser/api/extHostLanguageFeatures.test.ts b/src/vs/workbench/test/electron-browser/api/extHostLanguageFeatures.test.ts
@@ -864,7 +864,7 @@ suite('ExtHostLanguageFeatures', function () {
 
 		return rpcProtocol.sync().then(() => {
 
-			return provideSignatureHelp(model, new EditorPosition(1, 1), { triggerReason: modes.SignatureHelpTriggerReason.Invoke }, CancellationToken.None).then(value => {
+			return provideSignatureHelp(model, new EditorPosition(1, 1), { triggerReason: modes.SignatureHelpTriggerReason.Invoke, isRetrigger: false }, CancellationToken.None).then(value => {
 				assert.ok(value);
 			});
 		});
@@ -879,7 +879,7 @@ suite('ExtHostLanguageFeatures', function () {
 
 		return rpcProtocol.sync().then(() => {
 
-			return provideSignatureHelp(model, new EditorPosition(1, 1), { triggerReason: modes.SignatureHelpTriggerReason.Invoke }, CancellationToken.None).then(value => {
+			return provideSignatureHelp(model, new EditorPosition(1, 1), { triggerReason: modes.SignatureHelpTriggerReason.Invoke, isRetrigger: false }, CancellationToken.None).then(value => {
 				assert.equal(value, undefined);
 			});
 		});