runners: Take arguments into account when setting match id

runners/services/autotests/servicerunnertest.cpp

@@ -1,5 +1,5 @@
 /*
- *   Copyright (C) 2016-2020 Harald Sitter <sitter@kde.org>
+ *   Copyright (C) 2016-2021 Harald Sitter <sitter@kde.org>
  *
  *   This library is free software; you can redistribute it and/or
  *   modify it under the terms of the GNU Lesser General Public
@@ -47,6 +47,7 @@ private Q_SLOTS:
     void testSystemSettings();
     void testForeignAppsOutscoreKCMs();
     void testINotifyUsage();
+    void testKCMId();
 };
 
 void ServiceRunnerTest::initTestCase()
@@ -239,6 +240,21 @@ void ServiceRunnerTest::testINotifyUsage()
     QVERIFY(inotifyCountCool);
 }
 
+void ServiceRunnerTest::testKCMId()
+{
+    ServiceRunner runner(this, KPluginMetaData(), QVariantList());
+    Plasma::RunnerContext context;
+    context.setQuery(QStringLiteral("kcm_kwin_virtualdesktops"));
+
+    runner.match(context);
+
+    const auto matches = context.matches();
+    QCOMPARE(matches.count(), 1);
+    // Ensure KCM ids are uniquely identifying the KCM, not just the host application.
+    // The specific format of the id is a bit besides the point so we'll only check for our kcm being mentioned.
+    QVERIFY(matches.at(0).id().contains("kcm_kwin_virtualdesktops"));
+}
+
 QTEST_MAIN(ServiceRunnerTest)
 
 #include "servicerunnertest.moc"

runners/services/servicerunner.cpp

@@ -184,8 +184,8 @@ private:
             const static QRegularExpression snapCleanupRegex(QStringLiteral("env BAMF_DESKTOP_FILE_HINT=.+ "));
             exec.remove(snapCleanupRegex);
         }
-        match.setId(QStringLiteral("exec://") + KIO::DesktopExecParser::executableName(exec));
-
+        const QStringList resultingArgs = KIO::DesktopExecParser(KService(QString(), exec, QString()), {}).resultingArguments();
+        match.setId(QStringLiteral("exec://") + resultingArgs.join(QLatin1Char(' ')));
         if (!service->genericName().isEmpty() && service->genericName() != name) {
             match.setSubtext(service->genericName());
         } else if (!service->comment().isEmpty()) {

runners/shell/autotests/shellrunnertest.cpp

@@ -83,41 +83,43 @@ void ShellRunnerTest::testShellrunnerQueries_data()
     QTest::newRow("Should bot show result for non-existent path")
         << 0 << "/bin/trueeeeeee" << QString() << QStringList{};
     QTest::newRow("Should show result for executable name")
-        << 1 << "true" << "true" << QStringList{};
+        << 1 << "true" << executablePath << QStringList{};
     QTest::newRow("Should show result for executable name and args")
-        << 1 << "true --help" << "true --help" << QStringList{};
+        << 1 << "true --help" << executablePath + " --help" << QStringList{};
 
     QTest::newRow("Should show result for executable and ENV variables")
-        << 1 << "LC_ALL=C true" << "true" << QStringList{"LC_ALL=C"};
+        << 1 << "LC_ALL=C true" << executablePath << QStringList{"LC_ALL=C"};
     QTest::newRow("Should show result for executable + args and ENV variables")
-        << 1 << "LC_ALL=C true --help" << "true --help" << QStringList{"LC_ALL=C"};
+        << 1 << "LC_ALL=C true --help" << executablePath + " --help" << QStringList{"LC_ALL=C"};
     QTest::newRow("Should show result for executable and multiple ENV variables")
-        << 1 << "LC_ALL=C TEST=1 true" << "true" << QStringList{"LC_ALL=C", "TEST=1"};
+        << 1 << "LC_ALL=C TEST=1 true" << executablePath << QStringList{"LC_ALL=C", "TEST=1"};
     QTest::newRow("Should show no result for non-existent executable path and ENV variable")
         << 0 << "LC_ALL=C /bin/trueeeeeeeeeeee" << "" << QStringList{};
 
     // Some file we can access with a ~
     const QFileInfo testFile = createExecutableFile("test.sh");
-    const QString tildePath = KShell::tildeCollapse(testFile.absoluteFilePath());
+    const QString testFilePath = testFile.absoluteFilePath();
+    const QString tildePath = KShell::tildeCollapse(testFilePath);
 
     QTest::newRow("Should show result for full path with tilde")
-        << 1 << tildePath << KShell::quoteArg(tildePath) << QStringList{};
+        << 1 << tildePath << KShell::quoteArg(testFilePath) << QStringList{};
     QTest::newRow("Should show result for full path with tilde and envs")
-        << 1 << "LC_ALL=C " + tildePath << KShell::quoteArg(tildePath) << QStringList{"LC_ALL=C"};
+        << 1 << "LC_ALL=C " + tildePath << KShell::quoteArg(testFilePath) << QStringList{"LC_ALL=C"};
     QTest::newRow("Should show result for full path with tilde + args and envs")
-        << 1 << "LC_ALL=C " + tildePath + " --help" << KShell::quoteArg(tildePath) + " --help" << QStringList{"LC_ALL=C"};
+        << 1 << "LC_ALL=C " + tildePath + " --help" << KShell::quoteArg(testFilePath) + " --help" << QStringList{"LC_ALL=C"};
 
     // Some file we can access with a ~ and which has a space in its filename
     const QFileInfo testSpaceFile = createExecutableFile("test space.sh");
+    const QString testSpaceFilePath = testSpaceFile.absoluteFilePath();
     const QString tildeSpacePath = KShell::tildeCollapse(testSpaceFile.absoluteFilePath());
 
     QTest::newRow("Should show no result for full path with tilde and unquoted space")
             << 0 << tildeSpacePath << QString() << QStringList{};
     QTest::newRow("Should show result for full path with tilde and quoted space")
-            << 1 << KShell::quoteArg(tildeSpacePath) << KShell::quoteArg(tildeSpacePath) << QStringList{};
+            << 1 << KShell::quoteArg(tildeSpacePath) << KShell::quoteArg(testSpaceFilePath) << QStringList{};
     QTest::newRow("Should show result for full path with tilde, quoted space and args")
             << 1 << KShell::quoteArg(tildeSpacePath) + " --help"
-            << KShell::joinArgs({tildeSpacePath, "--help"}) << QStringList{};
+            << KShell::joinArgs({testSpaceFilePath, "--help"}) << QStringList{};
     // clang-format on
 }
 

runners/shell/shellrunner.cpp

@@ -59,7 +59,7 @@ void ShellRunner::match(Plasma::RunnerContext &context)
     if (parseShellCommand(context.query(), envs, command)) {
         const QString term = context.query();
         Plasma::QueryMatch match(this);
-        match.setId(QStringLiteral("exec://") + context.query());
+        match.setId(QStringLiteral("exec://") + command);
         match.setType(Plasma::QueryMatch::ExactMatch);
         match.setIcon(m_matchIcon);
         match.setText(i18n("Run %1", term));
@@ -88,8 +88,11 @@ bool ShellRunner::parseShellCommand(const QString &query, QStringList &envs, QSt
     const static QRegularExpression envRegex = QRegularExpression(QStringLiteral("^.+=.+$"));
     const QStringList split = KShell::splitArgs(query);
     for (const auto &entry : split) {
-        if (!QStandardPaths::findExecutable(KShell::tildeExpand(entry)).isEmpty()) {
-            command = KShell::joinArgs(split.mid(split.indexOf(entry)));
+        const QString executablePath = QStandardPaths::findExecutable(KShell::tildeExpand(entry));
+        if (!executablePath.isEmpty()) {
+            QStringList executableParts{executablePath};
+            executableParts << split.mid(split.indexOf(entry) + 1);
+            command = KShell::joinArgs(executableParts);
             return true;
         } else if (envRegex.match(entry).hasMatch()) {
             envs.append(entry);