Skip to content

Added support for a json file for keymapping - #653

Open
MoeFwacky wants to merge 4 commits into
shane-mason:mainfrom
MoeFwacky:main
Open

Added support for a json file for keymapping#653
MoeFwacky wants to merge 4 commits into
shane-mason:mainfrom
MoeFwacky:main

Conversation

@MoeFwacky

Copy link
Copy Markdown
Contributor

Changed keymap behavior to look first for keymap.json in the same directory, if not found falls back to defaults. keymap.json file uses the same format as KEY_MAPPINGS

Changed keymap behavior to look first for keymap.json in the same directory, if not found falls back to defaults.
keymap.json file uses the same format as `KEY_MAPPINGS`
@freelancer42

Copy link
Copy Markdown

The existing script reads custom exec mappings from "runtime/remote_callback_map.json", while the new keymap file in this commit would be read from os.path.join(os.getcwd(),'keymap.json'). Wouldn't it make more sense to read both config files from the same place?

Modified the keymap file lookup location to be in-line with similar lookups in the runtime directory
The script now looks for DEBOUNCE_TIME, USE_SYSTEMCTL and SYSTEMCTL_TO_TOGGLE in the json file, as well as the keymap, allowing these to be set outside of the script. Anything not found in the external file falls back to the defaults set in the script.
@MoeFwacky

Copy link
Copy Markdown
Contributor Author

I've moved the file lookup to be under runtime to match the remote_callback_map.json file, and it's been expanded to include other user-configurable variables.

@shane-mason

Copy link
Copy Markdown
Owner

Hey - this is great. There are a couple of issues that need looking at before I accept it:

  • If the json is malformed, its going to crash. It needs a try/except around it. Otherwise, the service will go into a doom loop.
  • keymapping gets overwritten, not merged together. That means they have to put everything in your file or they lose the builtin keymappings. I might be better if the two were merged, so users only had to put the keys they wanted to override?

The second part could be a follow on, but the first should be corrected.

Added error handling to code that loads settings from file
Added dictionary merge to keep keymap defaults if not present in the file
@MoeFwacky

Copy link
Copy Markdown
Contributor Author

The latest commit should correct for both of those issues, unless I missed something.

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.

3 participants