Re: gEDA-dev: [PATCH] Modified toolbar to use GtkIconFactory foricons.

[ehennes-at-NOSPAMsbcglobal_dot_net]

Thanks for the feedback.  These are all good changes, and I can put  
them in.

On Jul 28, 2008, at 9:24 AM, Patrick Bernaud wrote:
>  - if you call x_icons_init() from x_window_setup() you are going to
>    add an icon factory every time a new window is created, most of
>    the time (if bitmap directory is unchanged) only duplicating
>    it. Do we really need one icon factory per window? The code makes
>    the change of the bitmap directory from a configuration file
>    loaded after the system config file but I do not think this is
>    desirable and ultimately should be changed. What are other people
>    thinking about that?

There should only be one icon factory per application.  However,  
x_icons_init() has an initialization dependency on i_vars_set(), or  
more specifically, i_vars_libgeda_set().  To fix this problem, would  
it be logical to change the initialization function signature to  
x_icons_init(const gchar* bitmap directory). Then, move the function  
call out of x_window_setup() and into main_prog(), place immediately  
after x_window_set_default_icon() such that:?

x_icons_init(default_bitmap_directory);

This change would fix the one to many problem and the initialization  
dependency.

Which brings up another potential refactor.  The  
x_window_set_default_icon() function might need to move to x_icons.c?

Thanks,
Ed


_______________________________________________
geda-dev mailing list
geda-dev@moria.seul.org
http://www.seul.org/cgi-bin/mailman/listinfo/geda-dev