[Differential] [Updated] D1523: Project Manager View plugin has hardcoded/fixed shortcuts

rjvbb (René J.V. Bertin) noreply at phabricator.kde.org
Wed May 25 14:11:10 UTC 2016


rjvbb marked 2 inline comments as done.
rjvbb added a comment.


  In https://phabricator.kde.org/D1523#30919, @mwolff wrote:
  
  > While configurability is always nice, do note that all shortcuts are standard shortcuts for their purpose, F2 e.g. to rename is ubiquitous
  
  
  Evidently ubiquitous doesn't include OS X ;)
  Regardless of that, I've been using that F2 mapping I mentioned since well before MS Windows 3 (supposing that's where the F2=Rename mapping originated). F1=Raise, F2=Lower, https://phabricator.kde.org/F5=Minimise etc. might come from DomainOS but could also be a personal choice. Too long ago.

INLINE COMMENTS

> mwolff wrote in projectmanagerview.cpp:151
> can't you connect directly to the action's triggered slot instead of doing the custom comparisons here?

I don't know, I don't even know why it isn't enough to use `QAction::setShortCut` to call (in this case) `ProjectManagerViewPlugin::removeFromContextMenu()`. It should, according to the QAction documentation, because that's what the `triggered` signal is connected to.
May I presume there's a reason that even the Copy and Paste actions already did a manual check (supposing KStandardAction does define shortcuts for them)?

The other thing is that `ProjectManagerViewPlugin::removeFromContextMenu()` calls `removeItems()` with `ProjectManagerViewPlugin::itemsFromIndexes()`, and I don't have the slightest idea how different or not that is from `ProjectManagerView::selectedItems()`.

> mwolff wrote in projectmanagerviewplugin.cpp:293
> you'll need to use KActionCollection which depends on KXMLGui to get configurable shortcuts.

adding the actions to `actionCollection()` indeed makes them show up in the shortcut settings, but I must still be doing something wrong because changes I make don't "stick".

> mwolff wrote in projectmanagerviewplugin.h:62
> all getters should be const, also move the * next to the typename please

"official" KDE guidelines have the "*" and "&" attached to the variable, no (like `QObject *parent` in the ctor)?

REPOSITORY
  rKDEVPLATFORM KDevPlatform

REVISION DETAIL
  https://phabricator.kde.org/D1523

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: rjvbb, KDevelop, mwolff
Cc: mwolff, kdevelop-devel, KDevelop, Pilzschaf, akshaydeo, surgenight, joshiakshay, arrowdodger


More information about the KDevelop-devel mailing list