----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129299/#review100674 -----------------------------------------------------------
Note that this check won't happen in apps based on KParts::MainWindow (e.g. konqueror) since these apps don't use KXmlGuiWindow::createGUI(). This might be a reason to extract the new code into a protected method. src/kxmlguiwindow.cpp (line 312) <https://git.reviewboard.kde.org/r/129299/#comment67599> This should be moved out of the loop, so that this lookup only happens once. src/kxmlguiwindow.cpp (line 315) <https://git.reviewboard.kde.org/r/129299/#comment67600> We could optimize this a bit, to not change the list just to filter out the main shortcut (and then that requires fetching the list again). QList<QKeySequence> editCutActionShortcuts = editCutAction->shortcuts(); if (editCutActionShortcuts.indexOf(shortcut) > 0) // alternate shortcut { editCutActionShortcuts.removeAll(shortcut); editCutAction->setShortcuts(editCutActionShortcuts); } src/kxmlguiwindow.cpp (line 331) <https://git.reviewboard.kde.org/r/129299/#comment67601> shortcuts.insert(portableShortcutText, action) is faster (see "Effective C++") - David Faure On Oct. 31, 2016, 7:18 p.m., Albert Astals Cid wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129299/ > ----------------------------------------------------------- > > (Updated Oct. 31, 2016, 7:18 p.m.) > > > Review request for KDE Frameworks, David Faure and Elvis Angelaccio. > > > Repository: kxmlgui > > > Description > ------- > > Add a warning at the createGui stage about ambiguous shortcuts being found in > the same action collection. > > This is usually a developer issue, but the error message about ambiguity will > only show up when someone tries to use the shortcut, so it is relatively easy > to miss if you do not try all your actions via a shortcut. > > Also if the involved shortcut is one of the non primary shortcuts of > edit_cut, just give it away, since it's usually Shift+Delete being fought > over. > > > Diffs > ----- > > src/kxmlguiwindow.cpp 519fb26 > > Diff: https://git.reviewboard.kde.org/r/129299/diff/ > > > Testing > ------- > > gwenview now defaults to Shift+Delete being "Hard delete" and not "Cut", if > you remove the > if (action == editCutAction || existingShortcutAction == editCutAction) { > part, you get warning about the actions involved > > > Thanks, > > Albert Astals Cid > >
