D28421: Add a new standalone executable to replace X-KDE-Autostart-Condition

[David Edmundson]

davidedmundson added a comment.


  Code is fine; both the change in KAutostart and the helper.
  
  I'm not sure if this the helper should be in the framework.
  
  X-KDE-Autostart condition is very tied to Plasma doing the booting, at which point we're better off putting the binary in plasma-workspace/startkde somewhere.

INLINE COMMENTS

> kde-systemd-start-condition.cpp:17
> +    if (!parser.positionalArguments().count()) {
> +        parser.showHelp(0);
> +    }

Should it be  showHelp(255)?

Not that we should end up here in the generated case.

> kde-systemd-start-condition.cpp:20
> +
> +    if (KAutostart::isStartConditionMet(parser.positionalArguments().at(0))) {
> +        return 0;

A group name can have a space in it.

For example:
X-KDE-autostart-condition=baloofilerc:Basic Settings:Indexing-Enabled:true

I assume this will be ok as long as we make sure that we make the generator put things in quotes. But can you double check.

REPOSITORY
  R309 KService

REVISION DETAIL
  https://phabricator.kde.org/D28421

To: hchain, davidedmundson, broulik, dfaure, kossebau
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200330/7144f9f6/attachment.html>