Changeset View
Standalone View
src/dolphinmainwindow.cpp
| Context not available. | |||||
| 57 | #include <KJobWidgets> | 57 | #include <KJobWidgets> | ||
|---|---|---|---|---|---|
| 58 | #include <KLocalizedString> | 58 | #include <KLocalizedString> | ||
| 59 | #include <KMessageBox> | 59 | #include <KMessageBox> | ||
| 60 | #include <KNS3/KMoreToolsMenuFactory> | ||||
| 60 | #include <KProtocolInfo> | 61 | #include <KProtocolInfo> | ||
| 61 | #include <KProtocolManager> | 62 | #include <KProtocolManager> | ||
| 62 | #include <KRun> | 63 | #include <KRun> | ||
| Context not available. | |||||
| 187 | toolBar()->installEventFilter(middleClickEventFilter); | 188 | toolBar()->installEventFilter(middleClickEventFilter); | ||
| 188 | 189 | | |||
| 189 | setupWhatsThis(); | 190 | setupWhatsThis(); | ||
| 191 | | ||||
| 192 | QTimer::singleShot(0, this, &DolphinMainWindow::setupUpdateOpenPreferredSearchToolAction); | ||||
| 190 | } | 193 | } | ||
| 191 | 194 | | |||
| 192 | DolphinMainWindow::~DolphinMainWindow() | 195 | DolphinMainWindow::~DolphinMainWindow() | ||
| Context not available. | |||||
| 872 | } | 875 | } | ||
| 873 | } | 876 | } | ||
| 874 | 877 | | |||
| 875 | void DolphinMainWindow::openTerminal() | 878 | QString DolphinMainWindow::localPath() | ||
| 876 | { | 879 | { | ||
| 877 | QString dir(QDir::homePath()); | | |||
| 878 | | ||||
| 879 | // If the given directory is not local, it can still be the URL of an | | |||
| 880 | // ioslave using UDS_LOCAL_PATH which to be converted first. | | |||
| 881 | KIO::StatJob* statJob = KIO::mostLocalUrl(m_activeViewContainer->url()); | 880 | KIO::StatJob* statJob = KIO::mostLocalUrl(m_activeViewContainer->url()); | ||
elvisangelaccio: Coding style: opening brace should go to next line | |||||
| 882 | KJobWidgets::setWindow(statJob, this); | 881 | KJobWidgets::setWindow(statJob, this); | ||
| 883 | statJob->exec(); | 882 | statJob->exec(); | ||
| 884 | QUrl url = statJob->mostLocalUrl(); | 883 | QUrl url = statJob->mostLocalUrl(); | ||
| 885 | | ||||
| 886 | //If the URL is local after the above conversion, set the directory. | | |||
| 887 | if (url.isLocalFile()) { | 884 | if (url.isLocalFile()) { | ||
| 888 | dir = url.toLocalFile(); | 885 | return url.toLocalFile(); | ||
| 889 | } | 886 | } | ||
| 887 | return QDir::homePath(); | ||||
| 888 | } | ||||
| 890 | 889 | | |||
| 891 | KToolInvocation::invokeTerminal(QString(), dir); | 890 | QPointer<QAction> DolphinMainWindow::preferredSearchTool() | ||
| 891 | { | ||||
| 892 | m_searchTools.clear(); | ||||
| 893 | KMoreToolsMenuFactory("dolphin/search-tools").fillMenuFromGroupingNames( | ||||
| 894 | &m_searchTools, { "files-find" }, QUrl(localPath()) | ||||
elvisangelaccio: `QUrl::fromLocalFile()` | |||||
| 895 | ); | ||||
| 896 | QList<QAction*> actions = m_searchTools.actions(); | ||||
| 897 | if (actions.isEmpty()) { | ||||
| 898 | return nullptr; | ||||
| 899 | } | ||||
| 900 | QAction* action = actions.first(); | ||||
| 901 | if (action->isSeparator()) { | ||||
| 902 | return nullptr; | ||||
| 903 | } | ||||
| 904 | return action; | ||||
| 905 | } | ||||
| 906 | | ||||
| 907 | void DolphinMainWindow::setupUpdateOpenPreferredSearchToolAction() | ||||
| 908 | { | ||||
| 909 | QAction* openPreferredSearchTool = actionCollection()->action(QStringLiteral("open_preferred_search_tool")); | ||||
| 910 | for (QWidget* widget : openPreferredSearchTool->associatedWidgets()) { | ||||
This might detach the QList container. Either use a temp 'const QList` variable or use qAsConst elvisangelaccio: This might detach the QList container. Either use a temp 'const QList` variable or use… | |||||
| 911 | QMenu* menu = dynamic_cast<QMenu*>(widget); | ||||
elvisangelaccio: `qobject_cast` | |||||
| 912 | if (menu) { | ||||
| 913 | connect(menu, &QMenu::aboutToShow, this, &DolphinMainWindow::updateOpenPreferredSearchToolAction); | ||||
| 914 | } | ||||
| 915 | } | ||||
| 916 | connect( | ||||
| 917 | actionCollection()->action(KStandardAction::name(KStandardAction::KeyBindings)), &QAction::hovered, | ||||
| 918 | this, &DolphinMainWindow::updateOpenPreferredSearchToolAction | ||||
| 919 | ); | ||||
elvisangelaccio: I don't get this connection. Can you explain what's needed for? | |||||
There is no other way to update the open_preferred_search_tool action *before* the Configure Shortcuts window is shown. pdabrowski: There is no other way to update the open_preferred_search_tool action *before* the Configure… | |||||
elvisangelaccio: Please write this information in a comment above that `connect()`. | |||||
| 920 | updateOpenPreferredSearchToolAction(); | ||||
| 921 | } | ||||
| 922 | | ||||
| 923 | void DolphinMainWindow::updateOpenPreferredSearchToolAction() | ||||
| 924 | { | ||||
| 925 | QAction* openPreferredSearchTool = actionCollection()->action(QStringLiteral("open_preferred_search_tool")); | ||||
| 926 | if (!openPreferredSearchTool) { | ||||
| 927 | return; | ||||
| 928 | } | ||||
| 929 | QPointer<QAction> tool = preferredSearchTool(); | ||||
| 930 | if (tool) { | ||||
| 931 | openPreferredSearchTool->setVisible(true); | ||||
| 932 | openPreferredSearchTool->setText(i18nc("@action:inmenu Tools", "Open %1", tool->text())); | ||||
| 933 | openPreferredSearchTool->setIcon(tool->icon()); | ||||
| 934 | } else { | ||||
| 935 | openPreferredSearchTool->setVisible(false); | ||||
| 936 | // still visible in Shortcuts configuration window | ||||
| 937 | openPreferredSearchTool->setText(i18nc("@action:inmenu Tools", "Open Preferred Search Tool")); | ||||
| 938 | openPreferredSearchTool->setIcon(QIcon::fromTheme(QStringLiteral("search"))); | ||||
| 939 | } | ||||
| 940 | } | ||||
| 941 | | ||||
| 942 | void DolphinMainWindow::openPreferredSearchTool() | ||||
| 943 | { | ||||
exec() on a job is generally bad, but looks like it was like this already? broulik: `exec()` on a job is generally bad, but looks like it was like this already? | |||||
Yes, it was like that in DolphinMainWindow::openTerminal() before. // If the given directory is not local, it can still be the URL of an // ioslave using UDS_LOCAL_PATH which to be converted first. pdabrowski: Yes, it was like that in DolphinMainWindow::openTerminal() before.
It was needed as explained… | |||||
| 944 | QPointer<QAction> tool = preferredSearchTool(); | ||||
| 945 | if (tool) { | ||||
| 946 | tool->trigger(); | ||||
| 947 | } | ||||
| 948 | } | ||||
| 949 | | ||||
| 950 | void DolphinMainWindow::openTerminal() | ||||
| 951 | { | ||||
| 952 | KToolInvocation::invokeTerminal(QString(), localPath()); | ||||
| 892 | } | 953 | } | ||
| 893 | 954 | | |||
| 894 | void DolphinMainWindow::editSettings() | 955 | void DolphinMainWindow::editSettings() | ||
elvisangelaccio: Missing `const` | |||||
| Context not available. | |||||
| 1063 | QMenu* toolsMenu = new QMenu(i18nc("@action:inmenu", "Tools"), menu); | 1124 | QMenu* toolsMenu = new QMenu(i18nc("@action:inmenu", "Tools"), menu); | ||
| 1064 | toolsMenu->addAction(ac->action(QStringLiteral("show_filter_bar"))); | 1125 | toolsMenu->addAction(ac->action(QStringLiteral("show_filter_bar"))); | ||
| 1065 | toolsMenu->addAction(ac->action(QStringLiteral("compare_files"))); | 1126 | toolsMenu->addAction(ac->action(QStringLiteral("compare_files"))); | ||
| 1127 | toolsMenu->addAction(ac->action(QStringLiteral("open_preferred_search_tool"))); | ||||
| 1066 | toolsMenu->addAction(ac->action(QStringLiteral("open_terminal"))); | 1128 | toolsMenu->addAction(ac->action(QStringLiteral("open_terminal"))); | ||
| 1067 | toolsMenu->addAction(ac->action(QStringLiteral("change_remote_encoding"))); | 1129 | toolsMenu->addAction(ac->action(QStringLiteral("change_remote_encoding"))); | ||
| 1068 | menu->addMenu(toolsMenu); | 1130 | menu->addMenu(toolsMenu); | ||
| 1131 | connect(toolsMenu, &QMenu::aboutToShow, this, &DolphinMainWindow::updateOpenPreferredSearchToolAction); | ||||
| 1069 | 1132 | | |||
| 1070 | // Add "Settings" menu entries | 1133 | // Add "Settings" menu entries | ||
| 1071 | addActionToMenu(ac->action(KStandardAction::name(KStandardAction::KeyBindings)), menu); | 1134 | addActionToMenu(ac->action(KStandardAction::name(KStandardAction::KeyBindings)), menu); | ||
Since this patch was made, we've re-organized the hamburger menu a bit to simplify it and hide not-commonly-used functionality. I think this feature counts, since it's an alternative to the built-in search, and as such, it won't be useful for the majority of Dolphin's users who use the built-in search instead. Could you remove it from the hamburger menu? ngraham: Since this patch was made, we've re-organized the hamburger menu a bit to simplify it and hide… | |||||
I've added this action everywhere where "Open Terminal" appeared. If you say the hamburger menu should not contain it though, I will remove it from there. pdabrowski: I've added this action everywhere where "Open Terminal" appeared. If you say the hamburger menu… | |||||
One more thing to consider: no alternative tool (including KFind) is installed by default (at least in Kubuntu and KDE Neon). So by default this action will not appear at all. pdabrowski: One more thing to consider: no alternative tool (including KFind) is installed by default (at… | |||||
| Context not available. | |||||
| 1400 | compareFiles->setEnabled(false); | 1463 | compareFiles->setEnabled(false); | ||
| 1401 | connect(compareFiles, &QAction::triggered, this, &DolphinMainWindow::compareFiles); | 1464 | connect(compareFiles, &QAction::triggered, this, &DolphinMainWindow::compareFiles); | ||
| 1402 | 1465 | | |||
| 1466 | QAction* openPreferredSearchTool = actionCollection()->addAction(QStringLiteral("open_preferred_search_tool")); | ||||
| 1467 | openPreferredSearchTool->setText(i18nc("@action:inmenu Tools", "Open Preferred Search Tool")); | ||||
| 1468 | openPreferredSearchTool->setWhatsThis(xi18nc("@info:whatsthis", | ||||
| 1469 | "<para>This opens a preferred search tool for the viewed location.</para>" | ||||
| 1470 | "<para>Use <emphasis>More Search Tools</emphasis> menu to configure it.</para>")); | ||||
| 1471 | openPreferredSearchTool->setIcon(QIcon::fromTheme(QStringLiteral("search"))); | ||||
| 1472 | actionCollection()->setDefaultShortcut(openPreferredSearchTool, Qt::CTRL + Qt::SHIFT + Qt::Key_F); | ||||
| 1473 | connect(openPreferredSearchTool, &QAction::triggered, this, &DolphinMainWindow::openPreferredSearchTool); | ||||
| 1474 | | ||||
| 1403 | #ifdef HAVE_TERMINAL | 1475 | #ifdef HAVE_TERMINAL | ||
| 1404 | if (KAuthorized::authorize(QStringLiteral("shell_access"))) { | 1476 | if (KAuthorized::authorize(QStringLiteral("shell_access"))) { | ||
| 1405 | QAction* openTerminal = actionCollection()->addAction(QStringLiteral("open_terminal")); | 1477 | QAction* openTerminal = actionCollection()->addAction(QStringLiteral("open_terminal")); | ||
| Context not available. | |||||
| 2123 | QWhatsThisClickedEvent* whatsThisEvent = dynamic_cast<QWhatsThisClickedEvent*>(event); | 2195 | QWhatsThisClickedEvent* whatsThisEvent = dynamic_cast<QWhatsThisClickedEvent*>(event); | ||
| 2124 | QDesktopServices::openUrl(QUrl(whatsThisEvent->href())); | 2196 | QDesktopServices::openUrl(QUrl(whatsThisEvent->href())); | ||
| 2125 | return true; | 2197 | return true; | ||
| 2198 | } else if (event->type() == QEvent::WindowActivate) { | ||||
| 2199 | updateOpenPreferredSearchToolAction(); | ||||
| 2126 | } | 2200 | } | ||
| 2127 | return KXmlGuiWindow::event(event); | 2201 | return KXmlGuiWindow::event(event); | ||
| 2128 | } | 2202 | } | ||
| Context not available. | |||||
This causes quite severe bug https://bugs.kde.org/show_bug.cgi?id=420911 broulik: This causes quite severe bug https://bugs.kde.org/show_bug.cgi?id=420911
Why is this needed… | |||||
It was added to handle this action as a button in toolbar (can be added by user, and this was requested in #384798). I didn't notice this causes such a problem with remote connections, sorry. It is a rare occasion that user changes preferred search tools, but I guess they would like to have it updated all over Dolphin when they do. pdabrowski: It was added to handle this action as a button in toolbar (can be added by user, and this was… | |||||
So, the URL does not have any impact on the search tool chosen? I thought that was the main reason for using the URL and updating it all the time. broulik: > It is a rare occasion that user changes preferred search tools,
So, the URL does not have… | |||||
This depends on KMoreToolsMenuFactory. But it doesn't seem to choose different tools for different URLs. The main reason for the updates was so that all the Open Preferred Search Tool actions reflect the user choice of preferred tool (can be modified with More Search Tools -> More -> Configure) pdabrowski: >So, the URL does not have any impact on the search tool chosen? I thought that was the main… | |||||
Then there should be a signal in KMoreTools a user of this class can connect to to get notified when user preference changes. Or, the actions were actually governed by KMoreTools and automatically updated. broulik: > (can be modified with More Search Tools -> More -> Configure)
Then there should be a signal… | |||||
It is still impossible to detect (un)installation of such tool. pdabrowski: It is still impossible to detect (un)installation of such tool.
"More Search Tools" menu… | |||||
There's signals when the KSycoca service database changes which we use in the application menu to update when apps are installed/uninstalled. broulik: > It is still impossible to detect (un)installation of such tool.
There's signals when the… | |||||
I agree with @broulik. This is a niche use case anyway, so if we really need to support it, we should do it right without nasty side effects. elvisangelaccio: I agree with @broulik. This is a niche use case anyway, so if we really need to support it, we… | |||||
D29568: use KSycoca for updating OpenPreferredSearchTool action pdabrowski: D29568: use KSycoca for updating OpenPreferredSearchTool action | |||||
Coding style: opening brace should go to next line