Open externally called files/directories in new tabs

Summary:
FEATURE: 183429
FIXED-IN: 19.08.0
GUI: new cli argument --new-window

Externally called files/directories are opened in a a new tab of an instance of Dolphin that already exists. If any of the given URIs are already open in a tab, then those tabs are activated instead of a new tab being opened.  If there is no instance then the files/directories are opened in a new window. The newly opened file/directory has its tab activated, and consequently, the window is also activated.

When the user clicks "Open In New Window" or "Detach Tab", the files/directories are opened in a new window.

Test Plan:
[Manual]
Before testing, set the default file manager in system settings as the newly built Dolphin executable.
One must also include the new dolphin executable in the $PATH, otherwise some functions will attempt to open the system dolphin instead of the new one.

Furthermore, running two different versions of Dolphin (in particular, where one does not have this patch included) can result in bugs appearing, in particular, new tabs not opening as old instances will not recognise the DBus commands sent to it. However, I see no reason why a user will have two different versions of Dolphin (apart from people like us :D).

Open directories with the help of auxillary programs (i.e. a browser). The files/directories should appear in a new window if an instance does not exist. If an existence already exists, then a new tab should be opened and activated in that instance and the window activated.
Use QDBusViewer to open folders/items by calling the ShowFolders/ShowItems methods in org.freedesktop.FileManager1 of the Dolphin instance.
When a user chooses to "Open In New Window"/"Detach Tab" then the files/directories should be opened in a new window.

Reviewers: #dolphin, elvisangelaccio

Subscribers: zzag, dfaure, fvogt, fikrim, magar, fbg13, davidedmundson, kwin, ngraham, elvisangelaccio, anthonyfieroni, kfm-devel

Tags: #dolphin

Differential Revision: https://phabricator.kde.org/D16648

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7816b5717e4a0323f7a2bf044f3dbaf9a4330b1e..2feef8e28f856104bbca3e1d9c1093acdf0d5c7d 100644 (file)
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -64,6 +64,7 @@ find_package(KF5 ${KF5_MIN_VERSION} REQUIRED COMPONENTS
     TextWidgets
     Notifications
     Crash
+    WindowSystem
 )
 find_package(KF5 ${KF5_MIN_VERSION} OPTIONAL_COMPONENTS
     Activities

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index f1b7534aea5d2e46983494afce709fcb4d992f48..8a025e584369bbcc12b24e00414bc16ebf56f92c 100644 (file)
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -148,6 +148,7 @@ target_link_libraries(
     KF5::ConfigCore
     KF5::NewStuff
     KF5::Parts
+    KF5::WindowSystem
 )
 
 if(HAVE_BALOO)

diff --git a/src/dbusinterface.cpp b/src/dbusinterface.cpp
index c780bc7cdc9cb853f4df70cc9d57a4df72183872..4e24354abefe5578c05ac501bea288417000df1f 100644 (file)
--- a/src/dbusinterface.cpp
+++ b/src/dbusinterface.cpp
@@ -19,10 +19,13 @@
 
 #include "dbusinterface.h"
 #include "global.h"
+#include "dolphin_generalsettings.h"
 
 #include <KPropertiesDialog>
 
+#include <QApplication>
 #include <QDBusConnection>
+#include <QDBusInterface>
 #include <QDBusConnectionInterface>
 
 DBusInterface::DBusInterface() :
@@ -41,7 +44,10 @@ void DBusInterface::ShowFolders(const QStringList& uriList, const QString& start
     if (urls.isEmpty()) {
         return;
     }
-    Dolphin::openNewWindow(urls);
+    const auto serviceName = QStringLiteral("org.kde.dolphin-%1").arg(QCoreApplication::applicationPid());
+    if(!Dolphin::attachToExistingInstance(urls, false, GeneralSettings::splitView(), serviceName)) {
+        Dolphin::openNewWindow(urls);
+    }
 }
 
 void DBusInterface::ShowItems(const QStringList& uriList, const QString& startUpId)
@@ -51,7 +57,10 @@ void DBusInterface::ShowItems(const QStringList& uriList, const QString& startUp
     if (urls.isEmpty()) {
         return;
     }
-    Dolphin::openNewWindow(urls, nullptr, Dolphin::OpenNewWindowFlag::Select);
+    const auto serviceName = QStringLiteral("org.kde.dolphin-%1").arg(QCoreApplication::applicationPid());
+    if(!Dolphin::attachToExistingInstance(urls, true, GeneralSettings::splitView(), serviceName)) {
+        Dolphin::openNewWindow(urls);
+    };
 }
 
 void DBusInterface::ShowItemProperties(const QStringList& uriList, const QString& startUpId)

diff --git a/src/dolphinmainwindow.cpp b/src/dolphinmainwindow.cpp
index b57ed4fc481e8aa3bfd1024cee5c5f5c4d0c8a06..385b61a2a7fb8ca9b033854c9ee8bf239471f78f 100644 (file)
--- a/src/dolphinmainwindow.cpp
+++ b/src/dolphinmainwindow.cpp
@@ -61,11 +61,13 @@
 #include <KRun>
 #include <KShell>
 #include <KStandardAction>
+#include <KStartupInfo>
 #include <KToggleAction>
 #include <KToolBar>
 #include <KToolInvocation>
 #include <KUrlComboBox>
 #include <KUrlNavigator>
+#include <KWindowSystem>
 
 #include <QApplication>
 #include <QClipboard>
@@ -200,11 +202,27 @@ void DolphinMainWindow::openDirectories(const QList<QUrl>& dirs, bool splitView)
     m_tabWidget->openDirectories(dirs, splitView);
 }
 
+void DolphinMainWindow::openDirectories(const QStringList& dirs, bool splitView)
+{
+    openDirectories(QUrl::fromStringList(dirs), splitView);
+}
+
 void DolphinMainWindow::openFiles(const QList<QUrl>& files, bool splitView)
 {
     m_tabWidget->openFiles(files, splitView);
 }
 
+void DolphinMainWindow::openFiles(const QStringList& files, bool splitView)
+{
+    openFiles(QUrl::fromStringList(files), splitView);
+}
+
+void DolphinMainWindow::activateWindow()
+{
+    KStartupInfo::setNewStartupId(window(), KStartupInfo::startupId());
+    KWindowSystem::activateWindow(window()->effectiveWinId());
+}
+
 void DolphinMainWindow::showCommand(CommandType command)
 {
     DolphinStatusBar* statusBar = m_activeViewContainer->statusBar();
@@ -1707,3 +1725,12 @@ void DolphinMainWindow::UndoUiInterface::jobError(KIO::Job* job)
     }
 }
 
+bool DolphinMainWindow::isUrlOpen(const QString& url)
+{
+    if (m_tabWidget->getIndexByUrl(QUrl::fromUserInput((url))) >= 0) {
+        return true;
+    } else {
+        return false;
+    }
+}
+

diff --git a/src/dolphinmainwindow.h b/src/dolphinmainwindow.h
index 1e2460768bdfb7488c6a8c8cb501020ecbd47b50..4037c14689fc82d2f429501a35de694c0d70358c 100644 (file)
--- a/src/dolphinmainwindow.h
+++ b/src/dolphinmainwindow.h
@@ -101,6 +101,39 @@ public:
     void setTabsToHomeIfMountPathOpen(const QString& mountPath);
 
 public slots:
+    /**
+     * Opens each directory in \p dirs in a separate tab. If \a splitView is set,
+     * 2 directories are collected within one tab.
+     * \pre \a dirs must contain at least one url.
+     *
+     * @note this function is overloaded so that it is callable via DBus.
+     */
+    void openDirectories(const QStringList &dirs, bool splitView);
+
+    /**
+     * Opens the directories which contain the files \p files and selects all files.
+     * If \a splitView is set, 2 directories are collected within one tab.
+     * \pre \a files must contain at least one url.
+     *
+     * @note this is overloaded so that this function is callable via DBus.
+     */
+    void openFiles(const QStringList &files, bool splitView);
+
+    /**
+     * Tries to raise/activate the Dolphin window.
+     */
+    void activateWindow();
+
+    /**
+     * Determines if a URL is open in any tab.
+     * @note Use of QString instead of QUrl is required to be callable via DBus.
+     *
+     * @param url URL to look for
+     * @returns true if url is currently open in a tab, false otherwise.
+     */
+    bool isUrlOpen(const QString &url);
+
+
     /**
      * Pastes the clipboard data into the currently selected folder
      * of the active view. If not exactly one folder is selected,

diff --git a/src/dolphintabwidget.cpp b/src/dolphintabwidget.cpp
index e54ab5ada87cf0fb24082d6e10b6c55022840316..defd089c16075cf48e1ca31e0a3779eb0aba7faf 100644 (file)
--- a/src/dolphintabwidget.cpp
+++ b/src/dolphintabwidget.cpp
@@ -186,11 +186,16 @@ void DolphinTabWidget::openDirectories(const QList<QUrl>& dirs, bool splitView)
     QList<QUrl>::const_iterator it = dirs.constBegin();
     while (it != dirs.constEnd()) {
         const QUrl& primaryUrl = *(it++);
+        const int index = getIndexByUrl(primaryUrl);
+        if (index >= 0) {
+            setCurrentIndex(index);
+            continue;
+        }
         if (splitView && (it != dirs.constEnd())) {
             const QUrl& secondaryUrl = *(it++);
-            openNewTab(primaryUrl, secondaryUrl);
+            openNewActivatedTab(primaryUrl, secondaryUrl);
         } else {
-            openNewTab(primaryUrl);
+            openNewActivatedTab(primaryUrl);
         }
     }
 }
@@ -290,6 +295,7 @@ void DolphinTabWidget::detachTab(int index)
         args << tabPage->secondaryViewContainer()->url().url();
         args << QStringLiteral("--split");
     }
+    args << QStringLiteral("--new-window");
 
     const QString command = QStringLiteral("dolphin %1").arg(KShell::joinArgs(args));
     KRun::runCommand(command, this);
@@ -374,3 +380,17 @@ QString DolphinTabWidget::tabName(DolphinTabPage* tabPage) const
     // and not misinterpreted as a keyboard shortcut in QTabBar::setTabText()
     return name.replace('&', QLatin1String("&&"));
 }
+
+int DolphinTabWidget::getIndexByUrl(const QUrl& url) const
+{
+    for (int i = 0; i < count(); i++) {
+        // Conversion to display string is necessary to deal with the '~' alias.
+        // i.e. to acknowledge that ~/ is equivalent to /home/user/
+        const QUrl tabUrl = tabPageAt(i)->activeViewContainer()->url();
+        if (url == tabUrl ||
+            url.toDisplayString(QUrl::StripTrailingSlash) == tabUrl.toDisplayString(QUrl::StripTrailingSlash)) {
+            return i;
+        }
+    }
+    return -1;
+}

diff --git a/src/dolphintabwidget.h b/src/dolphintabwidget.h
index 7e3b30c0dff5285cc534d44fa21674070f503804..3e8301725c71e83c69fca4ab62d570aae87d5f51 100644 (file)
--- a/src/dolphintabwidget.h
+++ b/src/dolphintabwidget.h
@@ -78,6 +78,12 @@ public:
      */
     void refreshViews();
 
+    /**
+     * @param url The URL that we would like
+     * @return index of the tab with the desired URL. returns -1 if not found
+     */
+    int getIndexByUrl(const QUrl& url) const;
+
 signals:
     /**
      * Is emitted when the active view has been changed, by changing the current

diff --git a/src/global.cpp b/src/global.cpp
index aaeb5812060e18d7f2a38d494d9afa3afc4eedcd..e7ff67d776578cfa88368e1177b99b1e94c77b91 100644 (file)
--- a/src/global.cpp
+++ b/src/global.cpp
@@ -23,9 +23,12 @@
 #include "dolphindebug.h"
 
 #include <KRun>
+#include <KWindowSystem>
 
 #include <QApplication>
 #include <QIcon>
+#include <QDBusInterface>
+#include <QDBusConnectionInterface>
 
 QList<QUrl> Dolphin::validateUris(const QStringList& uriList)
 {
@@ -49,7 +52,7 @@ QUrl Dolphin::homeUrl()
 
 void Dolphin::openNewWindow(const QList<QUrl> &urls, QWidget *window, const OpenNewWindowFlags &flags)
 {
-    QString command = QStringLiteral("dolphin");
+    QString command = QStringLiteral("dolphin --new-window");
 
     if (flags.testFlag(OpenNewWindowFlag::Select)) {
         command.append(QLatin1String(" --select"));
@@ -58,6 +61,83 @@ void Dolphin::openNewWindow(const QList<QUrl> &urls, QWidget *window, const Open
     if (!urls.isEmpty()) {
         command.append(QLatin1String(" %U"));
     }
+    KRun::run(
+        command,
+        urls,
+        window,
+        QApplication::applicationDisplayName(),
+        QApplication::windowIcon().name()
+    );
+}
+
+bool Dolphin::attachToExistingInstance(const QList<QUrl>& urls, bool openFiles, bool splitView, const QString& preferredService)
+{
+    if (KWindowSystem::isPlatformWayland()) {
+        // TODO: once Wayland clients can raise or activate themselves remove this conditional
+        return false;
+    }
+
+    const QStringList services = QDBusConnection::sessionBus().interface()->registeredServiceNames().value();
+
+    // Don't match the service without trailing "-" (unique instance)
+    const QString pattern = QStringLiteral("org.kde.dolphin-");
+    const QString myPid = QString::number(QCoreApplication::applicationPid());
+    QVector<QPair<QSharedPointer<QDBusInterface>, QStringList>> dolphinServices;
+    if (!preferredService.isEmpty()) {
+        QSharedPointer<QDBusInterface> preferred(
+            new QDBusInterface(preferredService,
+            QStringLiteral("/dolphin/Dolphin_1"),
+            QStringLiteral("org.kde.dolphin.MainWindow"))
+        );
+        if (preferred->isValid()) {
+            dolphinServices.append(qMakePair(preferred, QStringList() ));
+        }
+    }
+
+    // find all dolphin instances
+    for (const QString& service : services) {
+        if (service.startsWith(pattern) && !service.endsWith(myPid)) {
+            // Check if instance can handle our URLs
+            QSharedPointer<QDBusInterface> instance(
+                new QDBusInterface(service,
+                QStringLiteral("/dolphin/Dolphin_1"),
+                QStringLiteral("org.kde.dolphin.MainWindow"))
+            );
+            if (!instance->isValid()) {
+                continue;
+            }
+            dolphinServices.append(qMakePair(instance, QStringList()));
+        }
+    }
+
+    if (dolphinServices.isEmpty()) {
+        return false;
+    }
+
+    QStringList newUrls;
+
+    // check to see if any instances already have any of the given URLs open
+    for (const QString& url : QUrl::toStringList(urls)) {
+        bool urlFound = false;
+        for (auto& service: dolphinServices) {
+            QDBusReply<bool> isUrlOpen = service.first->call(QStringLiteral("isUrlOpen"), url);
+            if (isUrlOpen.isValid() && isUrlOpen.value()) {
+                    service.second.append(url);
+                    urlFound = true;
+                    break;
+            }
+        }
+        if (!urlFound) {
+            newUrls.append(url);
+        }
+    }
+    dolphinServices.front().second << newUrls;
 
-    KRun::run(command, urls, window, qApp->applicationDisplayName(), qApp->windowIcon().name());
+    for (const auto& service: dolphinServices) {
+        if (!service.second.isEmpty()) {
+            service.first->call(openFiles ? QStringLiteral("openFiles") : QStringLiteral("openDirectories"), service.second, splitView);
+            service.first->call(QStringLiteral("activateWindow"));
+        }
+    }
+    return true;
 }

diff --git a/src/global.h b/src/global.h
index e3c4a1cca10ee508ef4d3fb05eed65f16250cb81..16305eafebbaec9a63b64b94ab21fbac155f4098 100644 (file)
--- a/src/global.h
+++ b/src/global.h
@@ -42,6 +42,12 @@ namespace Dolphin {
      */
     void openNewWindow(const QList<QUrl> &urls = {}, QWidget *window = nullptr, const OpenNewWindowFlags &flags = OpenNewWindowFlag::None);
 
+    /**
+     * Attaches URLs to an existing Dolphin instance if possible.
+     * Returns true if URLs were successfully attached
+     */
+    bool attachToExistingInstance(const QList<QUrl>& urls, bool openFiles, bool splitView, const QString& preferredService = QString());
+
     /**
      * TODO: Move this somewhere global to all KDE apps, not just Dolphin
      */

diff --git a/src/main.cpp b/src/main.cpp
index 08405d0073b963d6ea187bd8e1aaf705e4c51903..639dc32efa6694a8fdf00142935552200c0512e5 100644 (file)
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -34,6 +34,10 @@
 
 #include <QApplication>
 #include <QCommandLineParser>
+#include <QDBusConnection>
+#include <QDBusInterface>
+#include <QDBusAbstractInterface>
+#include <QDBusConnectionInterface>
 
 #ifndef Q_OS_WIN
 #include <unistd.h>
@@ -122,33 +126,42 @@ extern "C" Q_DECL_EXPORT int kdemain(int argc, char **argv)
     parser.addOption(QCommandLineOption(QStringList() << QStringLiteral("select"), i18nc("@info:shell", "The files and folders passed as arguments "
                                                                                         "will be selected.")));
     parser.addOption(QCommandLineOption(QStringList() << QStringLiteral("split"), i18nc("@info:shell", "Dolphin will get started with a split view.")));
+    parser.addOption(QCommandLineOption(QStringList() << QStringLiteral("new-window"), i18nc("@info:shell", "Dolphin will explicitly open in a new window.")));
     parser.addOption(QCommandLineOption(QStringList() << QStringLiteral("daemon"), i18nc("@info:shell", "Start Dolphin Daemon (only required for DBus Interface)")));
     parser.addPositionalArgument(QStringLiteral("+[Url]"), i18nc("@info:shell", "Document to open"));
 
     parser.process(app);
     aboutData.processCommandLine(&parser);
 
+    const bool splitView = parser.isSet(QStringLiteral("split")) || GeneralSettings::splitView();
+    const bool openFiles = parser.isSet(QStringLiteral("select"));
+    const QStringList args = parser.positionalArguments();
+    QList<QUrl> urls = Dolphin::validateUris(args);
+
     if (parser.isSet(QStringLiteral("daemon"))) {
         return app.exec();
     }
 
-    const QStringList args = parser.positionalArguments();
-    QList<QUrl> urls = Dolphin::validateUris(args);
-
     if (urls.isEmpty()) {
         // We need at least one URL to open Dolphin
         urls.append(Dolphin::homeUrl());
     }
 
-    const bool splitView = parser.isSet(QStringLiteral("split")) || GeneralSettings::splitView();
     if (splitView && urls.size() < 2) {
         // Split view does only make sense if we have at least 2 URLs
         urls.append(urls.last());
     }
 
+    if (!parser.isSet(QStringLiteral("new-window"))) {
+        if (Dolphin::attachToExistingInstance(urls, openFiles, splitView)) {
+            // Successfully attached to existing instance of Dolphin
+            return 0;
+        }
+    }
+
     DolphinMainWindow* mainWindow = new DolphinMainWindow();
 
-    if (parser.isSet(QStringLiteral("select"))) {
+    if (openFiles) {
         mainWindow->openFiles(urls, splitView);
     } else {
         mainWindow->openDirectories(urls, splitView);