[Differential] [Requested Changes To] D4123: Show shortcuts on the tooltips.

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a reviewer: mwolff.
mwolff added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> mainwindow.cpp:353
> +        //First, try to remove the old shortcut from the current string.
> +        QString tooltip = action->toolTip();
> +        if (tooltip.indexOf("<i>(") != -1) {

this is pretty brittle code. I'd rather we use a dynamic property to store the original tooltip string, i.e. before overwriting it, call

  action->setProperty("__kdev_original_tooltip", tooltip);

Then use that later on to append the shortcut string to.

> mainwindow.cpp:358
> +
> +        if (!action->shortcut().isEmpty()) {
> +            action->setToolTip(tooltip + " <i>(" + action->shortcut().toString() + ")</i>");

auto shortcut = action->shortcut();
  if (shortcut.isEmpty()) {
      shortcut = actionCollection()->defaultShortcut(action);
  }
  if (!shortcut.isEmpty()) {
      action->setTooltip(...);
  }

REPOSITORY
  R33 KDevPlatform

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

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

To: tcanabrava, apol, antonanikin, kfunk, #kdevelop, mwolff
Cc: mwolff, antonanikin, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170122/17c196a8/attachment-0001.html>