Changeset View
Changeset View
Standalone View
Standalone View
shell/mainwindow.cpp
Show First 20 Lines • Show All 270 Lines • ▼ Show 20 Line(s) | 263 | for(int a = 1; a < Core::self()->uiControllerInternal()->mainWindows().size(); ++a) { | |||
---|---|---|---|---|---|
271 | } | 271 | } | ||
272 | } | 272 | } | ||
273 | 273 | | |||
274 | } | 274 | } | ||
275 | 275 | | |||
276 | void MainWindow::shortcutsChanged() | 276 | void MainWindow::shortcutsChanged() | ||
277 | { | 277 | { | ||
278 | KTextEditor::View *activeClient = Core::self()->documentController()->activeTextDocumentView(); | 278 | KTextEditor::View *activeClient = Core::self()->documentController()->activeTextDocumentView(); | ||
279 | if(!activeClient) | | |||
mwolff: this should guard the loops below now, otherwise you do useless work here
or could you simply… | |||||
280 | return; | | |||
281 | | ||||
282 | foreach(IDocument * doc, Core::self()->documentController()->openDocuments()) { | 279 | foreach(IDocument * doc, Core::self()->documentController()->openDocuments()) { | ||
283 | KTextEditor::Document *textDocument = doc->textDocument(); | 280 | KTextEditor::Document *textDocument = doc->textDocument(); | ||
284 | if (textDocument) { | 281 | if (textDocument) { | ||
285 | foreach(KTextEditor::View *client, textDocument->views()) { | 282 | foreach(KTextEditor::View *client, textDocument->views()) { | ||
286 | if (client != activeClient) { | 283 | if (client != activeClient) { | ||
287 | client->reloadXML(); | 284 | client->reloadXML(); | ||
288 | } | 285 | } | ||
289 | } | 286 | } | ||
290 | } | 287 | } | ||
291 | } | 288 | } | ||
289 | updateActionTooltips(); | ||||
292 | } | 290 | } | ||
293 | 291 | | |||
294 | 292 | | |||
295 | void MainWindow::initialize() | 293 | void MainWindow::initialize() | ||
296 | { | 294 | { | ||
297 | KStandardAction::keyBindings(this, SLOT(configureShortcuts()), actionCollection()); | 295 | KStandardAction::keyBindings(this, SLOT(configureShortcuts()), actionCollection()); | ||
298 | setupGUI( KXmlGuiWindow::ToolBar | KXmlGuiWindow::Create | KXmlGuiWindow::Save ); | 296 | setupGUI( KXmlGuiWindow::ToolBar | KXmlGuiWindow::Create | KXmlGuiWindow::Save ); | ||
299 | 297 | | |||
Show All 38 Lines | |||||
338 | 336 | | |||
339 | void MainWindow::cleanup() | 337 | void MainWindow::cleanup() | ||
340 | { | 338 | { | ||
341 | } | 339 | } | ||
342 | 340 | | |||
343 | void MainWindow::setVisible( bool visible ) | 341 | void MainWindow::setVisible( bool visible ) | ||
344 | { | 342 | { | ||
345 | KXmlGuiWindow::setVisible( visible ); | 343 | KXmlGuiWindow::setVisible( visible ); | ||
344 | updateActionTooltips(); | ||||
346 | emit finishedLoading(); | 345 | emit finishedLoading(); | ||
347 | } | 346 | } | ||
348 | 347 | | |||
348 | void KDevelop::MainWindow::updateActionTooltips() | ||||
349 | { | ||||
350 | qCDebug(SHELL) << "Applying the shortcut on the accelerator tooltip"; | ||||
const auto ORIGINAL_TOOLTIP = QByteArrayLiteral("__kdev_original_tooltip"); mwolff: ```
const auto ORIGINAL_TOOLTIP = QByteArrayLiteral("__kdev_original_tooltip");
``` | |||||
antonanikin: Maybe also add `static` ? | |||||
mwolff: not needed for Q{ByteArray,String}Literal | |||||
351 | foreach(QAction *action, actionCollection()->actions()) { | ||||
352 | //First, try to remove the old shortcut from the current string. | ||||
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); mwolff: simplify and fix some bugs:
```
QString tooltip = action->property(ORIGINAL_TOOLTIP).toString… | |||||
mwolff: don't use foreach in new code please (use range-based for) | |||||
353 | QString tooltip = action->toolTip(); | ||||
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. mwolff: this is pretty brittle code. I'd rather we use a dynamic property to store the original tooltip… | |||||
354 | if (tooltip.indexOf("<i>(") != -1) { | ||||
kfunk: `curr` -> `toolTip` | |||||
355 | tooltip = tooltip.split("<i>(").at(0).trimmed(); | ||||
356 | } | ||||
357 | | ||||
mwolff: this is the wrong way around, no? | |||||
358 | if (!action->shortcut().isEmpty()) { | ||||
auto shortcut = action->shortcut(); if (shortcut.isEmpty()) { shortcut = actionCollection()->defaultShortcut(action); } if (!shortcut.isEmpty()) { action->setTooltip(...); } mwolff: auto shortcut = action->shortcut();
if (shortcut.isEmpty()) {
shortcut =… | |||||
359 | action->setToolTip(tooltip + " <i>(" + action->shortcut().toString() + ")</i>"); | ||||
360 | } else if (!actionCollection()->defaultShortcut(action).isEmpty()) { | ||||
361 | action->setToolTip(tooltip + " <i>(" + actionCollection()->defaultShortcut(action).toString() + ")</i>"); | ||||
mwolff: you have to reset the tooltip if the shortcut was unset | |||||
362 | } | ||||
+1 on Anton's idea, maybe a different font format for the shortcut string would make sense. /me proposes italics kfunk: +1 on Anton's idea, maybe a different font format for the shortcut string would make sense. | |||||
363 | } | ||||
mwolff: this is not localization aware, use i18nc (imagine RTL languages) | |||||
364 | } | ||||
365 | | ||||
349 | bool MainWindow::queryClose() | 366 | bool MainWindow::queryClose() | ||
350 | { | 367 | { | ||
351 | if (!Core::self()->documentControllerInternal()->saveAllDocumentsForWindow(this, IDocument::Default)) | 368 | if (!Core::self()->documentControllerInternal()->saveAllDocumentsForWindow(this, IDocument::Default)) | ||
352 | return false; | 369 | return false; | ||
353 | 370 | | |||
354 | return Sublime::MainWindow::queryClose(); | 371 | return Sublime::MainWindow::queryClose(); | ||
355 | } | 372 | } | ||
356 | 373 | | |||
▲ Show 20 Lines • Show All 117 Lines • Show Last 20 Lines |
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?