Re: [PATCH] add location button context menu

[Alexander Larsson <alexl redhat com>]

On Tue, 2005-05-17 at 11:06 +0200, Christian Neumair wrote:
> Am Montag, den 16.05.2005, 15:49 +0200 schrieb Alexander Larsson:
> > On Fri, 2005-04-29 at 22:06 +0200, Christian Neumair wrote:
> > > This is the second version of a patch I've submitted about 2.5 months
> > > ago to nautilus-list [1]. There were no direct followups, but I was now
> > > asked to update the patch, though, so it looks like people are
> > > interested in it.
> > > The new version only has the Cut, Copy, Paste, Trash + Delete and
> > > Properties items in the context menu, but - in contrast to the last
> > > version - doesn't make nautilus spit out g_warnings. Also, I've added a
> > > context menu to the navigation view "Location:" label.
> > > Comments, suggestions?
> > > 
> > > [1]
> > > http://mail.gnome.org/archives/nautilus-list/2005-February/msg00040.html
> > 
> > I like this idea. Is there a later version of this patch fixing the
> > various bugs pointed out somewhere?
> 
> Here comes a new version. It has grown somewhat large. It adds some
> context menu code to the NautilusView interface so that other views can
> react appropriately as well and ports the directory view-related code to
> that semantics. Also note that I've added a new item on top of the
> context menu for opening the currently open folder in a new nav window.
> Are you OK with the popup menu code being potentially async or should we
> force to return a gboolean? Currently, we just return "FALSE" for the
> button_press_event handlers, since we can't know whether the popup
> succeeded, but who cares? :)
> I've adapted to the old
> fm_directory_view_pop_up_background/selection_context_menu semantics
> here.

I don't see why you've added a nautilus_view_popup_menu(). This
hardcodes a set of popup menus that may not fit for all types of views,
and is not needed by the general window code. Having a specific popup
function for the location context button seems better, and keeping the
specific ones in FMDirectoryView (although at some point FMDirectoryView
needs to be split in two so that parts of it can be used even for views
that show multiple directories).

I see no need to have paste in the location context menu. It doesn't do
anything other than the main paste.


+	if (g_utf8_collate (gtk_label_get_text (GTK_LABEL (label)), LOCATION_LABEL))

collate??? just use strcmp.


+
+	files = g_list_append (NULL, file);
+	copy_or_cut_files (view, files, TRUE);
+	nautilus_file_list_free (files);

This double-frees the file, since you never ref:ed the file in the
list, but you unref it. Same in other places. Better to just use
g_list_free.


 	gtk_widget_show (window->details->location_button);
-
 	hbox = gtk_hbox_new (FALSE, 3);

and

 	gtk_action_set_sensitive (action, !is_read_only);
-	
+
 	action = gtk_action_group_get_action (view->details->dir_action_group,

and

 }
 
+
 GtkWidget *
	
Don't make unnecessary whitespace changes like this. They mess up cvs
history.


=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's an old-fashioned coffee-fuelled card sharp plagued by the memory of his 
family's brutal murder. She's a ditzy bisexual advertising executive looking 
for love in all the wrong places. They fight crime!