Re: [Nautilus-list] adapter port

[Dave Camp <dave ximian com>]

On Fri, 2002-01-04 at 15:16, Darin Adler wrote:
> On 1/4/02 11:42 AM, "Dave Camp" <dave ximian com> wrote:
> 
> > This patch ports the adapter component, minus the embeddable and
> > progressive data sink strategies.  It also ports/re-enables the code in
> > nautilus-component-adapter-factory.c.
> 
> > -#include <bonobo/bonobo-generic-factory.h>
> > -#include <bonobo/bonobo-main.h>
> > +#include <bonobo.h>
> 
> Normally I prefer to include the minimum set of individual headers instead
> of catch-all headers like <bonobo.h>. Across the entire project, this speeds
> up compile times.

Fixed.

> 
> > +               (GWeakNotify)adapter_factory_object_weak_notify,
> 
> As it says in the Nautilus coding style guide, we try to avoid casting the
> type of function pointers. Is this cast really needed? Can't we just change
> the signature of adapter_factory_object_weak_notify instead?

Fixed.

> > +        g_error (_("I could not initialize Bonobo"));
> 
> Is this really what programs should write out if they fail to initialize?
> The message seems a little silly to me.

I dunno.  Users will very rarely see that (bonobo_ui_init shouldn't fail
often, and the component is started by bonobo-activation-server
anyway).  I changed it to "bonobo_ui_init() failed" to be a bit more
useful to developers.

> 
> > +#ifdef GNOME2_CONVERSION_COMPLETE
> >      g_free (registration_id);
> > +#endif
> 
> What's the issue here? Does something go wrong if we free the registration
> ID?

It looks like a g_free() here is ok.  However, I grabbed this from
nautilus_view_standard_main(), so this should be fixed there too.  Any
objections to me changing it there?

> 
> > +        g_print ("couldn't get stream object\n");
> > +        fprintf (stderr, "before load\n");
> > +        fprintf (stderr, "after load\n");
> > +            g_print ("couldn't load stream object: %s\n",
> bonobo_exception_get_text (&ev));
> > +            g_print ("couldn't unref stream object: %s\n",
> bonobo_exception_get_text (&ev));
> 
> Maybe these should be removed. If we want to keep some, they should be
> changed to g_message or g_warning.

rrgh, those shouldn't have stayed in.  They're gone.

> Otherwise, the code looks good. Please fix and commit.

Done.

-dave