KAutoStart

[Aaron J. Seigo]

On Friday 03 March 2006 03:03, Frerich Raabe wrote:
>  On Thursday 02 March 2006 21:02, Aaron J. Seigo wrote:
>  o) The first thing I noticed is the sample code:

>  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?

i'll make the example clearer. it is indeed unclear, but made sense in my head 
when i wrote it ;) i suppose that's why we do these reviews

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

no, i just forgot it until i started working on the actual implementation. 
ditto for virtual_hook, etc...

>  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?

in our current implementation it refers to a .desktop file in form of: path + 
entryName + ".desktop", but i'm trying to abstract away the actual 
implementation details so don't go into that.

i'll see what i can do about making this clearer in the final revision

>  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? 

yes

> Also, please use enums instead of two bools, because I anticipate 

actually, one of those bools shouldn't be there since this is leading to the 
deprecation of X-KDE-start-condition. but yes, i've changed it to use an 
enumeration

>  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:
...
>  I think 2c) and 2d) are especially strange. How renaming them to
>  'addToEnvironments' and 'removeFromEnvironments'?

i missed those when renaming things... i've also renamed it to 
"allowedEnvironments" to match with "excludedEnvironments" and make it a 
little clearer

>  In general - what happens when an environment is neither in 'environments'
> nor in 'excludedEnvironments', is it allowed or not?

it is not allowed. if there is anything in 'environments' (OnlyShowIn) then 
the env (e.g. KDE) must be in that list.. i'll add that to the API docu.

> 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.

unfortunately that's not the case =(

i'm still not perfectly happy with the names for allowedEnvironments and 
excludedEnvironments ... perhaps someone has a better idea? in the .desktop 
files they are OnlyShowIn and NotShowIn, which is hardly better IMO.

>  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.

true. it doesn't map to other platforms at all. done.

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

heh.. fair enough.. see new version attached

-- 
Aaron J. Seigo
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

Full time KDE developer sponsored by Trolltech (http://www.trolltech.com)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kautostart.h
Type: text/x-c++hdr
Size: 8051 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20060303/95af9543/attachment.h>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20060303/95af9543/attachment.sig>