Add Autostart KCM to kdebase/workspace/kcontrol

[Stephen Leaf]

On Monday 19 February 2007 12:17:19 pm Aaron J. Seigo wrote:
> On February 18, 2007, Stephen Leaf wrote:
> > I've finished my port to kde4
> > So first I'd like to ask is the above location Ok?
> > Or should this be in a different section?
>
> it should go into kdereview first for .. well, review =)
Will do

> > Also does anyone have a name suggestion? I'm horrible with names and I'm
> > told that autostart already exists and does about the same thing.
>
> it does?
>
> > And finally could I get a review of my code, suggestions, comments,
> > questions, concerns?
>
> a description of what it does and a url to a screenshot or two (so the
> usability folks who often don't have access to current trunk/ can look at
> it as well) would be helpful ...
http://images.smileaf.org/img:31e995e1cd49674370884b402e27700a

> you don't need to include a copy of the GPL or installation instructions if
> it goes into a main module. the doxyfile can go to if i'm not mistaken.
Removed

> i'm not sure the term "ENV" will mean much to anyone. a more user friendly
> string would be good there. essentially the difference between env/ and
> autostart/ is that env is pre-desktop loading and are actual scripts,
> whereas autostart/ is during desktop load and are .desktop files, correct?
> perhaps the name could reflect that difference.
Yah, I wasn't quite sure how to word it exactly. Just to verify the env/ can 
handle .desktop correct? If not then I have a special case to figure in. 
which breaks the ui design. Oh well it was ugly anyway =) need to get in 
touch with the usability folks.

> the UI is hand-coded; using designer is the preferred mechanism.
When I was adding kdmtheme into the KDM module ossi told me to lose the .ui 
So I figured that was preferred. But if its the otherway around I suppose I  
could separate it.

> why do you create your own local kstandarddirs in autostart::load()? you
> should be using componentData().dirs() no?
have not had a look at it, Will look into that.

> the for loop is a bit interesting as well =) wouldn't it be a bit cleaner
> to just do sth like:
>
> QStringList paths;
> paths << KGlobalSettings::autostartPath()
>          << ksd->localkdedir() + "/shutdown"
>          << ksd->localkdedir() + "/env";
>
> foreach (const QString& path, paths) {
>     // do the loading
> }
Quite cleaner, I'll make the change. Not use to doing foreach() in C++ yet.

> adding paths then becomes a matter of a new entry in the stringlist. or you
> could put the loading code in a method that takes the path as a parameter
> and call it three times =) *shrug* just looks a bit odd as it is...
>
> more importantly though, while env/ works this way, autostart/ uses the
> cascading config system. so autostart items in $PREFIX can be overridden in
> $KDEHOME, for instance. it would be nice to list all those items, really,
> both so users can turn off global autostarts as well as not accidently
> override them with something of the same name.
>
> so perhaps adding autostart and shutdown entries to the resource dirs in
> the kstandardirs object and using findAllResources would be more accurate?
> env/ would still need to be loaded as you are doing ...
Good idea. I'll see if I can get that in after I fix up the first 2 issues.
Would be very nice for this and possibly the loading of the 
autostart/shutdown.
And while I'm on this, wouldn't it make a bit more sense to rename autostart 
to startup ? startup/shutdown?
>
> 0.02 =)
-- 
Stephen Leaf