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

Optional prefix arg for dynamic key completions #13921

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

TomNicholas
Copy link
Contributor

@TomNicholas TomNicholas commented Feb 2, 2023

Closes #12420 by adding an optional prefix argument to _ipython_key_completions_.

@TomNicholas
Copy link
Contributor Author

Q: Should I have done this using a custom completer instead? I couldn't find any documentation on how to write or register custom completers...

@TomNicholas
Copy link
Contributor Author

@meeseeksdev tag tab-completion

@krassowski
Copy link
Member

If _ipython_key_completions_ were to be extended, I would think that it might be worth adding a full support for the Matcher API (https://ipython.readthedocs.io/en/stable/api/generated/IPython.core.completer.html#matcher-api). What do you think? What you are proposing would already make the signature compatible with MatcherAPIv1, but instead of inspecting the signature we could require users to use a decorator that would mark it with matcher_api_version = 1 attribute (not sure on this yet).

As for adding a custom completers, you can do this by appending to IPCompleter.custom_matchers list (but that does not resolve the relevant object for you).

@TomNicholas
Copy link
Contributor Author

TomNicholas commented Feb 3, 2023

Hi @krassowski - thanks for the quick reply!

What do you think?

I'm potentially open to working on generalising this, but this is my first PR to ipython's main codebase, and I would need a bit of guidance.

I've read through IPython.core.completer.py, but I'm still not sure exactly what you're suggesting. My understanding is that there is already a dict_key_matcher, which implements the Matcher API, and can get custom keys via ._get_keys() - that's what my PR changes.

Are you suggesting we define a new matcher method on IPCompleter? How would that work exactly?

As for adding a custom completers, you can do this by appending to IPCompleter.custom_matchers list (but that does not resolve the relevant object for you).

I also don't really understand if this is a potential solution to my original problem or just a dead end.

@Carreau
Copy link
Member

Carreau commented Feb 13, 2023

@krassowski You've worked more than me on the completion recently, so I'll follow your decision here.

Comment on lines +2496 to +2502
if "prefix" in inspect.signature(key_completions_getter).parameters:
if prefix is not None:
# strip leading apostrophe from key before passing to ._ipython_key_completions
return key_completions_getter(prefix=prefix[1:])
else:
return key_completions_getter(prefix=None)

Copy link
Member

Choose a reason for hiding this comment

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

What I was suggesting was that instead of passing a prefix string we would pass an entire CompletionContext, something like:

class ItemCompletionContext(CompletionContext):
    """Completion context for item sub-matchers (providing information about `__getitem__`)"""
    key_prefix: str
Suggested change
if "prefix" in inspect.signature(key_completions_getter).parameters:
if prefix is not None:
# strip leading apostrophe from key before passing to ._ipython_key_completions
return key_completions_getter(prefix=prefix[1:])
else:
return key_completions_getter(prefix=None)
if (
hasattr(key_completions_getter, 'matcher_api_version')
and key_completions_getter.matcher_api_version == 2
):
item_context = ItemCompletionContext(
key_prefix=key_prefix,
full_text=context.full_text,
cursor_line=context.cursor_line,
cursor_position=context.cursor_position,
token=context.token,
)
return key_completions_getter(item_context)['completions']

Copy link
Member

Choose a reason for hiding this comment

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

As for stripping leading apostrophe: what if the key is a number? Or hex-number? Or raw string? I think we do not want to modify the prefix.

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.

Custom dict completion with _ipython_key_completions_ should allow access to prefix string
3 participants