Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds rank to ICompletionProvider #14800

Merged
merged 4 commits into from Jul 25, 2023
Merged

Conversation

hbcarlos
Copy link
Member

@hbcarlos hbcarlos commented Jul 7, 2023

References

fixes #14790

Code changes

Adds rank to the ICompletionProvider and sets a rank of ~500 to the default providers.

User-facing changes

The default rank of the providers.

Backwards-incompatible changes

N/A

@hbcarlos hbcarlos self-assigned this Jul 7, 2023
@jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

packages/completer/src/tokens.ts Show resolved Hide resolved
* priority to your provider, use a rank of 1000 or above.
*
* The rank is optional for backwards compatibility. If the rank is `undefined`,
* it will assign a rank of [1, 499] making the provider available but with a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this limit implemented?

Copy link
Member Author

@hbcarlos hbcarlos Jul 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the upper limit is not implemented, but you need 49 providers to reach a rank of 500.

packages/completer/src/tokens.ts Outdated Show resolved Hide resolved
@krassowski krassowski added this to the 4.0.x milestone Jul 7, 2023
hbcarlos and others added 3 commits July 11, 2023 16:15
Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com>
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hbcarlos

I would wait before merging so @krassowski can have another look at this one.

Copy link
Member

@krassowski krassowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failing tests is relevant:

jupyterlab › test/jupyterlab/completer.test.ts:85:9 › Completer › Notebook › Token completions show up without running the cell when in the same cell

packages/completer-extension/schema/manager.json Outdated Show resolved Hide resolved
Copy link
Member

@krassowski krassowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the relevant test passing now - thank you!

@krassowski krassowski merged commit 8cd0988 into jupyterlab:main Jul 25, 2023
76 of 81 checks passed
@krassowski
Copy link
Member

@meeseeksdev please backport to 4.0.x

meeseeksmachine pushed a commit to meeseeksmachine/jupyterlab that referenced this pull request Jul 25, 2023
krassowski pushed a commit that referenced this pull request Jul 30, 2023
Co-authored-by: Carlos Herrero <26092748+hbcarlos@users.noreply.github.com>
@hbcarlos hbcarlos deleted the fix_#14790 branch July 31, 2023 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should completer providers be able to advertise suggested default rank?
3 participants