Bug 369239 - RunCommandPlugin: sort commands by name
Summary: RunCommandPlugin: sort commands by name
Status: RESOLVED FIXED
Alias: None
Product: kdeconnect
Classification: Applications
Component: android-application (show other bugs)
Version: 1.4
Platform: Android Android 5.x
: NOR wishlist
Target Milestone: ---
Assignee: Albert Vaca Cintora
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-23 12:26 UTC by Thomas Posch
Modified: 2016-09-30 10:56 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
sort commands by name patch (6.04 KB, patch)
2016-09-23 12:28 UTC, Thomas Posch
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Posch 2016-09-23 12:26:57 UTC
Commands are sorted by their random UUID, which makes them hard to find when several commands are defined.

Attached patch sorts them by name.

Reproducible: Always
Comment 1 Thomas Posch 2016-09-23 12:28:15 UTC
Created attachment 101241 [details]
sort commands by name patch
Comment 2 Albert Vaca Cintora 2016-09-24 18:12:31 UTC
I think they are shown in the order you define them in the desktop (which shouldn't be based on the uuid). If we want to sort them, shouldn't we be sorting them on the desktop side so the ordering is the same on both sides?
Comment 3 Thomas Posch 2016-09-25 10:10:19 UTC
They are not. key{name,command} entries are stored/transmitted in an JSON object, which is unordered by definition [1]. The ordering by key probably is an implementation detail of the used JSON library. I.e. restoring commands from JSON orders them by key.

Same ordering on both sides seems like a good idea. This would also allow for custom ordering (without having to resort to some common prefix). A protocol change would be required however. I see two approaches:
* Add a sortIndex field and sort on both sides. {key:{name,command,idx},...}
* Send by Array. [{key,name,command},...]

I prefer the second, since this is how you express ordered data in JSON and I expect it to lead to a simpler implementation. What is your opinion?

[1] json.org
Comment 4 Albert Vaca Cintora 2016-09-28 15:24:49 UTC
Changing the protocol should be done in a backwards compatible way, and I don't think the benefit is worth the hassle. Let's use this patch for now, so at least we have the commands sorted on the side you use them.

If you send new patches, it would be good if you use http://reviewboard.kde.org instead, as it's easier to discuss code changes there :)

PS: I'll try to spell your name correctly this time!
Comment 5 Albert Vaca Cintora 2016-09-29 10:57:38 UTC
Git commit 5320c3c226b7d09c4f7cd043508364e053f0b899 by Albert Vaca, on behalf of Thomas Posch.
Committed on 29/09/2016 at 10:57.
Pushed by albertvaka into branch '1.x'.

Make commands sorted by name

A  +40   -0    src/org/kde/kdeconnect/Plugins/RunCommandPlugin/CommandEntry.java
M  +17   -11   src/org/kde/kdeconnect/Plugins/RunCommandPlugin/RunCommandActivity.java
M  +2    -2    src/org/kde/kdeconnect/UserInterface/List/EntryItem.java

http://commits.kde.org/kdeconnect-android/5320c3c226b7d09c4f7cd043508364e053f0b899
Comment 6 Albert Vaca Cintora 2016-09-29 10:58:15 UTC
Another (easier) solution that wouldn't require changing the protocol would be to display them also in alphabetical order in the desktop plugin settings. This would work regardless of how they are stored and sent internally.
Comment 7 Albert Vaca Cintora 2016-09-30 10:56:33 UTC
Git commit 91cf466ee2f04daad20ab4f4a474a92fdb0b1e38 by Albert Vaca, on behalf of Thomas Posch.
Committed on 30/09/2016 at 10:56.
Pushed by albertvaka into branch '1.x'.

Sort commands by name, as we do on Android
REVIEW: 129076

M  +3    -0    plugins/runcommand/runcommand_config.cpp

http://commits.kde.org/kdeconnect-kde/91cf466ee2f04daad20ab4f4a474a92fdb0b1e38