Re: [Nautilus-list] More merge

[Darin Adler <darin bentspoon com>]

on 9/5/01 2:51 PM, Alex Larsson at alexl redhat com wrote:

> +    return volume && nautilus_volume_is_read_only (volume);

Nautilus coding style would use volume != NULL here.

> +        if (volume->device == device)
> +            return volume;

Nautilus coding style would use {} around the return statement.

> +    /* Returning NULL from this function oddly enough keeps the current
> +     * mount list. Go figure.
> +     */

I would normally deal with this by renaming get_mount_list and adding a
comment at the top of the function rather than adding a comment down here.

> +    if (sb.st_mtime <= last_mtime) {
> +        return NULL;
> +    }

Does this handle rollover of mtime? Does it need to? Can we just use ==
instead of <=?

> +    if (stat (volume->mount_path, &statbuf) == 0)
> +        volume->device = statbuf.st_dev;

Nautilus coding style would use {} around the body of the if.

> +set_gdk_window_background (GdkWindow *window,
> +               gboolean   have_pixel,
> +               Pixmap     pixmap,
> +               gulong     pixel)
> {
> -    NautilusDesktopWindow *window;
> +    Window w;
> +
> +    w = GDK_WINDOW_XWINDOW (window);
> +
> +    if (pixmap != None)
> +        XSetWindowBackgroundPixmap (GDK_DISPLAY (), w,
> +                        pixmap);
> +    else if (have_pixel)
> +        XSetWindowBackground (GDK_DISPLAY (), w,
> +                      pixel);
> +    else
> +        XSetWindowBackgroundPixmap (GDK_DISPLAY (), w,
> +                        None);
> +}

Nautilus coding style would use {} around the body of the if and the else.

> +    if (GTK_IS_WINDOW (widget))
> +        gtk_widget_set_app_paintable (widget, TRUE);

Nautilus coding style would use {} around the body of the if.

I wish we didn't have a "Nautilus coding style". We should fix that soon so
that it matches at least one other major gnome component. I see no reason
for it to be different from what's used in every other project.


> +    /* Recurse to child widget (assumes we already chained up and
> +     * realized child widget)
> +     */
> +    if (GTK_IS_BIN (widget) &&
> +        GTK_BIN (widget)->child) {
> +        /* Ensure we're realized */
> +        gtk_widget_realize (GTK_BIN (widget)->child);
> +        
> +        set_window_background (GTK_BIN (widget)->child,
> +                       already_have_root_bg, have_pixel,
> +                       pixmap, pixel);
> +    }

Comment doesn't match code here. Comment says we assume that the child is
realized. Code calls gtk_widget_realize on the child rather than asserting
it. And the caller calls this before chaining up the map call.

> +    /* FIXME the following FIXME is bogus, we aren't mapped yet. */
> /* FIXME bugzilla.eazel.com 1253:
> * Looking at the gnome_win_hints implementation,
> * it looks like you can call these with an unmapped window,

I'd prefer a fix to the FIXME, rather than a FIXME complaining about the
FIXME.

> -    if (!position_good) {
> -        if (nautilus_file_is_local (file) && nautilus_file_is_in_desktop
(file)) {
> -            uri = nautilus_file_get_uri (file);
> -            path = gnome_vfs_get_local_path_from_uri (uri);
> -
> -            if (path != NULL) {
> -                res = gnome_metadata_get (path, "icon-position", &size,
&buf);
> -                if (res == 0) {
> -                    if (sscanf (buf, "%d%d", &position->x, &position->y) ==
2) {
> -                        position_good = TRUE;
> -                    }
> -                    g_free (buf);
> -                }
> -            }
> -            g_free (path);
> -            g_free (uri);
> -        }
> -    }
> -    

We should also remove the code that tries to set the icon-position metadata,
then, and the includes to get at the metadata calls, too.

    -- Darin