Skip to content

Conversation

@byrro
Copy link
Contributor

@byrro byrro commented Oct 24, 2020

Making an app exitable is commonly needed (almost all fullscreen apps will want it?). Being able to set this key-binding right when an Application object is being created is handy:

Quick examples: app = Application(exit_on_key="c-q") or app = Application(exit_on_key=("escape", "q")).

This would also simply some examples in the docs that are currently unexit'able (quite annoying to run, actually).

Let me know if this makes sense or not. Open to suggestions for changing anything also.

This feature does not change the default behavior of the Application object (no key-bindings). It's also overriden by the currently available key_bindings argument.

Wrote four tests which I think covers most common scenarios. Didn't see the need to write more, but let me know if I missed anything important.

byrro added 6 commits October 24, 2020 08:43
How it works:

    Make app exitable with custom key-binding by passing 'exit_on_key':
    app = Application(exit_on_key='c-q')

    Also accepts a tuple:
    app = Application(exit_on_key=('escape', 'q'))

    Obs1: 'exit_on_key' defaults to None (same behavior as currently)
    Obs2: a key set in 'key_bindings' argument always takes precedence.

Rationale:

    An app.exit() key-binding is arguably needed in many (if not most)
    applications. Being able to set as argument to Application is handy.

    Side benefit: simplify examples in documentation. Many aren't exit'able
    actually. Currently not the most welcoming for someone starting to learn.
@byrro
Copy link
Contributor Author

byrro commented Oct 24, 2020

I'm running isort locally on py36-38 against tests/test_application.py, it doesn't point any errors, but on TravisCI it keeps complaining. It shows a deprecation and update warning related to isort, perhaps it's an old version in the build server? At this point, I'm unsure what to do to get it passed...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant