[Nautilus-list] Re: RH merge

[Darin Adler <darin bentspoon com>]

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.

What should we do about people who already have scripts in ~/Nautilus?

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

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

> @@ -902,23 +902,8 @@ default_default_folder_viewer_callback (
> static gpointer
> default_home_location_callback (int user_level)
> {
> -    char *default_home_location;
> -    char *user_main_directory;
> -
> g_return_val_if_fail (eel_preferences_user_level_is_valid (user_level), NULL);
> -
> -    if (user_level == EEL_USER_LEVEL_NOVICE) {
> -        user_main_directory = nautilus_get_user_main_directory ();
> -        default_home_location = gnome_vfs_get_uri_from_local_path
> (user_main_directory);
> -        g_free (user_main_directory);
> -        return default_home_location;
> -    }
> -
> -    if (user_level == EEL_USER_LEVEL_INTERMEDIATE) {
> -        return gnome_vfs_get_uri_from_local_path (g_get_home_dir ());
> -    }
> -    
> -    return NULL;
> +    return gnome_vfs_get_uri_from_local_path (g_get_home_dir ());
> }

I'd like to see further cleanup. If we are removing the Novice home
directory feature, there's a lot more code that can be removed rather than
just doing it at this level.

But I guess that's not a reason to reject the patch. Just work I'll have to
do after it's committed.

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

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

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

    -- Darin