-
Notifications
You must be signed in to change notification settings - Fork 92
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
feat: add custom_key_binding argument to add custom keyboard handlings #282
base: master
Are you sure you want to change the base?
Conversation
…-instruction-messages-for-the-multi-choice-questions feat: customize instruction messages for the multi choice questions
…port-to-questionary-confirm feat: Add customized instruction support to confirm
Ia 222 implement skip question
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.
Thanks for the PR.
I've done a quick pass of code changes and made some suggestions. I think you've disabled the option for reviewers to apply changes themselves, so can you look over these changes and then apply them? Once you've done that, I'll do a more comprehensive review.
FYI, I might have missed changing custom_key_binding
to custom_key_bindings
in some places - please fix if I have!
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
Co-authored-by: Kian Cross <[email protected]>
…222-implement-skip-question
Co-authored-by: Kian Cross <[email protected]>
…odadata/ia-questionary into IA-222-implement-skip-question
Thanks for your review @kiancross, I fixed your feedbacks :) |
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.
I wonder if a better interface might be
{"c": lambda: "custom"}
or even just
{"c": "custom"}
The other code would be modified to something like this
if custom_key_bindings is not None:
for key, func in custom_key_bindings.items():
bindings.add(key, eager=True)(lamba event: event.app.exit(result=func()))
# or, if `func` is just a string
bindings.add(key, eager=True)(lamba event: event.app.exit(result=func))
The reason for this is that I don't think we expose event.app
anywhere else, and in most (all?) places, we just return a value, which is handled by internal code, rather than requiring users to call event.app.exit
explicitly.
What do you think?
What is the problem that this PR addresses?
Users were not able to customize keyboard inputs. Thanks to this PR, we can now customize the keyboard input as shown below thanks to new
custom_key_binding
argument. For now,custom_key_binding
field has been added forselect
,text
,checkbox
, andconfirm
.We will add a custom key handling for the
s
button in this example. Whenever user presss
, we will return_skipped_
string.custom_key_binding
argument takes a dictionary where the key is either a string orprompt_toolkit.keys.Keys
object the value is a callable that accepts aprompt_toolkit.key_binding.KeyBinding
event.or the same example can be written as
Checklist