Differential D10990 Diff 28675 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>
▲ Show 20 Lines • Show All 300 Lines • ▼ Show 20 Line(s)360{
362 if (!event->spontaneous()) { 363​ if (!event->spontaneous()) {
363 m_activeViewContainer->view()->setFocus(); 364​ m_activeViewContainer->view()->setFocus();
364 } 365​ }
365} 366​}
366 367​
367void DolphinMainWindow::closeEvent(QCloseEvent* event) 368​void DolphinMainWindow::closeEvent(QCloseEvent* event)
368{ 369​{
369 // Find out if Dolphin is closed directly by the user or 370​ // Find out if Dolphin is closed directly by the user or
370 // by the session manager because the session is closed 371​ // 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; 372​ bool closedByUser = true;
372 if (qApp->isSavingSession()) { 373​ if (qApp->isSavingSession()) {
373 closedByUser = false; 374​ closedByUser = false;
374 } 375​ }
375 376​
376 if (m_tabWidget->count() > 1 && GeneralSettings::confirmClosingMultipleTabs() && closedByUser) { 377​ if (m_tabWidget->count() > 1 && GeneralSettings::confirmClosingMultipleTabs() && closedByUser) {
377 // Ask the user if he really wants to quit and close all tabs. 378​ // Ask the user if he really wants to quit and close all tabs.
378 // Open a confirmation dialog with 3 buttons: 379​ // Open a confirmation dialog with 3 buttons:
▲ Show 20 Lines • Show All 830 Lines • ▼ Show 20 Line(s)1188#endif
1209 QAction* activatePrevTab = actionCollection()->addAction(QStringLiteral("activate_prev_tab")); 1210​ QAction* activatePrevTab = actionCollection()->addAction(QStringLiteral("activate_prev_tab"));
1210 activatePrevTab->setIconText(i18nc("@action:inmenu", "Previous Tab")); 1211​ activatePrevTab->setIconText(i18nc("@action:inmenu", "Previous Tab"));
1211 activatePrevTab->setText(i18nc("@action:inmenu", "Activate Previous Tab")); 1212​ activatePrevTab->setText(i18nc("@action:inmenu", "Activate Previous Tab"));
1212 activatePrevTab->setEnabled(false); 1213​ activatePrevTab->setEnabled(false);
1213 connect(activatePrevTab, &QAction::triggered, m_tabWidget, &DolphinTabWidget::activatePrevTab); 1214​ connect(activatePrevTab, &QAction::triggered, m_tabWidget, &DolphinTabWidget::activatePrevTab);
1214 actionCollection()->setDefaultShortcuts(activatePrevTab, prevTabKeys); 1215​ actionCollection()->setDefaultShortcuts(activatePrevTab, prevTabKeys);
1215 1216​
1216 // for context menu 1217​ // for context menu
1218​ QAction* openSymlinkDestinationFolder = actionCollection()->addAction(QStringLiteral("open_symlink_destination_folder"));
1219​ openSymlinkDestinationFolder->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​ openSymlinkDestinationFolder->setIcon(QIcon::fromTheme(QStringLiteral("folder-open")));
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​ connect(openSymlinkDestinationFolder, &QAction::triggered, [this]() {
1222​ const KFileItemList& list = m_activeViewContainer->view()->selectedItems();
elvisangelaccioUnsubmitted
Done

Missing this as receiver parameter.

elvisangelaccio: Missing `this` as receiver parameter.
1223​ KIO::highlightInFileManager({QUrl::fromLocalFile(list.at(0).linkDest())});
1224​ });
1225​
1217 QAction* openInNewTab = actionCollection()->addAction(QStringLiteral("open_in_new_tab")); 1226​ QAction* openInNewTab = actionCollection()->addAction(QStringLiteral("open_in_new_tab"));
1218 openInNewTab->setText(i18nc("@action:inmenu", "Open in New Tab")); 1227​ openInNewTab->setText(i18nc("@action:inmenu", "Open in New Tab"));
elvisangelaccioUnsubmitted
Done

Missing braces

elvisangelaccio: Missing braces
1219 openInNewTab->setIcon(QIcon::fromTheme(QStringLiteral("tab-new"))); 1228​ openInNewTab->setIcon(QIcon::fromTheme(QStringLiteral("tab-new")));
elvisangelaccioUnsubmitted
Done

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

elvisangelaccio: Use the static `QFileInfo::exists()` which is faster.
1220 connect(openInNewTab, &QAction::triggered, this, &DolphinMainWindow::openInNewTab); 1229​ connect(openInNewTab, &QAction::triggered, this, &DolphinMainWindow::openInNewTab);
1221 1230​
1222 QAction* openInNewTabs = actionCollection()->addAction(QStringLiteral("open_in_new_tabs")); 1231​ QAction* openInNewTabs = actionCollection()->addAction(QStringLiteral("open_in_new_tabs"));
1223 openInNewTabs->setText(i18nc("@action:inmenu", "Open in New Tabs")); 1232​ openInNewTabs->setText(i18nc("@action:inmenu", "Open in New Tabs"));
1224 openInNewTabs->setIcon(QIcon::fromTheme(QStringLiteral("tab-new"))); 1233​ openInNewTabs->setIcon(QIcon::fromTheme(QStringLiteral("tab-new")));
1225 connect(openInNewTabs, &QAction::triggered, this, &DolphinMainWindow::openInNewTab); 1234​ connect(openInNewTabs, &QAction::triggered, this, &DolphinMainWindow::openInNewTab);
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.
1226 1235​
1227 QAction* openInNewWindow = actionCollection()->addAction(QStringLiteral("open_in_new_window")); 1236​ QAction* openInNewWindow = actionCollection()->addAction(QStringLiteral("open_in_new_window"));
1228 openInNewWindow->setText(i18nc("@action:inmenu", "Open in New Window")); 1237​ openInNewWindow->setText(i18nc("@action:inmenu", "Open in New Window"));
1229 openInNewWindow->setIcon(QIcon::fromTheme(QStringLiteral("window-new"))); 1238​ openInNewWindow->setIcon(QIcon::fromTheme(QStringLiteral("window-new")));
1230 connect(openInNewWindow, &QAction::triggered, this, &DolphinMainWindow::openInNewWindow); 1239​ connect(openInNewWindow, &QAction::triggered, this, &DolphinMainWindow::openInNewWindow);
1231} 1240​}
1232 1241​
1233void DolphinMainWindow::setupDockWidgets() 1242​void DolphinMainWindow::setupDockWidgets()
▲ Show 20 Lines • Show All 138 Lines • ▼ Show 20 Line(s)1380 if (list.isEmpty()) {
1372 stateChanged(QStringLiteral("has_no_selection")); 1381​ stateChanged(QStringLiteral("has_no_selection"));
1373 } else { 1382​ } else {
1374 stateChanged(QStringLiteral("has_selection")); 1383​ stateChanged(QStringLiteral("has_selection"));
1375 1384​
1376 KActionCollection* col = actionCollection(); 1385​ KActionCollection* col = actionCollection();
1377 QAction* renameAction = col->action(KStandardAction::name(KStandardAction::RenameFile)); 1386​ QAction* renameAction = col->action(KStandardAction::name(KStandardAction::RenameFile));
1378 QAction* moveToTrashAction = col->action(KStandardAction::name(KStandardAction::MoveToTrash)); 1387​ QAction* moveToTrashAction = col->action(KStandardAction::name(KStandardAction::MoveToTrash));
1379 QAction* deleteAction = col->action(KStandardAction::name(KStandardAction::DeleteFile)); 1388​ QAction* deleteAction = col->action(KStandardAction::name(KStandardAction::DeleteFile));
1380 QAction* cutAction = col->action(KStandardAction::name(KStandardAction::Cut)); 1389​ QAction* cutAction = col->action(KStandardAction::name(KStandardAction::Cut));
1381 QAction* deleteWithTrashShortcut = col->action(QStringLiteral("delete_shortcut")); // see DolphinViewActionHandler 1390​ QAction* deleteWithTrashShortcut = col->action(QStringLiteral("delete_shortcut")); // see DolphinViewActionHandler
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…
1382 1391​
1383 KFileItemListProperties capabilities(list); 1392​ KFileItemListProperties capabilities(list);
1384 const bool enableMoveToTrash = capabilities.isLocal() && capabilities.supportsMoving(); 1393​ const bool enableMoveToTrash = capabilities.isLocal() && capabilities.supportsMoving();
1385 1394​
1386 renameAction->setEnabled(capabilities.supportsMoving()); 1395​ renameAction->setEnabled(capabilities.supportsMoving());
1387 moveToTrashAction->setEnabled(enableMoveToTrash); 1396​ moveToTrashAction->setEnabled(enableMoveToTrash);
1388 deleteAction->setEnabled(capabilities.supportsDeleting()); 1397​ deleteAction->setEnabled(capabilities.supportsDeleting());
1389 deleteWithTrashShortcut->setEnabled(capabilities.supportsDeleting() && !enableMoveToTrash); 1398​ deleteWithTrashShortcut->setEnabled(capabilities.supportsDeleting() && !enableMoveToTrash);
▲ Show 20 Lines • Show All 214 Lines • Show Last 20 Lines