Searchable, Filterable List with Selection Callbacks #653
Conversation
|
please rebase onto current master |
examples/filter_list.py
Outdated
| try: | ||
| from fuzzyfinder import fuzzyfinder | ||
| except ImportError: | ||
| raise ImportError( | ||
| "The required package 'fuzzyfinder' is not installed. Install it using:\n\n" | ||
| " pip install fuzzyfinder\n" | ||
| ) |
There was a problem hiding this comment.
I am not the biggest fan of this.
I get where you comming with thee examples not beeing a real dependency, I would still prefere a more proper version
examples/filter_list.py
Outdated
|
|
||
| def filter_func(text, collection): | ||
| g = fuzzyfinder(text, collection, accessor=lambda x: str(x)) | ||
| return list(g) |
There was a problem hiding this comment.
why does this require a cast to list?
examples/filter_list.py
Outdated
| def callback_listener(item): | ||
| with open('choice_logs.txt', 'a') as f: | ||
| f.write(str(item) + '\n') |
There was a problem hiding this comment.
I would prefer a demo to not write stuff to disk
How about updating a global var and printing that at the end?
There was a problem hiding this comment.
I changed it to a global var and simplified the example 👍
src/inquirer/questions.py
Outdated
| def apply_filter(self, filter_func): | ||
| self._filtered_choices = filter_func(self.unfiltered_choices_generator) | ||
|
|
||
| def remove_filter(self): | ||
| self._filtered_choices = None |
There was a problem hiding this comment.
these are only relevant to the new class and shouldn't be inherited by every type of question
src/inquirer/questions.py
Outdated
| choices = self._filtered_choices or self._choices | ||
| for choice in self._solve(choices): | ||
| yield (TaggedValue(*choice) if isinstance(choice, tuple) and len(choice) == 2 else choice) | ||
|
|
||
| @property | ||
| def unfiltered_choices_generator(self): |
There was a problem hiding this comment.
same here, override the property for the new class instead of adding a new, second property and adding overhead to all other classes.
There was a problem hiding this comment.
I isolated all filter_list features away from other classes.
| @@ -42,6 +41,7 @@ def render(self, question, answers=None): | |||
| def _event_loop(self, render): | |||
| try: | |||
| while True: | |||
| self.clear_eos() | |||
There was a problem hiding this comment.
not sure if this a unintended side effects, I will have to think about this
There was a problem hiding this comment.
I removed this change in the last commit. Clearing is done in _filter_list.py though now
|
Thanks for the insightful review! I'll get back to this PR as soon as possible. |
- clearing elements is done in filter_list instead of main loop - remove clear_eos() from the main loop
|
FIxed the issues and I isolated filter_list features away from everything else like you asked. I hope it's ok like this. I'm not sure about delegating clearing to _filter_list, but at least it won't affect anything else. |
- Removed choice callback - Removed override of `choice_generator` in `Question` class
Please tell me if docs are needed or any other changes. Since it behaves similarly to List, I think the example is enough. Edit:
|
This is related to #73. This patch adds support for filtering choices. It adds a new component,
FilterList, instead of modifying the existing List.FilterListenables searching/filtering dynamically.FilterListalso provides a callback that triggers when the user moves between choices.Note: I tried to minimize changes to the original library files to avoid increasing complexity. I need this for a personal project, it would be nice to just import this library.
Edit: Please tell me it's possible to integrate these changes, so I can continue writing docs/tests for it.