Re: [PATCH] add location button context menu

[Alexander Larsson <alexl redhat com>]

On Tue, 2005-05-17 at 13:05 +0200, Christian Neumair wrote:
> Am Dienstag, den 17.05.2005, 12:54 +0200 schrieb Alexander Larsson:
> > On Tue, 2005-05-17 at 12:45 +0200, Christian Neumair wrote:
> > > Am Dienstag, den 17.05.2005, 12:30 +0200 schrieb Alexander Larsson:
> > > > 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).
> > > 
> > > How should the location button know that the current view is an
> > > FMDirectoryView? On IRC, I was told that NautilusView is an additional
> > > layer of abstraction and that I should never ever do things like if
> > > (FM_IS_DIRECTORY_VIEW (view)) in NautilusWindows.
> > > ...or should we add a "location_widget_pressed" signal to the
> > > NautilusWindow, which the view can connect to?
> > 
> > You'd have to add something, say
> > "nautilus_view_popup_location_context_menu()", just not something that
> > exposes details of FMDirectoryView (like its selection and background
> > context menus).
> 
> Hrm I thought that every view would expose a selection, considering
> that we have
> nautilus_view_get_selection_count/nautilus_view_get_selection/nautilus_view_set_selection, so I thought this might be a good idea to add NAUTILUS_VIEW_MENU_SELECTION. Whatever, I'm going to adapt the patch.

Yeah, but they might not have a selection context menu. It may also be
the case that they have more context menus which means your enumeration
would look strange (since it only handles some essentially random
subset). I'd like to keep the requirement that the general code has on
views down to the minimal set that is really needed. This helps for
future new views and changes to old views.

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's a lounge-singing Amish sorceror fleeing from a secret government 
programme. She's a supernatural cat-loving barmaid with a knack for trouble. 
They fight crime!