KAutoStart

[Frerich Raabe]

On Thursday 02 March 2006 21:02, Aaron J. Seigo wrote:
> on the xdg list there's been some discussion with issues surrounding
> autostart mechanisms.
>
> security issues related to apps being allowed to simply write into the
> autostart/ directory were raised on the one hand, and cross-desktop issues
> were raised on the other.
>
> it was suggested by thiago that in kde we should move away from
> X-KDE-autostart-condition since it's KDE specific and doesn't work very
> well in a not-all-KDE world. instead applications should simply add a
> .desktop file to the user's autostart directory with Hidden=true or
> Hidden=false
>
> to ensure that the latter happens i suggested we provide a utility class
> that app developers can use easily to set autostart on/off. this has the
> happy side effect of future proofing how this is handled by apps in case
> the methods used to accomplish this change, such as the addition of a
> command line tool to add/remove autostart .desktop files for security
> reasons or if we move away from .desktop files for this (though that's
> unlikely?)

A good idea IMHO.

> attached is an API for such a class for review. if it is accepted i'd like
> to add this for KDE4 as one of the "KDE component" classes rather than add
> it to kdecore and then later move it.

o) The first thing I noticed is the sample code:

 * Typical usage might look like:
 *
 * KAutoStart autostart; // without arg, gets app name from KAboutData
 * autostart.setAutoStart(config()->autoStart());
 *
 * config.setAutoStart(autostart.autoStarts());

I didn't understand that code fragment at all - I see 'autoStart' all over, 
but what does that actually do? If I have that code in my application, does 
that code make my application start automatically with each KDE session?

Also, if that is the common case, how about providing a simple static 
'KAutoStart::registerApplication()' convenience function which does the above 
(using KGlobal::config())?

o) A minor glitch: KAutoStart inherits QObject but has no Q_OBJECT macro (or 
is that out of fashion with Qt 4? :-)

o) First sentence of the ctor documentation is not clear to me:

"Creates a new KAutoStart object that represents the entryName service." - 
that transports little meaning to me. Is 'entryName' a path to a .desktop 
file?

o) The 'autoStarts' signature looks a bit odd:

[..]
         * Returns whether or not the service represented by entryName in the
         * autostart system is set to autostart at login or not
[..]
        bool autoStarts(const QString& environment = QString(),
                        bool checkStartCondition = false,
                        bool checkTryExec = false);

Given that it says 'Returns whether or not', shouldn't that function be const? 
Also, please use enums instead of two bools, because I anticipate that simple 
minds like mine will be confuzzled when they see something like this in a few 
months:

bool starts = autoStart.autoStarts( QString(), false, true );

o) 'environment' was misspelled as 'envrionment' a few times

o) You have the two concepts of 'allowed environments' and 'excluded 
environments'. For the latter, you have these functions:

1a) QStringList excludeEnvironments() const;
1b) void setExcludeEnvironments( const QStringList & );
1c) void addToExcludeEnvironments( const QString & );
1d) void removeFromExlcudedEnvironments( const QString & );

For the former, the naming is unfortunately a little less consistent:

2a) QStringList environments() const;
2b) void setEnvironments( const QStringList & );
2c) void addToOnlyShowIn( const QString & );
2d) void removeFromOnlyShowIn( const QString & );

I think 2c) and 2d) are especially strange. How renaming them to 
'addToEnvironments' and 'removeFromEnvironments'?

In general - what happens when an environment is neither in 'environments' nor 
in 'excludedEnvironments', is it allowed or not? If so (if the default policy 
is 'allow environment unless explicitely excluded) then maybe one should drop 
2*) and just maintain a list of forbidden environments.

o) You refer to the autostart specification like 'This maps to the XYZ feature 
in the freedesktop.org autostart specification' sometimes - please don't do 
that. IMHO that's an implementation detail.

o) A final minor nitpick: 'Private* d' ought to be 'Private* const d;' :-)

- Frerich