Differential D10990 Diff 29653 src/dolphinmainwindow.cpp

Changeset View

Standalone View

View Options

src/dolphinmainwindow.cpp

Show First 20 Lines • Show All 351 Lines • ▼ Show 20 Line(s)350 } else if (list.count() == 1) {
352 newWindowUrl = DolphinView::openItemAsFolderUrl(item); 352​ newWindowUrl = DolphinView::openItemAsFolderUrl(item);
353 } 353​ }
354 354​
355 if (!newWindowUrl.isEmpty()) { 355​ if (!newWindowUrl.isEmpty()) {
356 Dolphin::openNewWindow({newWindowUrl}, this); 356​ Dolphin::openNewWindow({newWindowUrl}, this);
357 } 357​ }
358} 358​}
359 359​
360​void DolphinMainWindow::showTarget()
361​{
362​ const auto link = m_activeViewContainer->view()->selectedItems().at(0);
363​ const auto linkLocationDir = QFileInfo(link.localPath()).absoluteDir();
364​ auto linkDestination = link.linkDest();
365​ if (QFileInfo(linkDestination).isRelative()) {
366​ linkDestination = linkLocationDir.filePath(linkDestination);
367​ }
368​ if (QFileInfo::exists(linkDestination)) {
369​ KIO::highlightInFileManager({QUrl::fromLocalFile(linkDestination).adjusted(QUrl::StripTrailingSlash)});
370​ } else {
371​ m_activeViewContainer->showMessage(
372​ i18n("Could not access \"%1\".").arg(linkDestination), DolphinViewContainer::Warning);
elvisangelaccioUnsubmitted
Not Done

One last thing: this should be

i18nc("@info", "Could not access <filename>%1</filename>.", linkDestination);

See https://api.kde.org/frameworks/ki18n/html/prg_guide.html#kuit_markup

elvisangelaccio: One last thing: this should be ``` i18nc("@info", "Could not access <filename>%1</filename>."…
373​ }
374​}
375​
360void DolphinMainWindow::showEvent(QShowEvent* event) 376​void DolphinMainWindow::showEvent(QShowEvent* event)
361{ 377​{
362 KXmlGuiWindow::showEvent(event); 378​ KXmlGuiWindow::showEvent(event);
363 379​
364 if (!event->spontaneous()) { 380​ if (!event->spontaneous()) {
365 m_activeViewContainer->view()->setFocus(); 381​ m_activeViewContainer->view()->setFocus();
366 } 382​ }
367} 383​}
▲ Show 20 Lines • Show All 842 Lines • ▼ Show 20 Line(s)1204#endif
1210 QAction* activatePrevTab = actionCollection()->addAction(QStringLiteral("activate_prev_tab")); 1226​ QAction* activatePrevTab = actionCollection()->addAction(QStringLiteral("activate_prev_tab"));
1211 activatePrevTab->setIconText(i18nc("@action:inmenu", "Previous Tab")); 1227​ activatePrevTab->setIconText(i18nc("@action:inmenu", "Previous Tab"));
1212 activatePrevTab->setText(i18nc("@action:inmenu", "Activate Previous Tab")); 1228​ activatePrevTab->setText(i18nc("@action:inmenu", "Activate Previous Tab"));
1213 activatePrevTab->setEnabled(false); 1229​ activatePrevTab->setEnabled(false);
1214 connect(activatePrevTab, &QAction::triggered, m_tabWidget, &DolphinTabWidget::activatePrevTab); 1230​ connect(activatePrevTab, &QAction::triggered, m_tabWidget, &DolphinTabWidget::activatePrevTab);
1215 actionCollection()->setDefaultShortcuts(activatePrevTab, prevTabKeys); 1231​ actionCollection()->setDefaultShortcuts(activatePrevTab, prevTabKeys);
1216 1232​
1217 // for context menu 1233​ // for context menu
1218 QAction* showOriginal = actionCollection()->addAction(QStringLiteral("show_original")); 1234​ QAction* showTarget = actionCollection()->addAction(QStringLiteral("show_target"));
1219 showOriginal->setText(i18nc("@action:inmenu", "Show Original")); 1235​ showTarget->setText(i18nc("@action:inmenu", "Show Target"));
elvisangelaccioUnsubmitted
Done

Why "Show Original"? What does original mean?

I'd have expected either "Show Destination" or "Show Target".

elvisangelaccio: Why "Show Original"? What does original mean? I'd have expected either "Show Destination" or…
rominfAuthorUnsubmitted
Done

I'm for "Show Target" ("symlink target" is 5x times more popular than "symlink destination" in Google).
BTW, the author of "Show Original" was @ngraham.

rominf: I'm for "Show Target" ("symlink target" is 5x times more popular than "symlink destination" in…
1220 showOriginal->setIcon(QIcon::fromTheme(QStringLiteral("document-open-folder"))); 1236​ showTarget->setIcon(QIcon::fromTheme(QStringLiteral("document-open-folder")));
rkflxUnsubmitted
Done

In Gwenview and Spectacle we recently used document-open-folder for the same feature. That icon basically looks the same, but does not "colourize" on HiDPI (and still has non-symbolic line-width).

(Hmh, we should really make this a standard action…)

rkflx: In Gwenview and Spectacle we recently used `document-open-folder` for the same feature. That…
1221 showOriginal->setEnabled(false); 1237​ showTarget->setEnabled(false);
1222 connect(showOriginal, &QAction::triggered, [this]() { 1238​ connect(showTarget, &QAction::triggered, this, &DolphinMainWindow::showTarget);
elvisangelaccioUnsubmitted
Done

Missing this as receiver parameter.

elvisangelaccio: Missing `this` as receiver parameter.
1223 const auto link = m_activeViewContainer->view()->selectedItems().at(0); ​
1224 const auto linkLocationDir = QFileInfo(link.localPath()).absoluteDir(); ​
1225 auto linkDestination = link.linkDest(); ​
1226 if (QFileInfo(linkDestination).isRelative()) ​
1227 linkDestination = linkLocationDir.filePath(linkDestination); ​
1228 if (QFileInfo(linkDestination).exists()) { ​
1229 KIO::highlightInFileManager({QUrl::fromLocalFile(linkDestination).adjusted(QUrl::StripTrailingSlash)}); ​
1230 } else { ​
1231 m_activeViewContainer->showMessage( ​
1232 i18n("Could not access \"%1\".").arg(linkDestination), DolphinViewContainer::Warning); ​
1233 } ​
1234 }); ​
1235 1239​
1236 QAction* openInNewTab = actionCollection()->addAction(QStringLiteral("open_in_new_tab")); 1240​ QAction* openInNewTab = actionCollection()->addAction(QStringLiteral("open_in_new_tab"));
1237 openInNewTab->setText(i18nc("@action:inmenu", "Open in New Tab")); 1241​ openInNewTab->setText(i18nc("@action:inmenu", "Open in New Tab"));
1238 openInNewTab->setIcon(QIcon::fromTheme(QStringLiteral("tab-new"))); 1242​ openInNewTab->setIcon(QIcon::fromTheme(QStringLiteral("tab-new")));
1239 connect(openInNewTab, &QAction::triggered, this, &DolphinMainWindow::openInNewTab); 1243​ connect(openInNewTab, &QAction::triggered, this, &DolphinMainWindow::openInNewTab);
elvisangelaccioUnsubmitted
Done

Missing braces

elvisangelaccio: Missing braces
1240 1244​
elvisangelaccioUnsubmitted
Done

Use the static QFileInfo::exists() which is faster.

elvisangelaccio: Use the static `QFileInfo::exists()` which is faster.
1241 QAction* openInNewTabs = actionCollection()->addAction(QStringLiteral("open_in_new_tabs")); 1245​ QAction* openInNewTabs = actionCollection()->addAction(QStringLiteral("open_in_new_tabs"));
1242 openInNewTabs->setText(i18nc("@action:inmenu", "Open in New Tabs")); 1246​ openInNewTabs->setText(i18nc("@action:inmenu", "Open in New Tabs"));
1243 openInNewTabs->setIcon(QIcon::fromTheme(QStringLiteral("tab-new"))); 1247​ openInNewTabs->setIcon(QIcon::fromTheme(QStringLiteral("tab-new")));
1244 connect(openInNewTabs, &QAction::triggered, this, &DolphinMainWindow::openInNewTab); 1248​ connect(openInNewTabs, &QAction::triggered, this, &DolphinMainWindow::openInNewTab);
1245 1249​
1246 QAction* openInNewWindow = actionCollection()->addAction(QStringLiteral("open_in_new_window")); 1250​ QAction* openInNewWindow = actionCollection()->addAction(QStringLiteral("open_in_new_window"));
elvisangelaccioUnsubmitted
Done

This should go in a dedicated function. Lambdas should not be bigger than 2/3 lines of code.

elvisangelaccio: This should go in a dedicated function. Lambdas should not be bigger than 2/3 lines of code.
1247 openInNewWindow->setText(i18nc("@action:inmenu", "Open in New Window")); 1251​ openInNewWindow->setText(i18nc("@action:inmenu", "Open in New Window"));
1248 openInNewWindow->setIcon(QIcon::fromTheme(QStringLiteral("window-new"))); 1252​ openInNewWindow->setIcon(QIcon::fromTheme(QStringLiteral("window-new")));
1249 connect(openInNewWindow, &QAction::triggered, this, &DolphinMainWindow::openInNewWindow); 1253​ connect(openInNewWindow, &QAction::triggered, this, &DolphinMainWindow::openInNewWindow);
1250} 1254​}
1251 1255​
1252void DolphinMainWindow::setupDockWidgets() 1256​void DolphinMainWindow::setupDockWidgets()
1253{ 1257​{
1254 const bool lock = GeneralSettings::lockPanels(); 1258​ const bool lock = GeneralSettings::lockPanels();
▲ Show 20 Lines • Show All 136 Lines • ▼ Show 20 Line(s)1394 if (list.isEmpty()) {
1391 stateChanged(QStringLiteral("has_no_selection")); 1395​ stateChanged(QStringLiteral("has_no_selection"));
1392 } else { 1396​ } else {
1393 stateChanged(QStringLiteral("has_selection")); 1397​ stateChanged(QStringLiteral("has_selection"));
1394 1398​
1395 KActionCollection* col = actionCollection(); 1399​ KActionCollection* col = actionCollection();
1396 QAction* renameAction = col->action(KStandardAction::name(KStandardAction::RenameFile)); 1400​ QAction* renameAction = col->action(KStandardAction::name(KStandardAction::RenameFile));
1397 QAction* moveToTrashAction = col->action(KStandardAction::name(KStandardAction::MoveToTrash)); 1401​ QAction* moveToTrashAction = col->action(KStandardAction::name(KStandardAction::MoveToTrash));
1398 QAction* deleteAction = col->action(KStandardAction::name(KStandardAction::DeleteFile)); 1402​ QAction* deleteAction = col->action(KStandardAction::name(KStandardAction::DeleteFile));
1399 QAction* cutAction = col->action(KStandardAction::name(KStandardAction::Cut)); 1403​ QAction* cutAction = col->action(KStandardAction::name(KStandardAction::Cut));
1400 QAction* deleteWithTrashShortcut = col->action(QStringLiteral("delete_shortcut")); // see DolphinViewActionHandler 1404​ QAction* deleteWithTrashShortcut = col->action(QStringLiteral("delete_shortcut")); // see DolphinViewActionHandler
1401 QAction* showOriginal = col->action(QStringLiteral("show_original")); 1405​ QAction* showTarget = col->action(QStringLiteral("show_target"));
elvisangelaccioUnsubmitted
Done

Unrelated code style change. Please revert.

elvisangelaccio: Unrelated code style change. Please revert.
rominfAuthorUnsubmitted
Done

Note = after showOriginal. It's aligned to the = after deleteWithTrashShortcut. Other actions should be formatted the same way. It's previous commiter fault that he didn't format the things well.
What is the purpose to revert formatting if it's already in the repository?
OK, now I understand that I don't have to commit formatting changes alongside the main code. Should I do separate patch like "And now format the code I pushed before"?

rominf: Note `=` after `showOriginal`. It's aligned to the `=` after `deleteWithTrashShortcut`. Other…
elvisangelaccioUnsubmitted
Done

Fine, let's forget about this. But next time remember that commits should be atomic (= no unrelated stuff).

elvisangelaccio: Fine, let's forget about this. But next time remember that commits should be atomic (= no…
1402 1406​
1403 KFileItemListProperties capabilities(list); 1407​ KFileItemListProperties capabilities(list);
1404 const bool enableMoveToTrash = capabilities.isLocal() && capabilities.supportsMoving(); 1408​ const bool enableMoveToTrash = capabilities.isLocal() && capabilities.supportsMoving();
1405 1409​
1406 renameAction->setEnabled(capabilities.supportsMoving()); 1410​ renameAction->setEnabled(capabilities.supportsMoving());
1407 moveToTrashAction->setEnabled(enableMoveToTrash); 1411​ moveToTrashAction->setEnabled(enableMoveToTrash);
1408 deleteAction->setEnabled(capabilities.supportsDeleting()); 1412​ deleteAction->setEnabled(capabilities.supportsDeleting());
1409 deleteWithTrashShortcut->setEnabled(capabilities.supportsDeleting() && !enableMoveToTrash); 1413​ deleteWithTrashShortcut->setEnabled(capabilities.supportsDeleting() && !enableMoveToTrash);
1410 cutAction->setEnabled(capabilities.supportsMoving()); 1414​ cutAction->setEnabled(capabilities.supportsMoving());
1411 showOriginal->setEnabled(list.length() == 1 && list.at(0).isLink()); 1415​ showTarget->setEnabled(list.length() == 1 && list.at(0).isLink());
1412 } 1416​ }
1413} 1417​}
1414 1418​
1415void DolphinMainWindow::updateViewActions() 1419​void DolphinMainWindow::updateViewActions()
1416{ 1420​{
1417 m_actionHandler->updateViewActions(); 1421​ m_actionHandler->updateViewActions();
1418 1422​
1419 QAction* showFilterBarAction = actionCollection()->action(QStringLiteral("show_filter_bar")); 1423​ QAction* showFilterBarAction = actionCollection()->action(QStringLiteral("show_filter_bar"));
▲ Show 20 Lines • Show All 205 Lines • Show Last 20 Lines