Skip to content

Commit

Permalink
Revert "Add activeSignatureHelp to SignatureHelpContext (#65440)"
Browse files Browse the repository at this point in the history
This reverts commit 4a38520.
  • Loading branch information
joaomoreno committed Jan 8, 2019
1 parent ddd3d15 commit 7b375fc
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 122 deletions.
1 change: 0 additions & 1 deletion src/vs/editor/common/modes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,6 @@ export interface SignatureHelpContext {
readonly triggerKind: SignatureHelpTriggerKind;
readonly triggerCharacter?: string;
readonly isRetrigger: boolean;
readonly activeSignatureHelp?: SignatureHelp;
}

/**
Expand Down
4 changes: 1 addition & 3 deletions src/vs/editor/contrib/parameterHints/parameterHintsModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,11 @@ export class ParameterHintsModel extends Disposable {
}

cancel(silent: boolean = false): void {
const priorState = this.state;
this.state = DefaultState;

this.throttledDelayer.cancel();

if (!silent && priorState.type === ActiveState.type) {
if (!silent) {
this._onChangedHints.fire(undefined);
}

Expand All @@ -99,7 +98,6 @@ 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,13 @@ 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
setTimeout(() => editor.trigger('keyboard', Handler.Type, { text: triggerChar }), 50);
editor.trigger('keyboard', Handler.Type, { text: triggerChar });
} 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;
Expand Down Expand Up @@ -128,7 +124,6 @@ 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();
Expand All @@ -138,7 +133,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;
Expand Down Expand Up @@ -303,82 +298,4 @@ 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<modes.SignatureHelp | undefined> {
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<modes.SignatureHelp | undefined> {
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<modes.SignatureHelp | undefined>(resolve => {
const sub = model.onChangedHints(e => {
sub.dispose();
return resolve(e);
});
});
}

1 change: 0 additions & 1 deletion src/vs/monaco.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4919,7 +4919,6 @@ declare namespace monaco.languages {
readonly triggerKind: SignatureHelpTriggerKind;
readonly triggerCharacter?: string;
readonly isRetrigger: boolean;
readonly activeSignatureHelp?: SignatureHelp;
}

/**
Expand Down
11 changes: 0 additions & 11 deletions src/vs/vscode.proposed.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1102,15 +1102,4 @@ 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
}
25 changes: 5 additions & 20 deletions src/vs/workbench/api/node/extHostLanguageFeatures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -731,36 +731,21 @@ class SignatureHelpAdapter {

constructor(
private readonly _documents: ExtHostDocuments,
private readonly _provider: vscode.SignatureHelpProvider,
private readonly _heap: ExtHostHeapService,
private readonly _provider: vscode.SignatureHelpProvider
) { }

provideSignatureHelp(resource: URI, position: IPosition, context: modes.SignatureHelpContext, token: CancellationToken): Promise<modes.SignatureHelp> {

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, vscodeContext)).then(value => {
return asPromise(() => this._provider.provideSignatureHelp(doc, pos, token, context)).then(value => {
if (value) {
const id = this._heap.keep(value);
return ObjectIdentifier.mixin(typeConvert.SignatureHelp.from(value), id);
return typeConvert.SignatureHelp.from(value);
}
return undefined;
});
}

private reviveContext(context: modes.SignatureHelpContext): vscode.SignatureHelpContext {
let activeSignatureHelp: vscode.SignatureHelp | undefined = undefined;
if (context.activeSignatureHelp) {
const saved = this._heap.get<vscode.SignatureHelp>(ObjectIdentifier.of(context.activeSignatureHelp));
if (saved) {
activeSignatureHelp = saved;
} else {
activeSignatureHelp = typeConvert.SignatureHelp.to(context.activeSignatureHelp);
}
}
return { ...context, activeSignatureHelp };
}
}

class LinkProviderAdapter {
Expand Down Expand Up @@ -1239,7 +1224,7 @@ export class ExtHostLanguageFeatures implements ExtHostLanguageFeaturesShape {
? { triggerCharacters: metadataOrTriggerChars, retriggerCharacters: [] }
: metadataOrTriggerChars;

const handle = this._addNewAdapter(new SignatureHelpAdapter(this._documents, provider, this._heapService), extension);
const handle = this._addNewAdapter(new SignatureHelpAdapter(this._documents, provider), extension);
this._proxy.$registerSignatureHelpProvider(handle, this._transformDocumentSelector(selector), metadata);
return this._createDisposable(handle);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,6 @@ suite('ExtHostLanguageFeatures', function () {
});
});
});

test('Parameter Hints, evil provider', function () {

disposables.push(extHost.registerSignatureHelpProvider(defaultExtension, defaultSelector, new class implements vscode.SignatureHelpProvider {
Expand Down

0 comments on commit 7b375fc

Please sign in to comment.