Skip to content

Commit

Permalink
Pass completion context to shouldShowContinuousHint() (jupyterlab#1…
Browse files Browse the repository at this point in the history
…5015)

* Who tests the tests? (fix error in reconciliator test definition)

* Add test for context being passed to shouldShowContinuousHint()

* Pass context to `shouldShowContinuousHint()`

* Remove spurious type cast
  • Loading branch information
krassowski authored Aug 28, 2023
1 parent eabf8aa commit 7b1effc
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 18 deletions.
5 changes: 2 additions & 3 deletions packages/completer/src/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,8 @@ export class CompletionHandler implements IDisposable {
}
if (
this._autoCompletion &&
(this._reconciliator as IProviderReconciliator)
.shouldShowContinuousHint &&
(this._reconciliator as IProviderReconciliator).shouldShowContinuousHint(
this._reconciliator.shouldShowContinuousHint &&
this._reconciliator.shouldShowContinuousHint(
this.completer.isVisible,
changed
)
Expand Down
3 changes: 2 additions & 1 deletion packages/completer/src/reconciliator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ export class ProviderReconciliator implements IProviderReconciliator {
if (this._providers[0].shouldShowContinuousHint) {
return this._providers[0].shouldShowContinuousHint(
completerIsVisible,
changed
changed,
this._context
);
}
return this._defaultShouldShowContinuousHint(completerIsVisible, changed);
Expand Down
48 changes: 35 additions & 13 deletions packages/metapackage/test/completer/handler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class TestCompletionHandler extends CompletionHandler {
}

class FooCompletionProvider implements ICompletionProvider {
methods: CompletionTriggerKind[] = [];
triggers: CompletionTriggerKind[] = [];

constructor(private _continuousHint: boolean) {}

Expand All @@ -75,11 +75,15 @@ class FooCompletionProvider implements ICompletionProvider {
context: ICompletionContext,
trigger: CompletionTriggerKind
): Promise<CompletionHandler.ICompletionItemsReply> {
this.methods.push(trigger);
this.triggers.push(trigger);
return Promise.resolve({ start: 0, end: 0, items: [] });
}

shouldShowContinuousHint(completerIsVisible: boolean, changed: any) {
shouldShowContinuousHint(
completerIsVisible: boolean,
changed: SourceChange,
context?: ICompletionContext
) {
return this._continuousHint;
}
}
Expand Down Expand Up @@ -347,19 +351,21 @@ describe('@jupyterlab/completer', () => {
});
});

describe('#CompletionTriggerKind', () => {
describe('#autoCompletion', () => {
let anchor: CodeEditorWrapper;
let provider: FooCompletionProvider;
let handler: CompletionHandler;
let context: symbol;

beforeAll(async () => {
anchor = createEditorWidget();
Widget.attach(anchor, document.body);
context = Symbol();

provider = new FooCompletionProvider(true);
handler = new CompletionHandler({
reconciliator: new ProviderReconciliator({
context: null as any,
context: context as any,
providers: [provider],
timeout: 0
}),
Expand All @@ -379,30 +385,46 @@ describe('@jupyterlab/completer', () => {
handler.dispose();
});

it('should use CompletionTriggerKind.Invoked', () => {
expect(provider.methods.length).toEqual(0);
it('should use Invoked for invoke()', () => {
expect(provider.triggers.length).toEqual(0);

handler.editor!.model.sharedModel.setSource('foo.');
anchor.node.focus();
anchor.editor.setCursorPosition({ line: 0, column: 4 });
handler.invoke();

expect(provider.methods).toEqual(
expect(provider.triggers).toEqual(
expect.arrayContaining([CompletionTriggerKind.Invoked])
);
});

it('should use CompletionTriggerKind.TriggerCharacter', () => {
// this test depends on the previous one ('should use CompletionTriggerKind.Invoked').
expect(provider.methods.length).toEqual(1);
it('should use TriggerCharacter for typed text', () => {
// this test depends on the previous one ('should use Invoked for invoke()').
expect(provider.triggers.length).toEqual(1);

handler.editor!.model.sharedModel.updateSource(4, 4, 'a');

expect(provider.methods.length).toEqual(2);
expect(provider.methods).toEqual(
expect(provider.triggers.length).toEqual(2);
expect(provider.triggers).toEqual(
expect.arrayContaining([CompletionTriggerKind.TriggerCharacter])
);
});

it('should pass context to `shouldShowContinuousHint()`', () => {
handler.editor!.model.sharedModel.setSource('foo.');
anchor.node.focus();
anchor.editor.setCursorPosition({ line: 0, column: 4 });
provider.shouldShowContinuousHint = jest.fn();
handler.editor!.model.sharedModel.updateSource(4, 4, 'a');
expect(provider.shouldShowContinuousHint).toHaveBeenCalledTimes(1);
expect(provider.shouldShowContinuousHint).toHaveBeenLastCalledWith(
false,
{
sourceChange: [{ retain: 4 }, { insert: 'a' }]
} as SourceChange,
context
);
});
});

it('should update cursor position after autocomplete on empty word', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/metapackage/test/completer/reconciliator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ describe('completer/reconciliator', () => {
fooProvider2.shouldShowContinuousHint = jest.fn();
const reconciliator = new ProviderReconciliator({
...defaultOptions,
providers: [fooProvider1, fooProvider1]
providers: [fooProvider1, fooProvider2]
});
reconciliator.shouldShowContinuousHint(true, null as any);
expect(fooProvider1.shouldShowContinuousHint).toHaveBeenCalledTimes(1);
Expand Down

0 comments on commit 7b1effc

Please sign in to comment.