-
Notifications
You must be signed in to change notification settings - Fork 97
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
Add select all and tab controls #170
Conversation
@@ -65,14 +65,16 @@ def get_options(self): | |||
yield choice, selector + " " + symbol, color | |||
|
|||
def process_input(self, pressed): | |||
# Remove after merge #83 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are referencing a #83 here probably from other project since it is completely unrelated to what we have here. For referencing other projects you should put the whole link (magmax/python-readchar#83) to avoid confusion of developers reading this code.
@@ -89,6 +91,11 @@ def process_input(self, pressed): | |||
elif pressed == key.RIGHT: | |||
if self.current not in self.selection: | |||
self.selection.append(self.current) | |||
elif pressed == key.CTRL_A: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You added new branches to the code without adding tests for them. So this change request is reducing our test coverage. Please update the tests.
@@ -65,14 +65,16 @@ def get_options(self): | |||
yield choice, selector + " " + symbol, color | |||
|
|||
def process_input(self, pressed): | |||
# Remove after merge #83 | |||
key.SHIFT_TAB = "\x1b\x5b\x5a" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of adding a code while a PR is not merged, I would focus in having the dependency updated. This is creating some double work for you, me and other people working on python-inquirer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last update in readchar 4 feb 2022. I dont now, how many time need for merge it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@magmax do you think we can have a new version with magmax/python-readchar#83 in forseable future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now magmax/python-readchar#83 is part of magmax/python-readchar#79 , but in case of a successful answer, I will reopen that MP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is already merged and in pre-release state
for x in self.question.choices: | ||
self.selection = list(range(len(self.question.choices))) | ||
elif pressed == key.CTRL_Q: | ||
self.selection = [] | ||
elif pressed == key.ENTER: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I see there are no mentions of the changes on documentation and change-logs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do i can found change-logs files?
552f71b
to
273131e
Compare
FYI: the changes requested in magmax/python-readchar#83 are now merged and will come avalabel in |
change comment in tests
|
part of this is handeld in #275 and shift support is not portable anyway. So I am closing this |
ctrl+a select all
ctrl+q deselect all
tab move down
tab+shift move uo
after merge #83 remove 68-69 strings