Differential D10990 Diff 29481 src/dolphinmainwindow.cpp

Changeset View

Standalone View

View Options

src/dolphinmainwindow.cpp

Show First 20 Lines • Show All 45 Lines • ▼ Show 20 Line(s)
46#include <KActionCollection> 46​#include <KActionCollection>
47#include <KActionMenu> 47​#include <KActionMenu>
48#include <KAuthorized> 48​#include <KAuthorized>
49#include <KConfig> 49​#include <KConfig>
50#include <KFileItemListProperties> 50​#include <KFileItemListProperties>
51#include <KFilePlacesModel> 51​#include <KFilePlacesModel>
52#include <KHelpMenu> 52​#include <KHelpMenu>
53#include <KIO/JobUiDelegate> 53​#include <KIO/JobUiDelegate>
54​#include <KIO/OpenFileManagerWindowJob>
54#include <KJobWidgets> 55​#include <KJobWidgets>
55#include <KLocalizedString> 56​#include <KLocalizedString>
56#include <KMessageBox> 57​#include <KMessageBox>
57#include <KProtocolInfo> 58​#include <KProtocolInfo>
58#include <KProtocolManager> 59​#include <KProtocolManager>
59#include <KRun> 60​#include <KRun>
60#include <KShell> 61​#include <KShell>
61#include <KStandardAction> 62​#include <KStandardAction>
62#include <KToggleAction> 63​#include <KToggleAction>
63#include <KToolBar> 64​#include <KToolBar>
64#include <KToolInvocation> 65​#include <KToolInvocation>
65#include <KUrlComboBox> 66​#include <KUrlComboBox>
66#include <KUrlNavigator> 67​#include <KUrlNavigator>
67 68​
68#include <QApplication> 69​#include <QApplication>
69#include <QClipboard> 70​#include <QClipboard>
70#include <QCloseEvent> 71​#include <QCloseEvent>
71#include <QDialog> 72​#include <QDialog>
73​#include <QFileInfo>
72#include <QLineEdit> 74​#include <QLineEdit>
73#include <QMenu> 75​#include <QMenu>
74#include <QMenuBar> 76​#include <QMenuBar>
75#include <QPushButton> 77​#include <QPushButton>
76#include <QShowEvent> 78​#include <QShowEvent>
77#include <QStandardPaths> 79​#include <QStandardPaths>
78#include <QTimer> 80​#include <QTimer>
79#include <QToolButton> 81​#include <QToolButton>
▲ Show 20 Lines • Show All 282 Lines • ▼ Show 20 Line(s)361{
362 if (!event->spontaneous()) { 364​ if (!event->spontaneous()) {
363 m_activeViewContainer->view()->setFocus(); 365​ m_activeViewContainer->view()->setFocus();
364 } 366​ }
365} 367​}
366 368​
367void DolphinMainWindow::closeEvent(QCloseEvent* event) 369​void DolphinMainWindow::closeEvent(QCloseEvent* event)
368{ 370​{
369 // Find out if Dolphin is closed directly by the user or 371​ // Find out if Dolphin is closed directly by the user or
370 // by the session manager because the session is closed 372​ // by the session manager because the session is closed
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>."…
371 bool closedByUser = true; 373​ bool closedByUser = true;
372 if (qApp->isSavingSession()) { 374​ if (qApp->isSavingSession()) {
373 closedByUser = false; 375​ closedByUser = false;
374 } 376​ }
375 377​
376 if (m_tabWidget->count() > 1 && GeneralSettings::confirmClosingMultipleTabs() && closedByUser) { 378​ if (m_tabWidget->count() > 1 && GeneralSettings::confirmClosingMultipleTabs() && closedByUser) {
377 // Ask the user if he really wants to quit and close all tabs. 379​ // Ask the user if he really wants to quit and close all tabs.
378 // Open a confirmation dialog with 3 buttons: 380​ // Open a confirmation dialog with 3 buttons:
▲ Show 20 Lines • Show All 829 Lines • ▼ Show 20 Line(s)1188#endif
1208 QAction* activatePrevTab = actionCollection()->addAction(QStringLiteral("activate_prev_tab")); 1210​ QAction* activatePrevTab = actionCollection()->addAction(QStringLiteral("activate_prev_tab"));
1209 activatePrevTab->setIconText(i18nc("@action:inmenu", "Previous Tab")); 1211​ activatePrevTab->setIconText(i18nc("@action:inmenu", "Previous Tab"));
1210 activatePrevTab->setText(i18nc("@action:inmenu", "Activate Previous Tab")); 1212​ activatePrevTab->setText(i18nc("@action:inmenu", "Activate Previous Tab"));
1211 activatePrevTab->setEnabled(false); 1213​ activatePrevTab->setEnabled(false);
1212 connect(activatePrevTab, &QAction::triggered, m_tabWidget, &DolphinTabWidget::activatePrevTab); 1214​ connect(activatePrevTab, &QAction::triggered, m_tabWidget, &DolphinTabWidget::activatePrevTab);
1213 actionCollection()->setDefaultShortcuts(activatePrevTab, prevTabKeys); 1215​ actionCollection()->setDefaultShortcuts(activatePrevTab, prevTabKeys);
1214 1216​
1215 // for context menu 1217​ // for context menu
1218​ QAction* showOriginal = actionCollection()->addAction(QStringLiteral("show_original"));
1219​ showOriginal->setText(i18nc("@action:inmenu", "Show Original"));
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")));
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);
1222​ connect(showOriginal, &QAction::triggered, [this]() {
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);
elvisangelaccioUnsubmitted
Done

Missing braces

elvisangelaccio: Missing braces
1228​ if (QFileInfo(linkDestination).exists()) {
elvisangelaccioUnsubmitted
Done

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

elvisangelaccio: Use the static `QFileInfo::exists()` which is faster.
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​ });
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.
1235​
1216 QAction* openInNewTab = actionCollection()->addAction(QStringLiteral("open_in_new_tab")); 1236​ QAction* openInNewTab = actionCollection()->addAction(QStringLiteral("open_in_new_tab"));
1217 openInNewTab->setText(i18nc("@action:inmenu", "Open in New Tab")); 1237​ openInNewTab->setText(i18nc("@action:inmenu", "Open in New Tab"));
1218 openInNewTab->setIcon(QIcon::fromTheme(QStringLiteral("tab-new"))); 1238​ openInNewTab->setIcon(QIcon::fromTheme(QStringLiteral("tab-new")));
1219 connect(openInNewTab, &QAction::triggered, this, &DolphinMainWindow::openInNewTab); 1239​ connect(openInNewTab, &QAction::triggered, this, &DolphinMainWindow::openInNewTab);
1220 1240​
1221 QAction* openInNewTabs = actionCollection()->addAction(QStringLiteral("open_in_new_tabs")); 1241​ QAction* openInNewTabs = actionCollection()->addAction(QStringLiteral("open_in_new_tabs"));
1222 openInNewTabs->setText(i18nc("@action:inmenu", "Open in New Tabs")); 1242​ openInNewTabs->setText(i18nc("@action:inmenu", "Open in New Tabs"));
1223 openInNewTabs->setIcon(QIcon::fromTheme(QStringLiteral("tab-new"))); 1243​ openInNewTabs->setIcon(QIcon::fromTheme(QStringLiteral("tab-new")));
▲ Show 20 Lines • Show All 144 Lines • ▼ Show 20 Line(s)
1368{ 1388​{
1369 const KFileItemList list = m_activeViewContainer->view()->selectedItems(); 1389​ const KFileItemList list = m_activeViewContainer->view()->selectedItems();
1370 if (list.isEmpty()) { 1390​ if (list.isEmpty()) {
1371 stateChanged(QStringLiteral("has_no_selection")); 1391​ stateChanged(QStringLiteral("has_no_selection"));
1372 } else { 1392​ } else {
1373 stateChanged(QStringLiteral("has_selection")); 1393​ stateChanged(QStringLiteral("has_selection"));
1374 1394​
1375 KActionCollection* col = actionCollection(); 1395​ KActionCollection* col = actionCollection();
1376 QAction* renameAction = col->action(KStandardAction::name(KStandardAction::RenameFile)); 1396​ QAction* renameAction = col->action(KStandardAction::name(KStandardAction::RenameFile));
1377 QAction* moveToTrashAction = col->action(KStandardAction::name(KStandardAction::MoveToTrash)); 1397​ QAction* moveToTrashAction = col->action(KStandardAction::name(KStandardAction::MoveToTrash));
1378 QAction* deleteAction = col->action(KStandardAction::name(KStandardAction::DeleteFile)); 1398​ QAction* deleteAction = col->action(KStandardAction::name(KStandardAction::DeleteFile));
1379 QAction* cutAction = col->action(KStandardAction::name(KStandardAction::Cut)); 1399​ QAction* cutAction = col->action(KStandardAction::name(KStandardAction::Cut));
1380 QAction* deleteWithTrashShortcut = col->action(QStringLiteral("delete_shortcut")); // see DolphinViewActionHandler 1400​ QAction* deleteWithTrashShortcut = col->action(QStringLiteral("delete_shortcut")); // see DolphinViewActionHandler
1401​ QAction* showOriginal = col->action(QStringLiteral("show_original"));
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…
1381 1402​
1382 KFileItemListProperties capabilities(list); 1403​ KFileItemListProperties capabilities(list);
1383 const bool enableMoveToTrash = capabilities.isLocal() && capabilities.supportsMoving(); 1404​ const bool enableMoveToTrash = capabilities.isLocal() && capabilities.supportsMoving();
1384 1405​
1385 renameAction->setEnabled(capabilities.supportsMoving()); 1406​ renameAction->setEnabled(capabilities.supportsMoving());
1386 moveToTrashAction->setEnabled(enableMoveToTrash); 1407​ moveToTrashAction->setEnabled(enableMoveToTrash);
1387 deleteAction->setEnabled(capabilities.supportsDeleting()); 1408​ deleteAction->setEnabled(capabilities.supportsDeleting());
1388 deleteWithTrashShortcut->setEnabled(capabilities.supportsDeleting() && !enableMoveToTrash); 1409​ deleteWithTrashShortcut->setEnabled(capabilities.supportsDeleting() && !enableMoveToTrash);
1389 cutAction->setEnabled(capabilities.supportsMoving()); 1410​ cutAction->setEnabled(capabilities.supportsMoving());
1411​ showOriginal->setEnabled(list.at(0).isLink());
1390 } 1412​ }
1391} 1413​}
1392 1414​
1393void DolphinMainWindow::updateViewActions() 1415​void DolphinMainWindow::updateViewActions()
1394{ 1416​{
1395 m_actionHandler->updateViewActions(); 1417​ m_actionHandler->updateViewActions();
1396 1418​
1397 QAction* showFilterBarAction = actionCollection()->action(QStringLiteral("show_filter_bar")); 1419​ QAction* showFilterBarAction = actionCollection()->action(QStringLiteral("show_filter_bar"));
▲ Show 20 Lines • Show All 205 Lines • Show Last 20 Lines