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

Added a search feature for List() #524

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

Conversation

fullfox
Copy link

@fullfox fullfox commented Feb 2, 2024

Hello,

This adds two features to the List():

  • When specifying a matcher function, the user can now select an entry by typing it, without keyboard arrows. The matcher function takes a "choice" and the "user query", and returns a boolean. On every keystroke (if the associated key is a letter), the first "choice" for which the matcher function returned True will become the current selected one. If nothing matches, the current item remains the same.
  • When specifying search=True (False by default), if a matcher function is defined, then the user search will be visible (using the already existing and unused get_current_value method)
matcher = lambda entry, search: entry.lower().startswith(search.lower())

questions = [
    inquirer.List(
        "size", message="What size do you need?", choices=["Jumbo", "Large", "Standard"], search=True, matcher=matcher
    ),
]
answers = inquirer.prompt(questions)

image

If the choices array contains tuples, it is the responsability of the user to provide a matcher function that handles tuple instead on string.

I've provided unit test and example in the commit. Hope this is clean enough. Please let me know if anything is unclear or if my implementation can be improved. If no matcher function is provided, the code behave exactly as before my commit.

@Cube707 Cube707 self-assigned this Feb 3, 2024
@Cube707 Cube707 added the python Pull requests that update Python code label Feb 3, 2024
@Cube707
Copy link
Collaborator

Cube707 commented Feb 3, 2024

Thank you very much for this. This feature has been on the ToDo list for quite some time.

However in the current example there is not way to correct or go back on the typed stuff. I think it would make sens if BACKSAPCE would work or something similar, in case someone makes a type.

Also documentation would be nice, even if its not beeing deployed at the moment

@Cube707
Copy link
Collaborator

Cube707 commented Feb 3, 2024

also some pre-commit stuff fails

@fullfox
Copy link
Author

fullfox commented Feb 4, 2024

Hello,

Actually I've implemented the backspace deletion. Did you test and didn't it work ?

I'll check why nox fails tomorrow.

@Cube707
Copy link
Collaborator

Cube707 commented Feb 4, 2024

Yes I tested the provided example on windows and it didn't seem to work.

The its the flake8 pre-commit hook that fails. It just wants some formating corrections

@fullfox
Copy link
Author

fullfox commented Feb 5, 2024

Okay I fixed the backspace handling for windows, and i made it flake8 compliant.

@fullfox
Copy link
Author

fullfox commented Feb 19, 2024

Hello, could you check my pr ?

@Cube707
Copy link
Collaborator

Cube707 commented Feb 19, 2024

Sorry for the delay but I am on holiday until 24. Of February.

But its on the todo list for when I am back

@Cube707 Cube707 mentioned this pull request Feb 20, 2024
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.

After thinking about this for a bit, I feel likie this adds a lot of complexity where it makes the howle thing quite unflexible. I think it should be up to the user/developer to implement the parsing logic that handles combining multile keypresses. That would give much more flexibility for different scenarios and make it much simpler and cleaner inside the libary. (for example, right now navigation via arrow keys and use of DEL, not BACKSPACE, is not implemendet. But is also probabyl overcomplication)

I would suggest we implement the matcher with the following callback signature. passing in the full list of choices and the current key. The user can implment what he wants and returnes the index the list should jump to (current).

def matcher(choices: list, pressedkey: str) -> int

What do you think?

src/inquirer/questions.py Show resolved Hide resolved
src/inquirer/render/console/_list.py Outdated Show resolved Hide resolved
@Cube707
Copy link
Collaborator

Cube707 commented Feb 24, 2024

also sidenote for the future: Rebasing onto the new state of main doesn't produce a bunch of merge-commits

@fullfox
Copy link
Author

fullfox commented Feb 24, 2024

Sure that would cover more use cases. I'll try to implement that this week.

@fullfox
Copy link
Author

fullfox commented Mar 6, 2024

Okay i changed the matcher signature to matcher(choices: list, pressedKey: str, searchString: str) -> (int, str)
where searchString acts like a memory to handle scenario where we want to keep track of previous keys (a search for example). And the function returns a tuple (index, searchString) with the choosed index and the new searchString.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants