Review Request 129299: Warn on startup about ambiguous shortcuts (with an exception for Shift+Delete)

[David Faure]

-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20161107/db01407f/attachment-0001.html>