Add keybinding config file #112
Conversation
This will add a `config.yml` file along side the `client.yml` file which will allow the user to configure custom keybindings. Currently this commit only covers global keybindings and not per-screen keybindings but this could be added in the future. This commit also doesn't currently allow for overriding default navigation keys (hjkl etc.) but that could also be added later. Resolves: Rigellute#46
Fixes clippy and rustfmt errors for 2c7884e
There was a problem hiding this comment.
This looks really good @zacklukem.
I have only one concern - now that the user can control global key presses, the user could add a global key press that clashes with block specific events or add duplicate global events, which might lead to confusion for the user.
For example, as it stands the user would not be able to create a global event using enter, hjkl, arrow keys or backspace since this would break the block level behaviour.
I suppose this is a tradeoff for providing customisation, and to help with this we could add some more user facing validation in user_config.
Thoughts?
| repeat: 'r' | ||
| search: '/' | ||
| ``` | ||
|
|
src/user_config.rs
Outdated
| let config_string = fs::read_to_string(&paths.config_file_path)?; | ||
| let config_yml: UserConfigString = serde_yaml::from_str(&config_string)?; | ||
|
|
||
| macro_rules! to_keys { |
Reserves the keys, hjkl up down left right, backspace and return and prevents them from being used in user keybinding config.
|
I added code to check hjkl, return and arrow keys |
| } | ||
|
|
||
| fn check_reserved_keys(key: Key) -> Result<(), failure::Error> { | ||
| let reserved = [ |
There was a problem hiding this comment.
We can add more keys to check here, but if we want to add checks for keys that the user can re-bind, we would need to use a vec or maybe a hashmap in order to not prevent the user from doing something like this:
decrease_volume: '+'
increase_volume: '-'Because with current code, if we add + and - to the reserved keys, the User wouldn't be able to do something like this, even if it is valid.
I think it isn't too important to block keys that are configurable from being reused by the user at all, as those keys are documented in the README.
Thoughts?
|
This looks very nice @zacklukem. I would suggest to move all the keybindings into one section in the config file to get a nicer structure into the config (because there are probably more settings sections than only keybindings in the future). This needs to be added asap so there is no need to transform existing configuration files into the new configuration file structure. I submitted a pull request with these changes to your branch. |
Move keybindings into its own section in config file
|
Sorry for late response - I intend to review this again tomorrow (not able to look at the moment) |
|
@all-contributors please add @zacklukem for code |
|
I've put up a pull request to add @zacklukem! 🎉 |
This will add a
config.ymlfile along side theclient.ymlfile whichwill allow the user to configure custom keybindings. Currently this
commit only covers global keybindings and not per-screen keybindings but
this could be added in the future. This commit also doesn't currently
allow for overriding default navigation keys (hjkl etc.) but that could
also be added later.
Resolves: #46