Re: GtkFileSystemGnomeVFS

[Owen Taylor <otaylor redhat com>]

On Mon, 2004-02-16 at 17:47, Alexander Larsson wrote:
> On Mon, 2004-02-16 at 16:34, Owen Taylor wrote:
> > On Mon, 2004-02-16 at 10:30, Alexander Larsson wrote:
> > 
> > > How about this patch? It changes the old "file-system" property into a
> > > string, and does named lookups for filesystem backends. It also adds
> > > gtk_file_chooser_dialog_new_with_backend and fixes an issue with the
> > > folder mode and volumes in gtkfilechooserdefault.
> > 
> > I'd like to see this done using GTypeModule, like themes, Pango modules,
> > input method modules, etc; with G_DEFINE_TYPE it shouldn't be more
> > than 30-40 extra lines of code.
> 
> Like this?

Generally, looks right. Detailed comments:

* "File System backend" property name should be "File System Backend"

* The gtkfilechooserdefault code for PROP_FILE_SYSTEM should be split
  out into a separate static function... it's too long to be inline  
  now.

* The gtkfilechooserdefault code for PROP_FILE_SYSTEM mixes
  if (file_system) with if (file_system == NULL), no fixed rule
  on this in GTK+ other than consistency in adjacent code
  but we generally use if (!file_system).

* gtk_file_chooser_dialog_new() and new_with_backend() IMO should
  be made wrappers around an internal function that takes a va_list,
  there's not a lot of duplicated code, but enough.

* Some typos in the new_with_backend() docs "especially useful if use"
  set_local_only should be set_local_only() to get the right markup.

Regards,
						Owen