[dolphin] /: Open externally called files/directories in new tabs

Elvis Angelaccio null at kde.org
Thu May 30 21:45:33 BST 2019


Git commit 27e3907a3daf9a63d05c00a0ff746de6cfdf2bdf by Elvis Angelaccio, on behalf of Alexander Saoutkin.
Committed on 30/05/2019 at 20:40.
Pushed by elvisangelaccio into branch 'master'.

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

M  +1    -0    CMakeLists.txt
M  +1    -0    src/CMakeLists.txt
M  +11   -2    src/dbusinterface.cpp
M  +27   -0    src/dolphinmainwindow.cpp
M  +33   -0    src/dolphinmainwindow.h
M  +22   -2    src/dolphintabwidget.cpp
M  +6    -0    src/dolphintabwidget.h
M  +82   -2    src/global.cpp
M  +6    -0    src/global.h
M  +18   -5    src/main.cpp

https://commits.kde.org/dolphin/27e3907a3daf9a63d05c00a0ff746de6cfdf2bdf

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7816b5717e..2feef8e28f 100644
--- 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 f1b7534aea..8a025e5843 100644
--- 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 c780bc7cdc..4e24354abe 100644
--- 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 b57ed4fc48..385b61a2a7 100644
--- 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 1e2460768b..4037c14689 100644
--- 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 e54ab5ada8..defd089c16 100644
--- 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 7e3b30c0df..3e8301725c 100644
--- 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 aaeb581206..e7ff67d776 100644
--- 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 e3c4a1cca1..16305eafeb 100644
--- 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 08405d0073..639dc32efa 100644
--- 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);


More information about the kde-doc-english mailing list