Clean up shortcuts without actions and load all shortcuts as late as possible #29388
+17
−306
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves: #29054
A couple of commits in the PR:
Renames one shortcut's action and deletes the rest of the shortcuts for which no corresponding actions exists. This removes all warnings about invalid actions except for the ones related to plugins.
The second group of warnings about invalid actions in the form
action://extensions/...
is dealt with in the second commit. Those actions are added whenExtensionsModule
'sonDelayedInit()
executes which currently happens after theShortcutsModule
has loaded all the shortcuts. As a result, the loading of the shortcuts does not find the actions for the extensions. The solution I see and that I've implemented here is to move theShortcutsModule
at the end of the modules list and the loading of the shortcuts in a newonDelayedInit()
method of theShortcutsModule
. Although the order of the modules should not matter, this is not really the case and has produced various issues, so moving the ShortcutsModule further down the list is a bit of a risky change and needs to be tested well. In my testing I looked at the uses ofshortcutsRegister()->shortcut
andshortcutsRegister()->shortcuts()
by any code that executes beforeShortcutsModule::onDelayedInit()
and here's what I found:UiActionsRegister::updateShortcuts
andUiActionsRegister::updateShortcutsAll()
TextInputModel::loadShortcuts()
LayoutPanelTreeModel::addInstrumentsKeyboardShortcut()
ShortcutsInstanceModel::doLoadShortcuts()
and then
ShortcutsModule::onDelayedInit()
executes. It callsShortcutsRegister::reload
which loads all shortcuts and at the end emitsm_shortcutsChanged
. Luckily, all classes above -UiActionsRegister
,TextInputModel
,LayoutPanelTreeModel
andShortcutsInstanceModel
- have subscribed to this signal and reload their shortcuts when they are notified about it. So theoretically all shortcuts should continue to work. I didn't find issues in my testing but again we need to test this well. Whoever reviews this PR (I suppose Igor) - let me know what you think.I spotted a small issue in
TextInputModel::loadShortcuts()
which is fixed in the second commit as well.