Discussion (moved from Discord)

GSoC Projects GSoC - Shortcut Project
1

Anjula’s post in Discord:

I’m working on detecting conflicting shortcuts at the moment. What I’m thinking is maybe check for duplicate shortcuts after loading the entire keymap file, rather than checking while reading each shortcut. Would that be okay?

If we check while loading each shortcut, IMO, there’s an disadvantage. Nothing significant, but still…

Let’s assume X, Y are two commands. We change X’s shortcut to Y’s default shortcut, and Y’s shortcut to X’s default shortcut.
If we check conflicts after doing all these changes, then there wouldn’t be an issue.
If we are parsing X first, then it would cause an issue since it’s used in Y too, and I think using some advanced algorithm for this is over-engineering, since editing the keymap file is a feature for advanced users.

In the shortcut editor however, we can just warn the user.

I’m thinking of getting these tasks done in Phase 1:

  1. Complete implementation of the Keymap handler module (PR: https://github.com/laurent22/joplin/pull/3252) and get it merged
  2. Write more unit tests and feature tests for Keymap handler
  3. Write a reference guide for custom keymaps
  4. Finalize designs (UML diagrams might be very useful) for Keymap config screen, and start implementation

Please give your feedback on this @tessus and @bedwardly-down. IMO, I think #1 should be merged first. I’m also designing keymap config screen at the moment, and it heavily depends on the Keymap handler module.
I’ll post my designs very soon, so I can get your feedback and make changes before implementation.

1 Like
2

I am ok with reading the entire keymap file. Although the final decision lies with Laurent. IMO it makes perfect sense, otherwise we’d run into the problem you mentioned.

There are a few things to consider. How do we handle the following:

  • invalid shortcut (e.g. accelerator: 'IdontExist+F' )
  • invalid action (e.g.: command: 'makeCoffee' )

The last part is only necessary for reading the json keymap file (not for the UI shortcut editor).

1 Like
3

@anjulalk did you see this topic and my answer (I also posted a link in Discord) ?

1 Like
4

Yes, it was helpful. Thanks!
I went with that method. It’s checking for duplicates after reading the entire keymap. If there are duplicates it uses the default keymap and log an error.
If there are errors in individual items, like the once you’ve mentioned (invalid accelerator/command) they’re logged as warnings while reading one by one and skipped.