feat: initial exploration for keybinding source addition and inverse map for keybinding registry

[dalthviz]

Closes #207

Based on napari/napari#6204

Currently this:

• Adds a source field and three prioritized sources (USER, PLUGIN, SYSTEM)
• Adds a new method that returns the highest priority active keybinding given the evaluated context (get_context_prioritized_keybinding)
• Adds an inverse map keyable by the key sequence sorting the keymap values using the source and weight (_keymap)
• Changes the _keybindings attribute to be computed from the new _keymap attribute
• Adds tests for the new _RegisteredKeyBinding comparison logic and the method to retrieve a prioritaized keybinding (get_context_prioritized_keybinding)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (56d0d4f) to head (772903d).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #226   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           31        31           
  Lines         1883      1916   +33     
=========================================
+ Hits          1883      1916   +33     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[DragaDoncila]

I had a look at this and I like the implementation, and sorting first by source, then by weight. The only question that came to mind is whether we think the sources should also be app-configured or whether we think USER > PLUGIN > SYSTEM is generic enough. I'm also thinking that SYSTEM might be ambiguous as OS-defined codes, and maybe we want to change that to APP?

@dalthviz it would be good to update the description to add specifically what is changing namely:

• adding source field and three prioritized sources
• adding new method that returns the highest priority active keybinding given the evaluated context
• adding an inverse map keyable by the key sequence
• sorting the keymap values using the source and weight

@tlambert03 what do you think about the changes here? Also cc. @lucyleeow

[tlambert03]

looks good! made a number of suggestions for cleanup of logic and syntax

[tlambert03]

would be a good candidate for IntEnum, to gain support for rich comparisons

[dalthviz]

The only question that came to mind is whether we think the sources should also be app-configured or whether we think USER > PLUGIN > SYSTEM is generic enough. I'm also thinking that SYSTEM might be ambiguous as OS-defined codes, and maybe we want to change that to APP?

Over napari/napari#6204 I think the value is named CORE. Other name could be BASE. Not totally sure what could be the best option to be honest 😅 but, if having a fixed set of values is the way to go, while working on this I would say I have seen the following options:

• USER > PLUGIN > CORE
• USER > EXT > CORE
• USER > PLUGIN > SYSTEM
• USER > PLUGIN > APP
• USER > PLUGIN > BASE

Let me know what could be the prefered combination or if providing an option to use a custom set of values/enum makes sense!

[tlambert03]

Let me know what could be the prefered combination or if providing an option to use a custom set of values/enum makes sense!

I'm happy to defer to the group opinion/decision here. I like APP or CORE or BASE over system. And don't have strong opinion on EXT v PLUGIN.

thanks for all this @dalthviz

[dalthviz]

I'm happy to defer to the group opinion/decision here. I like APP or CORE or BASE over system.

For the moment then I will follow the naming suggestion done by Draga (last commit changes SYSTEM for APP)

Thank you for the reviews @tlambert03 @DragaDoncila and let me know if something else is needed here!

[lucyleeow]

Late to the party but this is great @dalthviz. Great choices, will be nice to use for keybinding implementation in napari.