Re: [Nautilus-list] continued redhat merge

[Alex Larsson <alexl redhat com>]

On Wed, 19 Sep 2001, Darin Adler wrote:

> My comments:
> > +static GnomeVFSResult
> > +write_all (GnomeVFSHandle *handle,
> > +          gconstpointer buffer,
> > +          GnomeVFSFileSize bytes)
> > +{
> > +       GnomeVFSFileSize bytes_written;
> > +       GnomeVFSResult result;
> > +
> > +       while (bytes > 0) {
> > +               result = gnome_vfs_write (handle, buffer,
> > +                                         bytes, &bytes_written);
> > +               if (result != GNOME_VFS_OK) {
> > +                       return result;
> > +               }
> > +               bytes -= bytes_written;
> > +       }
> > +
> > +       return GNOME_VFS_OK;
> > +}
> 
> First of all, this function isn't really needed. I'm not sure why
> gnome_vfs_write returns the count of bytes written, but it always writes all
> the bytes you ask it to. I can understand if we don't want to code depending
> on this, but I think it's a design flaw in gnome-vfs, putting undue burden
> on the caller.
> 
> But if we decide this write_all function is needed, then we must fix it
> because this writes the start of the buffer over and over again, reusing the
> buffer pointer without moving it through the buffer.
> 
> We should really fix this in the gnome-vfs API.

Hmm. Since it had the bytes_written i just naturally assumed unix-style 
semantics. I'll remove this function then.

I guess you might want to see how many bytes were written before you got 
an error though. So the API might not be broken.

I'm unsure about the usefullness of that though. What are you gonna do 
with the information? Try to write the rest again?


> I got sick of the minor coding style issues and just skipped most of them;
> there are many places where variables are declared in blocks, which is
> forbidden in the nautilus coding style. I think it's OK for now.
> 
> nautilus-desktop-file.c is missing an #include <config.h> at the top.
> 
> There are lots of if statements in nautilus-desktop-file.c with one-line
> bodies without the braces that Nautilus coding style requires.

Hmmm. nautilus-desktop-file.[ch] has beed deleted. They are not used 
anymore.

> In nautilus_desktop_file_get_icon, it checks the first character of
> icon_name for '/' before calling gnome_vfs_get_uri_from_local_path. This is
> unnecessary, since that function is defined to return NULL for any strings
> without a leading '/'.
This is in nautilus-desktop-file.c too.

> I'd like to address the FIXMEs about missing operations for .desktop files
> in nautilus-link.c soon, by either removing the operation entirely, or
> implementing it for .desktop files. My guess is that if we can get away with
> having a function unimplemented for .desktop files, we can eliminate it
> entirely, but we want to visit all the callers to be sure by removing the
> declaration from the header and recompiling.

set_type() is unused, but set_uri seems to be called from 
fm-desktop-icon-view.c:update_link_and_delete_copies().
I guess we have to implement it.

I'm working on a better desktop file parser/validator/editor right now.

/ Alex