Re: Problem with popup menu cache system

[Alexander Larsson <alexl redhat com>]

On Thu, 2006-06-15 at 11:45 +0200, Frederic Ruaudel wrote:
> Hi again,
> 
>   Here is my first patch proposal to fix Bug #339273. I would like to 
> have your comments on it and ideally your approval :o)
> 
> Sorry, but I didn't manage to compile a working Gnome 2.15.x 
> environnement, so I did this patch against the last stable release of 
> nautilus on FC5 : 2.14.1. If needed I can try to adapt it to the last 
> CVS Head version.
> 
> All the details are in the Bugzilla entry here :
> 
> http://bugzilla.gnome.org/show_bug.cgi?id=339273

Some comments on the patch:

+
+	/* Signals */
+	void (*items_updated)          (NautilusMenuProvider *provider, 
+					GtkWidget            *window, 
+					gpointer             *data);

This adds a member to an interface implemented by others, which is a
binary incompatible change. Fortunately its not needed, as the
implementations have no need for a default handler for the signal, they
are the ones that omit it anyway.

Also, i see no need to pass in the window. Keeping track of that should
not be needed by the extensions.

 static void
+menu_provider_items_updated_handler (NautilusMenuProvider *provider, GtkWidget* parent_window, gpointer data)

I don't like this at all. You're hardcoding lots of knowledge about the
implementation of windows and views in a general place. A better
approach would be to create a new signal "menu_extensions_changed" in
NautilusSignaller and have the windows and views listen to that signal
and re-read their extension menus when its emitted.

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's an impetuous hunchbacked filmmaker on a mission from God. She's an 
artistic extravagent bounty hunter with a song in her heart and a spring in 
her step. They fight crime!