Skip to content
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

Key mapping feature #39

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

Key mapping feature #39

wants to merge 21 commits into from

Conversation

telday
Copy link
Contributor

@telday telday commented Mar 7, 2020

In the name of moving toward a more event driven interface, and having greater customization I have added KeyMaps.

Why

The basic theory is that each mapped key corresponds either to a method/function, or to another mapped key which then corresponds to a method. The original intent was twofold, to make declaring new keybindings simpler/to cleanup the handle_key_press methods, as well as making it easier to have multiple binds for the same action. I wanted to include some vim-like movement in my TUI, Now this can be done simply with four lines of code, an if you were to make a KeyMap and store it in a config file it could be used in multiple different programs.

Changes

The biggest change is the introduction of the KeyMap class, which provides the main functionality for the new mapping. Every widget/popup has a key_map which it inherits from the base Widget/Popup class. The children can then simply add their bindings to the parent key_map and when the parent goes to handle a key_press it will automatically consider the children's bindings.

Considerations

I have not fully finished implementing this feature yet, I will need to write a few docstrings and upate tests/examples to reflect the new api. I also feel like the scope of the change would warrant a MINOR version increment to 0.2.0 rather than PATCH

Let me know what you think, I have a few ideas about using decorators to perform the same duty as the KeyMap.bind_key method currently does. This would increase the event driven nature and reduce some excess code.

I have made an updated verision of the todo-list example with the new key binding method and the vim controls here

@jwlodek
Copy link
Owner

jwlodek commented Mar 7, 2020

I like these changes in theory, but I wish they didn't break compatibility with previous versions. Is there any way we can keep the add_key_command function that just adds to the raw_key_map, and keep variables that reference the keys with the old naming scheme?

@telday
Copy link
Contributor Author

telday commented Mar 7, 2020

I added back in the add_key_command method, which simply acts as a wrapper for making a new key_bind.

I removed some of the features used in order to comply with python 3.5. However I cannot get it actually working on windows because of a windows-curses issue that only seems to occur in version 3.5. Weird...

@telday telday changed the title WIP: Key mapping feature Key mapping feature Mar 7, 2020
@telday
Copy link
Contributor Author

telday commented Mar 7, 2020

Everything looks good now, I ported over all of the examples, however did not change them to directly using the key_map in place of add_key_command. I tried to test everything to the best of my abilities, and it seems good on both Fedora 32 and Windows 10. Let me know if you find any issues or if there is something you think should change

@jwlodek
Copy link
Owner

jwlodek commented Mar 9, 2020

I'll try and get to this pull request this week, I've been very busy lately, so not much time for fun projects. I want to merge it, but I would like to test everything on my end, and check how much work it would be to convert an existing project like pyautogit to use this, I just haven't had time to.

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.

2 participants