diff --git a/src/vs/editor/common/modes.ts b/src/vs/editor/common/modes.ts index 89ef48ed14171..6917144a7d121 100644 --- a/src/vs/editor/common/modes.ts +++ b/src/vs/editor/common/modes.ts @@ -618,6 +618,7 @@ export interface SignatureHelpContext { readonly triggerKind: SignatureHelpTriggerKind; readonly triggerCharacter?: string; readonly isRetrigger: boolean; + readonly activeSignatureHelp?: SignatureHelp; } /** diff --git a/src/vs/editor/contrib/hover/modesGlyphHover.ts b/src/vs/editor/contrib/hover/modesGlyphHover.ts index 9b1260a6c93a2..e19e55e74bd61 100644 --- a/src/vs/editor/contrib/hover/modesGlyphHover.ts +++ b/src/vs/editor/contrib/hover/modesGlyphHover.ts @@ -43,28 +43,25 @@ class MarginComputer implements IHoverComputer { }; }; - let lineDecorations = this._editor.getLineDecorations(this._lineNumber); + const lineDecorations = this._editor.getLineDecorations(this._lineNumber); - let result: IHoverMessage[] = []; + const result: IHoverMessage[] = []; if (!lineDecorations) { return result; } - for (let i = 0, len = lineDecorations.length; i < len; i++) { - let d = lineDecorations[i]; - + for (const d of lineDecorations) { if (!d.options.glyphMarginClassName) { continue; } const hoverMessage = d.options.glyphMarginHoverMessage; - if (!hoverMessage || isEmptyMarkdownString(hoverMessage)) { continue; } if (Array.isArray(hoverMessage)) { - result = result.concat(hoverMessage.map(toHoverMessage)); + result.push(...hoverMessage.map(toHoverMessage)); } else { result.push(toHoverMessage(hoverMessage)); } diff --git a/src/vs/editor/contrib/parameterHints/parameterHintsModel.ts b/src/vs/editor/contrib/parameterHints/parameterHintsModel.ts index c014294e5f701..efded57afffb3 100644 --- a/src/vs/editor/contrib/parameterHints/parameterHintsModel.ts +++ b/src/vs/editor/contrib/parameterHints/parameterHintsModel.ts @@ -72,11 +72,12 @@ export class ParameterHintsModel extends Disposable { } cancel(silent: boolean = false): void { + const priorState = this.state; this.state = DefaultState; this.throttledDelayer.cancel(); - if (!silent) { + if (!silent && priorState.type === ActiveState.type) { this._onChangedHints.fire(undefined); } @@ -98,6 +99,7 @@ export class ParameterHintsModel extends Disposable { triggerKind: context.triggerKind, triggerCharacter: context.triggerCharacter, isRetrigger: this.state.type === ActiveState.type || this.state.type === PendingState.type, + activeSignatureHelp: this.state.type === ActiveState.type ? this.state.hints : undefined }, triggerId), delay).then(undefined, onUnexpectedError); } diff --git a/src/vs/editor/contrib/parameterHints/test/parameterHintsModel.test.ts b/src/vs/editor/contrib/parameterHints/test/parameterHintsModel.test.ts index ff1931ae1e5e5..6c82e33ecf159 100644 --- a/src/vs/editor/contrib/parameterHints/test/parameterHintsModel.test.ts +++ b/src/vs/editor/contrib/parameterHints/test/parameterHintsModel.test.ts @@ -90,13 +90,17 @@ suite('ParameterHintsModel', () => { assert.strictEqual(context.triggerKind, modes.SignatureHelpTriggerKind.TriggerCharacter); assert.strictEqual(context.triggerCharacter, triggerChar); assert.strictEqual(context.isRetrigger, false); + assert.strictEqual(context.activeSignatureHelp, undefined); + // Retrigger - editor.trigger('keyboard', Handler.Type, { text: triggerChar }); + setTimeout(() => editor.trigger('keyboard', Handler.Type, { text: triggerChar }), 50); } else { assert.strictEqual(invokeCount, 2); assert.strictEqual(context.triggerKind, modes.SignatureHelpTriggerKind.TriggerCharacter); assert.strictEqual(context.isRetrigger, true); assert.strictEqual(context.triggerCharacter, triggerChar); + assert.strictEqual(context.activeSignatureHelp, emptySigHelpResult); + done(); } return emptySigHelpResult; @@ -124,6 +128,7 @@ suite('ParameterHintsModel', () => { assert.strictEqual(context.triggerKind, modes.SignatureHelpTriggerKind.TriggerCharacter); assert.strictEqual(context.triggerCharacter, triggerChar); assert.strictEqual(context.isRetrigger, false); + assert.strictEqual(context.activeSignatureHelp, undefined); // Cancel and retrigger hintModel.cancel(); @@ -133,7 +138,7 @@ suite('ParameterHintsModel', () => { assert.strictEqual(context.triggerKind, modes.SignatureHelpTriggerKind.TriggerCharacter); assert.strictEqual(context.triggerCharacter, triggerChar); assert.strictEqual(context.isRetrigger, false); - + assert.strictEqual(context.activeSignatureHelp, undefined); done(); } return emptySigHelpResult; @@ -298,4 +303,82 @@ suite('ParameterHintsModel', () => { // But a trigger character should editor.trigger('keyboard', Handler.Type, { text: triggerChar }); }); + + test('should use first result from multiple providers', async () => { + const triggerChar = 'a'; + const firstProviderId = 'firstProvider'; + const secondProviderId = 'secondProvider'; + const paramterLabel = 'parameter'; + + const editor = createMockEditor(''); + const model = new ParameterHintsModel(editor, 5); + disposables.push(model); + + disposables.push(modes.SignatureHelpProviderRegistry.register(mockFileSelector, new class implements modes.SignatureHelpProvider { + signatureHelpTriggerCharacters = [triggerChar]; + signatureHelpRetriggerCharacters = []; + + async provideSignatureHelp(_model: ITextModel, _position: Position, _token: CancellationToken, context: modes.SignatureHelpContext): Promise { + if (!context.isRetrigger) { + // retrigger after delay for widget to show up + setTimeout(() => editor.trigger('keyboard', Handler.Type, { text: triggerChar }), 50); + + return { + activeParameter: 0, + activeSignature: 0, + signatures: [{ + label: firstProviderId, + parameters: [ + { label: paramterLabel } + ] + }] + }; + } + + return undefined; + } + })); + + disposables.push(modes.SignatureHelpProviderRegistry.register(mockFileSelector, new class implements modes.SignatureHelpProvider { + signatureHelpTriggerCharacters = [triggerChar]; + signatureHelpRetriggerCharacters = []; + + async provideSignatureHelp(_model: ITextModel, _position: Position, _token: CancellationToken, context: modes.SignatureHelpContext): Promise { + if (context.isRetrigger) { + return { + activeParameter: 0, + activeSignature: context.activeSignatureHelp ? context.activeSignatureHelp.activeSignature + 1 : 0, + signatures: [{ + label: secondProviderId, + parameters: context.activeSignatureHelp ? context.activeSignatureHelp.signatures[0].parameters : [] + }] + }; + } + + return undefined; + } + })); + + editor.trigger('keyboard', Handler.Type, { text: triggerChar }); + + const firstHint = await getNextHint(model); + assert.strictEqual(firstHint!.signatures[0].label, firstProviderId); + assert.strictEqual(firstHint!.activeSignature, 0); + assert.strictEqual(firstHint!.signatures[0].parameters[0].label, paramterLabel); + + const secondHint = await getNextHint(model); + assert.strictEqual(secondHint!.signatures[0].label, secondProviderId); + assert.strictEqual(secondHint!.activeSignature, 1); + assert.strictEqual(secondHint!.signatures[0].parameters[0].label, paramterLabel); + }); }); + +function getNextHint(model: ParameterHintsModel) { + return new Promise(resolve => { + const sub = model.onChangedHints(e => { + sub.dispose(); + return resolve(e); + }); + }); +} + diff --git a/src/vs/monaco.d.ts b/src/vs/monaco.d.ts index ee181c5861bc6..b484673dbb573 100644 --- a/src/vs/monaco.d.ts +++ b/src/vs/monaco.d.ts @@ -4919,6 +4919,7 @@ declare namespace monaco.languages { readonly triggerKind: SignatureHelpTriggerKind; readonly triggerCharacter?: string; readonly isRetrigger: boolean; + readonly activeSignatureHelp?: SignatureHelp; } /** diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index d951c780a483f..2b0a298d872b2 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1102,4 +1102,15 @@ declare module 'vscode' { } //#endregion + + //#region SignatureHelpContext active paramters - mjbvz + export interface SignatureHelpContext { + /** + * The currently active [`SignatureHelp`](#SignatureHelp). + * + * Will have the [`SignatureHelp.activeSignature`] field updated based on user arrowing through sig help + */ + readonly activeSignatureHelp?: SignatureHelp; + } + //#endregion } diff --git a/src/vs/workbench/api/node/extHostLanguageFeatures.ts b/src/vs/workbench/api/node/extHostLanguageFeatures.ts index 27721c77ebd8b..50f967e18d810 100644 --- a/src/vs/workbench/api/node/extHostLanguageFeatures.ts +++ b/src/vs/workbench/api/node/extHostLanguageFeatures.ts @@ -731,21 +731,36 @@ class SignatureHelpAdapter { constructor( private readonly _documents: ExtHostDocuments, - private readonly _provider: vscode.SignatureHelpProvider + private readonly _provider: vscode.SignatureHelpProvider, + private readonly _heap: ExtHostHeapService, ) { } provideSignatureHelp(resource: URI, position: IPosition, context: modes.SignatureHelpContext, token: CancellationToken): Promise { - const doc = this._documents.getDocumentData(resource).document; const pos = typeConvert.Position.to(position); + const vscodeContext = this.reviveContext(context); - return asPromise(() => this._provider.provideSignatureHelp(doc, pos, token, context)).then(value => { + return asPromise(() => this._provider.provideSignatureHelp(doc, pos, token, vscodeContext)).then(value => { if (value) { - return typeConvert.SignatureHelp.from(value); + const id = this._heap.keep(value); + return ObjectIdentifier.mixin(typeConvert.SignatureHelp.from(value), id); } return undefined; }); } + + private reviveContext(context: modes.SignatureHelpContext): vscode.SignatureHelpContext { + let activeSignatureHelp: vscode.SignatureHelp | undefined = undefined; + if (context.activeSignatureHelp) { + const saved = this._heap.get(ObjectIdentifier.of(context.activeSignatureHelp)); + if (saved) { + activeSignatureHelp = saved; + } else { + activeSignatureHelp = typeConvert.SignatureHelp.to(context.activeSignatureHelp); + } + } + return { ...context, activeSignatureHelp }; + } } class LinkProviderAdapter { @@ -1224,7 +1239,7 @@ export class ExtHostLanguageFeatures implements ExtHostLanguageFeaturesShape { ? { triggerCharacters: metadataOrTriggerChars, retriggerCharacters: [] } : metadataOrTriggerChars; - const handle = this._addNewAdapter(new SignatureHelpAdapter(this._documents, provider), extension); + const handle = this._addNewAdapter(new SignatureHelpAdapter(this._documents, provider, this._heapService), extension); this._proxy.$registerSignatureHelpProvider(handle, this._transformDocumentSelector(selector), metadata); return this._createDisposable(handle); } diff --git a/src/vs/workbench/test/electron-browser/api/extHostLanguageFeatures.test.ts b/src/vs/workbench/test/electron-browser/api/extHostLanguageFeatures.test.ts index c1916f66807bd..9fe8565d7f31f 100644 --- a/src/vs/workbench/test/electron-browser/api/extHostLanguageFeatures.test.ts +++ b/src/vs/workbench/test/electron-browser/api/extHostLanguageFeatures.test.ts @@ -892,6 +892,7 @@ suite('ExtHostLanguageFeatures', function () { }); }); }); + test('Parameter Hints, evil provider', function () { disposables.push(extHost.registerSignatureHelpProvider(defaultExtension, defaultSelector, new class implements vscode.SignatureHelpProvider {