before emitting finishedLoading, go thru the list of all
actions and change them as needed to show the tooltip.
Signed-off-by: Tomaz Canabrava <tcanabrava@kde.org>
Details
Details
- Reviewers
mwolff apol antonanikin kfunk - Group Reviewers
KDevelop
manual
Diff Detail
Diff Detail
- Repository
- R33 KDevPlatform
- Branch
- show_shortcuts
- Lint
No Linters Available - Unit
No Unit Test Coverage
Comment Actions
This works, thanks.
But I think you should add some comments to code about current restrictions - for example, tooltips are not updated after shortcut changing - we need to restart kdevelop for this.
And also maybe it better to use monospace font for shortcut displaying? Something like this:
foreach(QAction *action, actionCollection()->actions()) {
QString shortCut;
if (!action->shortcut().isEmpty())
shortCut = action->shortcut().toString();
else if (!actionCollection()->defaultShortcut(action).isEmpty())
shortCut = actionCollection()->defaultShortcut(action).toString();
if (!shortCut.isEmpty())
action->setToolTip(QString("<html>%1 (<b><tt>%2</tt></b>)</html>").arg(action->toolTip()).arg(shortCut));
}Comment Actions
Tomaz, please also add link to the related bug: https://bugs.kde.org/show_bug.cgi?id=364963.
| shell/mainwindow.cpp | ||
|---|---|---|
| 402 | 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. | |
| 407 | auto shortcut = action->shortcut();
if (shortcut.isEmpty()) {
shortcut = actionCollection()->defaultShortcut(action);
}
if (!shortcut.isEmpty()) {
action->setTooltip(...);
} | |
Comment Actions
- React on when a shortcut changes
- Rename curr->toolTip, and show text in italics.
- Save the original tooltip in a property / retrieve later.
Comment Actions
still not good to go
| shell/mainwindow.cpp | ||
|---|---|---|
| 324 | this should guard the loops below now, otherwise you do useless work here or could you simply move the updateActionTooltips up and then keep the old code as-is? | |
| 399 | const auto ORIGINAL_TOOLTIP = QByteArrayLiteral("__kdev_original_tooltip"); | |
| 401 | simplify and fix some bugs: QString tooltip = action->property(ORIGINAL_TOOLTIP).toString();
if (tooltip.isEmpty()) {
tooltip = action->toolTip();
}
auto shortcut = action->sohrtcut();
if (shortcut.isEmpty()) { // NOTE: your patch uses !isEmpty - that sounds wrong?
shortcut = actionCollection()->defaultShortcut(action);
}
// make sure to always set the tooltip, e.g. when the shortcut gets unset
if (!shortcut.isEmpty()) {
tooltip = i18nc("%1: original tooltip, %2: shortcut", "%1 <i>(%2)</i>", shortcut.toString()); // NOTE: use i18n to make this localizable
}
action->setToolTip(tooltip); | |
| 406 | this is the wrong way around, no? | |
| 410 | you have to reset the tooltip if the shortcut was unset | |
| 412 | this is not localization aware, use i18nc (imagine RTL languages) | |
Comment Actions
- React on when a shortcut changes
- Rename curr->toolTip, and show text in italics.
- Save the original tooltip in a property / retrieve later.
- Fix displaying empty shortcuts and code cleanup
Comment Actions
if possible, remove the foreach. otherwise lgtm
| shell/mainwindow.cpp | ||
|---|---|---|
| 401 | don't use foreach in new code please (use range-based for) | |
| shell/mainwindow.cpp | ||
|---|---|---|
| 399 | Maybe also add static ? | |
| shell/mainwindow.cpp | ||
|---|---|---|
| 399 | not needed for Q{ByteArray,String}Literal | |
Comment Actions
Please check out the alternative approach in https://phabricator.kde.org/D4697 -- this version works for both actions added from the main window + actions from plugins.