[Nautilus-list] Re: RH merge

[Alex Larsson <alexl redhat com>]

On Wed, 5 Sep 2001, Darin Adler wrote:

> Looks good, please commit after reading my comments.
> 
> Have you compiled these changes with the default Nautilus flags (including
> -Werror)? I ask because I see some unused stuff here that would probably
> lead to warnings.
Strange. I do compile with -Werror.

> What should we do about people who already have scripts in ~/Nautilus?
Maybe we should look if ~/Nautilus/scripts contains any files, and put up 
a dialog asking the user to move them.
 
> > Removes ~/Nautilus, disables first time druid, don't fight with kdesktop,
> > add start-here, move scripts to ~/.gnome/nautilus-scripts.
> 
> I think we should probably remove the first time druid instead of just
> disabling it. I'm not one for keeping around dead code in the repository.

I agree.
 
> > @@ -511,7 +511,7 @@ static const PreferenceDefault preferenc
> > { NAUTILUS_PREFERENCES_TREE_SHOW_ONLY_DIRECTORIES,
> >  PREFERENCE_BOOLEAN,
> >  EEL_USER_LEVEL_INTERMEDIATE,
> > -      { EEL_USER_LEVEL_NOVICE, GINT_TO_POINTER (FALSE) },
> > +      { EEL_USER_LEVEL_NOVICE, GINT_TO_POINTER (TRUE) },
> >  { USER_LEVEL_NONE }
> > }
> 
> This one might be controversial. I seem to recall Maciej disputing this
> change to the preference default.

Ok. Reverted. We can discuss it later on nautilus-list.
 
> > static void
> > finish_startup (NautilusApplication *application)
> > {
> > @@ -467,8 +490,12 @@ nautilus_application_startup (NautilusAp
> > 
> > /* Run the first time startup druid if needed. */
> > if (do_first_time_druid_check && need_to_show_first_time_druid ()) {
> > -        nautilus_first_time_druid_show (application, urls);
> > -        return;
> > +        /* Do this at idle time, once nautilus has initialized
> > +         * itself. Otherwise we may spawn a second nautilus
> > +         * process when looking for a metadata factory..
> > +         */
> > +        g_idle_add (create_starthere_link_callback, NULL);
> 
> This hack is weak. There's probably a better way to do this at the right
> time than the idle trick. But I guess it's no so important to do better.

I bet there is a better place, although i don't know the nautilus startup 
sequence at all.
 
> > @@ -974,4 +1005,133 @@ init_session (void)
> > update_session, client);
> > 
> > update_session (client);
> > +}
> > +
> > +/* Keep these out of the way of the rest of the file */
> > +#include <gdk/gdkx.h>
> > +#include <X11/Xlib.h>
> 
> This goes against Nautilus coding style. If we really need these functions
> and it's important to keep the headers away from the rest of the file, then
> they should go in a separate file of their own, perhaps even in eel.

Ok. I moved them to the top. It doesn't really matter anyway. Nothing 
should collide with the X headers.
 
> > +static gboolean
> > +check_for_kdesktop (void)
> > +{
> > +    /* FIXME this is a pretty lame hack, should be replaced
> > +     * eventually with e.g. a requirement that desktop managers
> > +     * support a manager selection, ICCCM sec 2.8
> > +     */
> > +
> > +    return look_for_kdesktop_recursive (GDK_ROOT_WINDOW ());
> > }
> 
> Normally we like to name functions with boolean results something that makes
> it clear what the result means. Like "is_kdesktop_present" or something like
> that.

Renamed.

Ok. Commiting.

/ Alex