GNOME.org
 

Re: [Nautilus-list] [patch] - hierarchical script menus - still incomplete

Well I really should be doing something else right now, but Darin's
reponse was thoughtful enough that I think he deserves a quicker
reply...  :)

On 28 May 2001 10:19:25 -0700, Darin Adler wrote:
> Overall the patch is looking good. Here are some specific comments.
> 
> ----------------
> 
> > 1) if script directories are moved out of
> > the top level script directory (or deleted) then the associated
> > callbacks should be removed and the directory object dereferenced.
> > ...
> > I have put a FIXME in the code to show what needs to be done to fix 1).
> 
> I think the current code will already do the right thing, and no 
> additional work is needed here except for some testing. If a directory is 
> moved or deleted or even renamed, the "files_changed" signal for its 
> parent directory will be called. This will eventually cause 
> reset_scripts_menu to run, which rebuilds the entire scripts menu.

The menus will be rebuilt just fine, but the list,
scripts_directory_list, may contain extranious entries until nautilus
shuts down.  In addition we won't know to stop monitoring the old script
directories, unref them, etc.  In fact you can see Bonobo-WARNING's if
you create a couple of sub-directories and then delete them.  This
happens because the function call_when_ready_on_scripts_directory looks
at the scripts_directory_list when it setups the callbacks, so if you
delete a directory without removing it from the list then you get
extranious callbacks.  It's really not a hugh deal (unless someone
creates 10 million script directories), but if there is an easy fix I
think we should take it..

> > 2) I need to set the max menu depth to deal with recursive sym-links.
> > ...
> > 2 should also be easy enough, but I currently don't know how to do it.
> 
> A simple way to set the maximum menu depth would be to add code to 
> add_directory_to_scripts_directory_list that uses 
> view->details->scripts_directory_length and counts the number of "/" 
> characters. The call can then do nothing if there are too many levels to 
> the path name.
> 

Done.

> > +	gtk_signal_connect (GTK_OBJECT (directory),
> > +			    "files_added",
> > +			    scripts_added_or_changed_callback,
> > +			    view);
> > +
> > +	gtk_signal_connect (GTK_OBJECT (directory),
> > +			    "files_changed",
> > +			    scripts_added_or_changed_callback,
> > +			    view);
> 
> If we want to notice the directory being moved or destroyed, we also need 
> to connect to the "changed" handler of the directory's corresponding file.

I think this needs to be done to address problem 1).  When the "changed"
handler is called we clean up the scripts directory list....  I will
work on this for my next patch...

> > +	view->details->scripts_directory_list = NULL;
> 
> This line of code is unnecessary.

There are a few for loops, as well as an if statement in
call_when_ready_on_scripts_directory which depend on this.  I am using
this value to determine whether or not the scripts directory was
found/created.

> 
> > +	uri = nautilus_file_get_uri (file);
> > +	directory = nautilus_directory_get (uri);
> > +	g_free (uri);
> 
> Ideally we should add a nautilus_file_get_corresponding_directory call 
> instead of doing it by hand like this.

I created a function for this but I couldn't find a sensable place for
its declaration since nautilus-file.h doesn't know about the
NautilusDirectory type.  At any rate the new patch won't need it even
though I agree that this function should be intregrated into nautilus.
If you would like I will work on this latter.

Thanks alot for your detailed reply and sorry for so many style
violations on my part,

David

Here is the revised patch:


--- nautilus-1.0.3-orig/src/file-manager/fm-directory-view.c	Sat May 26 11:02:20 2001
+++ nautilus-1.0.3/src/file-manager/fm-directory-view.c	Mon May 28 13:36:37 2001
@@ -22,8 +22,9 @@
  *
  * Authors: Ettore Perazzoli,
  *          John Sullivan <sullivan eazel com>,
- *          Darin Adler <darin eazel com>
- *          Pavel Cisler <pavel eazel com>
+ *          Darin Adler <darin eazel com>,
+ *          Pavel Cisler <pavel eazel com>,
+ *          David Emory Watson <dwatson cs ucr edu>
  */
 
 #include <config.h>
@@ -135,6 +136,8 @@
 #define FM_DIRECTORY_VIEW_POPUP_PATH_OPEN_WITH				"/popups/selection/Open Placeholder/Open With"
 #define FM_DIRECTORY_VIEW_POPUP_PATH_SCRIPTS				"/popups/selection/Open Placeholder/Scripts"
 
+#define MAX_MENU_LEVELS 5
+
 enum {
 	ADD_FILE,
 	BEGIN_ADDING_FILES,
@@ -166,9 +169,15 @@
 	NautilusFile *directory_as_file;
 	BonoboUIComponent *ui;
 
-	NautilusDirectory *scripts_directory;
-	guint scripts_added_handler_id;
-	guint scripts_changed_handler_id;
+	/* The first link in this list is reserved for the top level scripts
+	 * directory.  This means that new links are always appended to the
+	 * list. In addition, lower level script  directories must always occur
+	 * after higher level ones so if you remove a directory, you must remove
+	 * its subdirectories as well.  This is done to preserve the ordering of
+	 * the callbacks when we rebuild the  script menus.
+	 */
+	GList *scripts_directory_list;
+	guint scripts_directory_length;
 
 	guint display_selection_idle_id;
 	guint update_menus_timeout_id;
@@ -1138,46 +1147,45 @@
 
 static void
 scripts_added_or_changed_callback (NautilusDirectory *directory,
-		        GList *files,
-		        gpointer callback_data)
+				   GList *files,
+				   gpointer callback_data)
 {
 	FMDirectoryView *view;
 
 	view = FM_DIRECTORY_VIEW (callback_data);
 
-	g_assert (directory == view->details->scripts_directory);
+	g_assert (g_list_find (view->details->scripts_directory_list, directory) != NULL);
 
 	view->details->scripts_invalid = TRUE;
 	schedule_update_menus (view);
 }
 
 static void
-connect_script_handlers (FMDirectoryView *view)
+connect_script_handlers (FMDirectoryView *view,
+			 NautilusDirectory *directory)
 {
-	if (view->details->scripts_directory == NULL) {
-		return;
-	}
+	g_assert(directory != NULL);
 
-	nautilus_directory_file_monitor_add (view->details->scripts_directory,
-					     &view->details->scripts_directory,
+	nautilus_directory_file_monitor_add (directory, &view->details->scripts_directory_list,
 					     FALSE, FALSE, NULL);
 
-    	view->details->scripts_added_handler_id = gtk_signal_connect
-		(GTK_OBJECT (view->details->scripts_directory),
-		 "files_added",
-		 scripts_added_or_changed_callback,
-		 view);
+	gtk_signal_connect (GTK_OBJECT (directory),
+			    "files_added",
+			    scripts_added_or_changed_callback,
+			    view);
 
-	view->details->scripts_changed_handler_id = gtk_signal_connect
-		(GTK_OBJECT (view->details->scripts_directory), 
-		 "files_changed",
-		 scripts_added_or_changed_callback,
-		 view);
+	gtk_signal_connect (GTK_OBJECT (directory), 
+			    "files_changed",
+			    scripts_added_or_changed_callback,
+			    view);
 }
 
 static void
 fm_directory_view_initialize (FMDirectoryView *view)
 {
+	NautilusDirectory *scripts_directory;
+	char *scripts_directory_uri;
+
 	view->details = g_new0 (FMDirectoryViewDetails, 1);
 
 	/* We need to have our own X window so that cut, copy, and
@@ -1196,8 +1204,19 @@
 
 	view->details->nautilus_view = nautilus_view_new (GTK_WIDGET (view));
 
-	view->details->scripts_directory = get_scripts_directory ();
-	connect_script_handlers (view);
+	view->details->scripts_directory_list = NULL;
+	scripts_directory = get_scripts_directory ();
+
+	if (scripts_directory != NULL) {
+		view->details->scripts_directory_list = g_list_append (view->details->scripts_directory_list,
+								       scripts_directory);
+
+		scripts_directory_uri = nautilus_directory_get_uri (scripts_directory);
+		view->details->scripts_directory_length = strlen (scripts_directory_uri);
+		g_free (scripts_directory_uri);
+
+		connect_script_handlers (view, scripts_directory);
+	}
 
 	view->details->zoomable = bonobo_zoomable_new ();
 	bonobo_zoomable_set_parameters_full (view->details->zoomable,
@@ -1297,6 +1316,7 @@
 fm_directory_view_destroy (GtkObject *object)
 {
 	FMDirectoryView *view;
+	GList *node;
 
 	view = FM_DIRECTORY_VIEW (object);
 
@@ -1315,7 +1335,12 @@
 	fm_directory_view_clear (view);
 
 	disconnect_script_handlers (view);
-	nautilus_directory_unref (view->details->scripts_directory);
+
+	for (node = view->details->scripts_directory_list; node != NULL; node = node->next) {
+		nautilus_directory_unref (node->data);
+	}
+
+	g_list_free(view->details->scripts_directory_list);
 
 	disconnect_model_handlers (view);
 	nautilus_directory_unref (view->details->model);
@@ -3176,7 +3201,24 @@
 		(ui, parent_path, index);	
 	bonobo_ui_component_add_verb_full (ui, verb_name, callback, callback_data, destroy_notify);	   
 	g_free (verb_name);
-}				 
+}
+
+/* FIXME: Allow the icon to be set. */
+static void
+add_menu (BonoboUIComponent *ui,
+	  const char *parent_path,
+	  const char *label)
+{
+	char *escaped_label;
+
+	escaped_label = eel_str_double_underscores (label);
+
+	nautilus_bonobo_add_submenu (ui,
+				     parent_path,
+				     escaped_label);
+
+	g_free (escaped_label);
+}
 
 static void
 add_application_to_bonobo_menu (FMDirectoryView *directory_view,
@@ -3521,12 +3563,14 @@
 	chdir (old_working_dir);		
 	g_free (old_working_dir);
 	g_free (quoted_path);
-}				    
+}
 
 static void
-add_script_to_menus (FMDirectoryView *directory_view,
-		     NautilusFile *file,
-		     int index)
+add_script_to_script_menus (FMDirectoryView *directory_view,
+			  NautilusFile *file,
+			  int index,
+			  const char *menu_path,
+			  const char *popup_path)
 {
 	ScriptLaunchParameters *launch_parameters;
 	char *tip;
@@ -3535,13 +3579,13 @@
 
 	name = nautilus_file_get_name (file);
 	tip = g_strdup_printf (_("Run \"%s\" on any selected items"), name);
-	
+
 	launch_parameters = script_launch_parameters_new (file, directory_view);
 	pixbuf = nautilus_icon_factory_get_pixbuf_for_file 
 		(file, NULL, NAUTILUS_ICON_SIZE_FOR_MENUS, TRUE);
 
- 	add_numbered_menu_item (directory_view->details->ui, 
-				FM_DIRECTORY_VIEW_MENU_PATH_SCRIPTS_PLACEHOLDER,
+	add_numbered_menu_item (directory_view->details->ui, 
+				menu_path,
 				name,
 				tip,
 				index,
@@ -3553,8 +3597,8 @@
 	/* Use same launch parameters and no DestroyNotify for popup item, which has same
 	 * lifetime as the item in the File menu in the menu bar.
 	 */
- 	add_numbered_menu_item (directory_view->details->ui, 
-				FM_DIRECTORY_VIEW_POPUP_PATH_SCRIPTS_PLACEHOLDER,
+	add_numbered_menu_item (directory_view->details->ui,
+				popup_path,
 				name,
 				tip,
 				index,
@@ -3569,17 +3613,109 @@
 }
 
 static void
+add_menu_to_script_menus (FMDirectoryView *directory_view,
+			  NautilusFile *file,
+			  const char *menu_path,
+			  const char *popup_path)
+{
+	ScriptLaunchParameters *launch_parameters;
+	char *tip;
+	char *name;
+	GdkPixbuf *pixbuf;
+
+	name = nautilus_file_get_name (file);
+	tip = g_strdup_printf (_("Run \"%s\" on any selected items"), name);
+
+	launch_parameters = script_launch_parameters_new (file, directory_view);
+	pixbuf = nautilus_icon_factory_get_pixbuf_for_file 
+		(file, NULL, NAUTILUS_ICON_SIZE_FOR_MENUS, TRUE);
+
+	add_menu (directory_view->details->ui, 
+		  menu_path,
+		  name);
+
+	add_menu (directory_view->details->ui,
+		  popup_path,
+		  name);
+
+	gdk_pixbuf_unref (pixbuf);
+	g_free (name);
+	g_free (tip);
+}
+
+static gboolean
+add_directory_to_scripts_directory_list (FMDirectoryView *view, NautilusFile *file)
+{
+	char *uri;
+	NautilusDirectory *directory;
+	int num_levels;
+	int i;
+
+	if (!nautilus_file_is_directory (file)) {
+		return FALSE;
+	}
+
+	uri = nautilus_file_get_uri (file);
+
+	num_levels = 0;
+	for (i = view->details->scripts_directory_length; uri[i] != '\0'; i++) {
+		if (uri[i] == '/') {
+			num_levels++;
+		}
+	}
+
+
+	if (num_levels > MAX_MENU_LEVELS) {
+		g_free (uri);
+		return FALSE;
+	}
+
+	directory = nautilus_directory_get (uri);
+	g_free (uri);
+
+	if (directory == NULL) {
+		return FALSE;
+	}
+
+	if (g_list_find(view->details->scripts_directory_list, directory) == NULL) {
+		connect_script_handlers(view, directory);
+		
+		view->details->scripts_directory_list = g_list_append (view->details->scripts_directory_list,
+								       directory);
+	} else {
+		nautilus_directory_unref(directory);
+	}
+
+	return TRUE;
+}
+
+static void
 reset_scripts_menu (FMDirectoryView *view, GList *all_files)
 {
 	GList *node;
 	NautilusFile *file;
 	int index;
 	gboolean any_scripts;
-	
-	nautilus_bonobo_remove_menu_items_and_commands
-		(view->details->ui, FM_DIRECTORY_VIEW_MENU_PATH_SCRIPTS_PLACEHOLDER);
-	nautilus_bonobo_remove_menu_items_and_commands 
-		(view->details->ui, FM_DIRECTORY_VIEW_POPUP_PATH_SCRIPTS_PLACEHOLDER);
+	NautilusDirectory *directory;
+	char *parent_path;
+	char *menu_path, *popup_path;
+
+	if (all_files == NULL) {
+		view->details->scripts_invalid = FALSE;
+		return;
+	}
+
+	parent_path = nautilus_file_get_parent_uri (all_files->data);
+
+	menu_path = g_strdup_printf ("%s%s", FM_DIRECTORY_VIEW_MENU_PATH_SCRIPTS_PLACEHOLDER,
+				     parent_path + view->details->scripts_directory_length);
+	popup_path = g_strdup_printf ("%s%s", FM_DIRECTORY_VIEW_POPUP_PATH_SCRIPTS_PLACEHOLDER,
+				      parent_path + view->details->scripts_directory_length);
+
+	g_free (parent_path);
+
+	nautilus_bonobo_remove_menu_items_and_commands (view->details->ui, menu_path);
+	nautilus_bonobo_remove_menu_items_and_commands (view->details->ui, popup_path);
 
 	all_files = nautilus_file_list_sort_by_name (all_files);
 
@@ -3588,11 +3724,19 @@
 		file = node->data;
 
 		if (file_is_launchable (file)) {
-			add_script_to_menus (view, file, index);
+			add_script_to_script_menus (view, file, index, menu_path, popup_path);
 			any_scripts = TRUE;
+		} else if (nautilus_file_is_directory (file)) {
+			if (add_directory_to_scripts_directory_list (view, file)) {
+				add_menu_to_script_menus (view, file, menu_path, popup_path);
+				any_scripts = TRUE;
+			}
 		}
 	}
 
+	g_free (popup_path);
+	g_free (menu_path);
+
 	nautilus_bonobo_set_hidden (view->details->ui, 
 				    FM_DIRECTORY_VIEW_MENU_PATH_SCRIPTS_SEPARATOR, 
 				    !any_scripts);
@@ -3613,7 +3757,7 @@
 	view = callback_data;
 
 	g_assert (FM_IS_DIRECTORY_VIEW (view));
-	g_assert (view->details->scripts_directory == directory);
+	g_assert (g_list_find (view->details->scripts_directory_list, directory) != NULL);
 
 	reset_scripts_menu (view, files);
 }
@@ -3623,23 +3767,30 @@
 				      NautilusDirectoryCallback scripts_directory_callback)
 {
 	GList *attributes;
+	GList *node;
 
-	if (view->details->scripts_directory == NULL) {
+	if (view->details->scripts_directory_list == NULL) {
 		return;
 	}
-
-	nautilus_directory_cancel_callback (view->details->scripts_directory,
-				    	    scripts_directory_callback,
-				    	    view);
+	
+	for (node = view->details->scripts_directory_list; node != NULL; node = node->next) {
+		nautilus_directory_cancel_callback (node->data,
+						    scripts_directory_callback,
+						    view);
+	}
 
 	/* Later we may want to add more attributes here to get icon, etc. */
 	attributes = nautilus_icon_factory_get_required_file_attributes ();
 	attributes = g_list_prepend (attributes, NAUTILUS_FILE_ATTRIBUTE_CAPABILITIES);
 	attributes = g_list_prepend (attributes, NAUTILUS_FILE_ATTRIBUTE_DIRECTORY_ITEM_COUNT);
-	nautilus_directory_call_when_ready (view->details->scripts_directory,
-					    attributes,
-					    scripts_directory_callback,
-					    view);
+
+	for (node = view->details->scripts_directory_list; node != NULL; node = node->next) {
+		nautilus_directory_call_when_ready (node->data,
+						    attributes,
+						    scripts_directory_callback,
+						    view);
+	}
+
 	g_list_free (attributes);
 }
 
@@ -3659,8 +3810,8 @@
 
 	view = FM_DIRECTORY_VIEW (callback_data);          
 
-	if (view->details->scripts_directory != NULL) {
-		uri = nautilus_directory_get_uri (view->details->scripts_directory);
+	if (view->details->scripts_directory_list != NULL) {
+		uri = nautilus_directory_get_uri (view->details->scripts_directory_list->data);
 		open_location (view, uri, RESPECT_PREFERENCE);
 		g_free (uri);
 
@@ -4892,9 +5043,17 @@
 }
 
 static void
-disconnect_scripts_directory_handler (FMDirectoryView *view, int *id)
+disconnect_scripts_directory_handler (FMDirectoryView *view, NautilusDirectory *directory)
 {
-	disconnect_handler (GTK_OBJECT (view->details->scripts_directory), id);
+	gtk_signal_disconnect_by_func (GTK_OBJECT (directory),
+				       scripts_added_or_changed_callback,
+				       directory);
+
+	nautilus_directory_file_monitor_remove (directory, &view->details->scripts_directory_list);
+	
+	nautilus_directory_cancel_callback (directory,
+					    reset_scripts_menu_callback,
+					    view);
 }
 
 static void
@@ -4929,19 +5088,11 @@
 static void
 disconnect_script_handlers (FMDirectoryView *view)
 {
-	if (view->details->scripts_directory == NULL) {
-		return;
-	}
-
-	disconnect_scripts_directory_handler (view, &view->details->scripts_added_handler_id);
-	disconnect_scripts_directory_handler (view, &view->details->scripts_changed_handler_id);
-
-	nautilus_directory_file_monitor_remove (view->details->scripts_directory,
-						&view->details->scripts_directory);
+	GList *node;
 
-	nautilus_directory_cancel_callback (view->details->scripts_directory,
-					    reset_scripts_menu_callback,
-					    view);
+	for (node = view->details->scripts_directory_list; node != NULL; node = node->next) {
+		disconnect_scripts_directory_handler (view, node->data);
+	}
 }
 
 /**
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]