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

[mwolff (Milian Wolff)]

mwolff added a comment.


  While configurability is always nice, do note that all shortcuts are standard shortcuts for their purpose, F2 e.g. to rename is ubiquitous - it works in dolphin and many other Qt apps with editable item views, but it also works under Window's Explorer e.g.

INLINE COMMENTS

> projectmanagerview.cpp:151
> +            QKeySequence shortcut = QKeySequence(keyEvent->modifiers() + keyEvent->key());
> +            if (m_plugin->removeAction() && shortcut == m_plugin->removeAction()->shortcut()) {
>                  m_plugin->removeItems(selectedItems());

can't you connect directly to the action's triggered slot instead of doing the custom comparisons here?

> projectmanagerviewplugin.cpp:86
>      ProjectManagerViewPluginPrivate()
> +        : m_remove(0), m_rename(0), m_copy(0), m_paste(0)
>      {}

one initializer per line:

  Foo()
      : m1(...)
      , m2(...)
      ...
  {}

> projectmanagerviewplugin.cpp:293
>      if ( needsRemoveAndRename ) {
> -        QAction* remove = new QAction( i18n( "Remove" ), this );
> -        remove->setIcon(QIcon::fromTheme(QStringLiteral("user-trash")));
> -        connect( remove, &QAction::triggered, this, &ProjectManagerViewPlugin::removeFromContextMenu );
> -        menuExt.addAction( ContextMenuExtension::FileGroup, remove );
> -        QAction* rename = new QAction( i18n( "Rename..." ), this );
> -        rename->setIcon(QIcon::fromTheme(QStringLiteral("edit-rename")));
> -        connect( rename, &QAction::triggered, this, &ProjectManagerViewPlugin::renameItemFromContextMenu );
> -        menuExt.addAction( ContextMenuExtension::FileGroup, rename );
> +        d->m_remove = new QAction( i18n( "Remove" ), this );
> +        d->m_remove->setIcon(QIcon::fromTheme(QStringLiteral("user-trash")));

you'll need to use KActionCollection which depends on KXMLGui to get configurable shortcuts.

> projectmanagerviewplugin.h:62
>  
> +    QAction *removeAction();
> +    QAction *renameAction();

all getters should be const, also move the * next to the typename please

REPOSITORY
  rKDEVPLATFORM KDevPlatform

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

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

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