Show the KCM with the run commands open when a setup packet is received
ClosedPublic

Authored by apol on Mar 25 2018, 2:06 PM.

Details

Summary

Depends on D11683.

Test Plan

Tested the kcmshell kdeconnect --args mydeviceid:kdeconnect_runcommand works

Diff Detail

Repository
R224 KDE Connect
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
apol requested review of this revision.Mar 25 2018, 2:06 PM
apol created this revision.
nicolasfella added inline comments.
plugins/runcommand/runcommandplugin.cpp
72

Why not separate by space instead of ':'? Would simplify the logic above

Restricted Application added a project: KDE Connect. · View Herald TranscriptApr 14 2018, 9:14 PM
apol added inline comments.Apr 15 2018, 12:01 AM
plugins/runcommand/runcommandplugin.cpp
72

How would it be simpler? Look closer :p

nicolasfella added inline comments.Apr 15 2018, 12:04 AM
kcm/kcm.cpp
122

this would be args.first

123

And this args.second

nicolasfella added inline comments.Apr 15 2018, 12:14 AM
plugins/runcommand/runcommandplugin.cpp
72

Sorry, what I actually meant was passing them in two args instead of one. In bash that would be separating them by space, hence my confusing statement

apol added inline comments.Apr 17 2018, 11:41 AM
plugins/runcommand/runcommandplugin.cpp
72

Ah, we are only getting one string from the --args kcmshell argument, there's not a whole lot we can do there.

Looks good to me, but needs min Frameworks version bumped to 5.45

nicolasfella requested changes to this revision.Apr 18 2018, 9:41 PM

Please apply

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4d5d0229..6ad7b225 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -8,7 +8,7 @@ set(KDECONNECT_VERSION_PATCH 1)
 set(KDECONNECT_VERSION "${KDECONNECT_VERSION_MAJOR}.${KDECONNECT_VERSION_MINOR}.${KDECONNECT_VERSION_PATCH}")
 
 set(QT_MIN_VERSION "5.7.0")
-set(KF5_MIN_VERSION "5.42.0")
+set(KF5_MIN_VERSION "5.45.0")
 
 find_package(ECM ${KF5_MIN_VERSION} REQUIRED NO_MODULE)
 set(CMAKE_MODULE_PATH ${ECM_MODULE_PATH} ${ECM_KDE_MODULE_DIR} ${CMAKE_SOURCE_DIR}/cmake)
diff --git a/plugins/runcommand/runcommandplugin.cpp b/plugins/runcommand/runcommandplugin.cpp
index 41787a79..482501e4 100644
--- a/plugins/runcommand/runcommandplugin.cpp
+++ b/plugins/runcommand/runcommandplugin.cpp
@@ -69,7 +69,7 @@ bool RunCommandPlugin::receivePacket(const NetworkPacket& np)
         QProcess::startDetached(QStringLiteral("/bin/sh"), QStringList()<< QStringLiteral("-c") << commandJson[QStringLiteral("command")].toString());
         return true;
     } else if (np.has("setup")) {
-        QProcess::startDetached(QStringLiteral("kcmshell5"), {QStringLiteral("--args"), QString(device()->id() + QStringLiteral(":kdeconnect_runcommand")) });
+        QProcess::startDetached(QStringLiteral("kcmshell5"), {QStringLiteral("kdeconnect"), QStringLiteral("--args"), QString(device()->id() + QStringLiteral(":kdeconnect_runcommand")) });
     }
 
     return false;
@@ -85,6 +85,7 @@ void RunCommandPlugin::sendConfig()
 {
     QString commands = config()->get<QString>(QStringLiteral("commands"),QStringLiteral("{}"));
     NetworkPacket np(PACKET_TYPE_RUNCOMMAND, {{"commandList", commands}});
+    np.set<bool>(QStringLiteral("canAddCommand"), true);
     sendPacket(np);
 }
This revision now requires changes to proceed.Apr 18 2018, 9:41 PM
apol updated this revision to Diff 32526.Apr 19 2018, 12:31 AM

Included Nico's patch

I wonder if we should ifdef for older KF5 versions...

nicolasfella accepted this revision.Apr 19 2018, 12:40 AM

I thought about ifdef'ing too, but by the time the next update will hit distros Frameworks 5.45 will be available too. The only reason would be for self-building people on distros that won't ship 5.45 soon

This revision is now accepted and ready to land.Apr 19 2018, 12:40 AM
This revision was automatically updated to reflect the committed changes.
In D11684#249394, @apol wrote:

Included Nico's patch

I wonder if we should ifdef for older KF5 versions...

Please do. For example Frameworks 5.45 is too late for inclusion in Ubuntu 18.04, and will be too big an update to do a SRU (stable release upgrade) in the main ubuntu archive.

Without this ifdef'd, we will have to either hope you make bugfix releases (1.3.1 etc) without this change included, or cherry pick your fix commits to apply as patches to 1.3.0 which is not optimal if it can be avoided.