make kcolorschemeditor desktop entry spec compliant and refine it
ClosedPublic

Authored by sitter on Aug 18 2016, 9:44 AM.

Details

Summary

quoting the menu spec [1]:

Additional Categories should always be used in combination
with one of the Main Categories.

So, following changes:

  • Settings is now the main category in the Categories entry
  • NoDisplay=true as we only use it from the KCM and it would only clutter the app menu otherwise
  • OnlyShowIn=KDE for good measure on top of nodisplay as the application has only value within a plasma context
  • Remove bogus MimeType entry. The Exec key takes no arguments, so the mimetype association is simply wrong.

[1] https://specifications.freedesktop.org/menu-spec/1.0/apa.html

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sitter updated this revision to Diff 6019.Aug 18 2016, 9:44 AM
sitter retitled this revision from to make kcolorschemeditor desktop entry spec compliant and refine it.
sitter updated this object.
sitter edited the test plan for this revision. (Show Details)
Restricted Application added a project: Plasma. · View Herald TranscriptAug 18 2016, 9:44 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
bshah accepted this revision.Aug 18 2016, 10:08 AM
bshah added a reviewer: bshah.
bshah added a subscriber: bshah.
bshah added inline comments.
kcms/colors/org.kde.kcolorschemeeditor.desktop
28 ↗(On Diff #6019)

Assuming systemsettings don't filter this one out.. "ship it".

This revision is now accepted and ready to land.Aug 18 2016, 10:08 AM
sitter added a subscriber: ochurlaud.

Pulling in @ochurlaud

The NoDisplay remark from Bhushan got me wondering how the KCM actually starts the app. And it turns out that it doesn't.

SchemeEditorDialog* dialog = new SchemeEditorDialog(path, this);
dialog->setModal(true);
dialog->show();

Which is now raising the question why kcolorschemeditor was made a standalone application to begin with seeing as it adds no value?

ochurlaud edited edge metadata.Aug 18 2016, 10:31 AM
In D2478#46347, @sitter wrote:

Pulling in @ochurlaud

The NoDisplay remark from Bhushan got me wondering how the KCM actually starts the app. And it turns out that it doesn't.

SchemeEditorDialog* dialog = new SchemeEditorDialog(path, this);
dialog->setModal(true);
dialog->show();

Which is now raising the question why kcolorschemeditor was made a standalone application to begin with seeing as it adds no value?

I would not put that in no-display but rather so that it goes in the tools or something similar

ochurlaud accepted this revision.Aug 18 2016, 10:38 AM
ochurlaud edited edge metadata.

After discussion on IRC: the nodisplay bit will be removed in a next iteration when the app will have more context.

sitter marked an inline comment as done.Aug 18 2016, 10:50 AM

Dump from IRC

[12:18] <sitter> ochurlaud: why is kcolorschemeeditor a standalone application if you don't use it as such?
[12:25] <ochurlaud> sitter: It can be used at such
[12:26] <ochurlaud> but you might want to access it from the colors kcm, and until now, i didn't find a way to reload the kcm after creating a new scheme
[12:27] <ochurlaud> maybe i would need to send a DBus signal or something...
[12:27] <sitter> ochurlaud: so the plan is to not use SchemeEditorDialog directly but run the standalone application from the KCM?
[12:28] <ochurlaud> sitter, if it is possible it is the plan
[12:28] <ochurlaud> the idea is also that notmart is creating a theme creation tool
[12:28] <ochurlaud> and a color scheme creation tool would be needed: it should finally move to plasma-sdk or something
[12:29] <sitter> ah yes, cool then
[12:30] <sitter> ochurlaud: also, yeah you'll probably need to send a dbus signal to the editor to have it load a different scheme
[12:30] <ochurlaud> sitter: well but don't expect it too early... :S
[12:31] <sitter> ochurlaud: no no. I was just perplexed by there being an application but the application not actually being used by the KCM :)
[12:36] <ochurlaud> sitter: see Thomas comments on https://phabricator.kde.org/D2409 about the context
[12:36] <sKreamer> Diff 2409 "Update Color KCM" [Closed] https://phabricator.kde.org/D2409
[12:36] <ochurlaud> It was the first iteration. Now that we have a standalone app, we can play with it so that it becomes better without breaking the kcm
[12:37] <ochurlaud> sitter: ok, then I accept your change, and we'll undo the nodisplay (maybe) in next iteration
[12:37] <sitter> ochurlaud: right now I think nodisplay is the way to go. That being said, I think even with it being a proper application it would be weird to have it in the menu. Even if it doesn't look like a KCM anymore, it still is effectively a settings app, which we all bundle into systemsettings
[12:38] <sitter> but yeah, that's a discussion to be had at a later time

This revision was automatically updated to reflect the committed changes.