feat: add search functionality to select and checkbox prompt, based on #42 #374
Conversation
This commit is heavily inspired by [gbataille's](https://github.com/gbataille) [PR](tmbo#42)
Instead of a prefix filter, the search filter is now searched within all entries. This seems to be more common than a prefix search. Using the search functionality disabled the ability to select all options or invert the selection in the checkbox control. If the search filter is enabled, these two functionalities can now be used with the key modifier ctrl. Updated the displayed instructions to match the changes made.
|
@mario-dg Thanks for this PR. Please can you merge latest changes into this branch, then I will run the CI and review. |
|
@kiancross, done 😄 |
There was a problem hiding this comment.
This PR is fantastic - thank you. I have tested it on my machine, and it all appears to work as expected.
I have left a couple of comments below, just to check you are happy with all the design decisions, but I think it is almost ready to merge (once the version numbers are fixed).
Thanks again.
| @@ -179,8 +194,9 @@ def get_prompt_tokens() -> List[Tuple[str, str]]: | |||
| "class:instruction", | |||
| "(Use arrow keys to move, " | |||
| "<space> to select, " | |||
There was a problem hiding this comment.
Are we happy to say that space can't be used in the search?
There was a problem hiding this comment.
We have to exclude it, since space is used to select an entry from the list. Otherwise we would have to come up with a rather complex solution to allow (de-)selection of an entry when the search is enabled.
| if not self.search_filter: | ||
| return self.choices | ||
| filtered = [ | ||
| c for c in self.choices if self.search_filter.lower() in c.title.lower() |
There was a problem hiding this comment.
I'm ok with this search mechanism, but it is not a prefix search like #42. Does this matter? Are there any downsides to searching for any occurrence of the substring? I suppose in the future, we can make the filter customisable, so it doesn't matter too much...
There was a problem hiding this comment.
After using the prefix search for a few days it felt unnatural and not intuitive. I personally don't know any application that prefers a prefix search above a substring search.
A fuzzy search would probably be the best, but add unnecessary complexity and/or dependencies to this library.
This commit is heavily inspired by gbataille's PR
What is the problem that this PR addresses?
Continued on the work mentioned above to integrate a search functionality to the
selectandcheckboxprompt.Should fix #33 and #42.
How did you solve it?
Taken the code from the previous PR, modified a bit to fit the latest updates, corrected type checking and linting.
Checklist