Skip to content

Conversation

@WilliamVenner
Copy link
Contributor

@WilliamVenner WilliamVenner commented Oct 10, 2022

(Branches off of #256)

This adds the ability to add an autocompletion function to Text prompts. The autocompletion function has similar semantics as readline.set_completer. Pressing TAB will call the autocompletion function and set the current text to whatever it returns.

@Cube707
Copy link
Collaborator

Cube707 commented Nov 14, 2022

@WilliamVenner please remove the merge. Instead, rebase your feature branch onto the current master and force push it (after reafirming it still works)

@WilliamVenner
Copy link
Contributor Author

Hey, do you need me to fix that code coverage thing? It seems a bit overzealous

@staticdev
Copy link
Collaborator

@WilliamVenner first of, autocompletion is a great feature and thanks for that.

I understand that is seems a overkill to have all this coverage, but also we are trying to maintain a well tested software so it is good for everyone. If you could add some unit tests for it would be great. If not, we see how we can deal with it and not loose also your effort.

@WilliamVenner
Copy link
Contributor Author

Hmmm I'm having trouble of thinking how to come up with a test for this code, which I think is the culprit of the code coverage check fail.

if pressed == key.TAB and self.question.autocomplete:
if self._autocomplete_state is None:
self._autocomplete_state = [self.current, 0]
[text, state] = self._autocomplete_state
autocomplete = self.question.autocomplete(text, state)
if isinstance(autocomplete, str):
self.current = autocomplete
self._autocomplete_state[1] += 1
else:
self._autocomplete_state = None
return
self._autocomplete_state = None

I would be inclined to say that the test added by this PR and the already present tests should cover this code, but that's understandably not being picked up by the code coverage tool - any thoughts?

@staticdev
Copy link
Collaborator

staticdev commented Nov 28, 2022

@WilliamVenner we have tests to render text here: https://github.com/magmax/python-inquirer/blob/main/tests/integration/console_render/test_text.py

You could add a case for autocomplete when you have tab key pressed.

@WilliamVenner
Copy link
Contributor Author

Thanks for your help @staticdev :)

Copy link
Collaborator

@staticdev staticdev left a comment

Choose a reason for hiding this comment

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

Lgtm. @Cube707 ?

Copy link
Collaborator

@Cube707 Cube707 left a comment

Choose a reason for hiding this comment

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

I am missing some documentation. Code looks fine as is, but I tried fiddeling arround in the example and couldn't easiely get some basict to work:

  • What is expected to be passed to autocomplete?
  • what do functons need to return when passed to autocomplete?

I tried the obvios thing and passed a list of strings, which just failed silently..

  • an example of using multiple suggestions would be helpful. Maybe something similar to this:
from itertools import cycle

suggestions = cycle(["inquirer", "hello"])
def autocomplete_fn(_text, _state):
    return next(suggestions)

@WilliamVenner WilliamVenner requested review from Cube707 and removed request for magmax November 30, 2022 14:00
Copy link
Collaborator

@Cube707 Cube707 left a comment

Choose a reason for hiding this comment

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

LGTM

@staticdev
Copy link
Collaborator

Thanks a lot @WilliamVenner !

@staticdev staticdev merged commit 75c1ad4 into magmax:main Nov 30, 2022
@WilliamVenner
Copy link
Contributor Author

hey guys, when do you plan on releasing this one? :p

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants