Created attachment 187943 [details]
Nautilus sidebar

-> GtkFileChooser

Not a nautilus bug, but I think this is something in the plans for GTK+ 3.2 already.

I have a branch in progress, called "places-sidebar" on git.gnome.org, which has a GtkPlacesSidebar widget in progress.  Help is much appreciated:

- GtkPlacesSidebar is the code for the equivalent widget from Nautilus, with a lot of Nautilus-isms removed.

- gtkplacessidebar.c has a DO_NOT_COMPILE macro, which is used to #ifdef out sections that refer to Nautilus code.  We need to see if that code has to be abstracted out through signals/methods/whatever, or if it can be done directly through GIO.

- It would be good to update gtkplacessidebar.c with the most recent changes to Nautilus's own code.

- gtkfilechooserdefault.c has sections with "#if REMOVE_FOR_PLACES_SIDEBAR".  Mostly they are the sections which used to implement the places sidebar within the file chooser itself; they should be removed when the independent GtkPlacesSidebar widget is finished.

- Nautilus needs a corresponding branch to be done with GtkPlacesSidebar.

*** Bug 670572 has been marked as a duplicate of this bug. ***

Both GTK+ and Nautilus have "places-sidebar" branches now.  I have to finalize the API and add things like GObject properties and getters for all setters.  But other than that, it is done and almost ready for merging.

As discussed on IRC:

The "location-selected" signal should be called "open-location".

The DnD signals are weird in that they let the file chooser and Nautilus have different behaviors.  We should standardize on something.

Also from IRC:

Always show Trash, and don't have an API to turn it off.  Showing "trash:///" works fine, anyway.  Figure out how to extract real pathnames and not trash:/// URIs later.

I've done a bit of API review on the git branch, so here goes. The review was done against http://git.gnome.org/browse/gtk+/tree/gtk/gtkplacessidebar.h?id=c26293eacdd936582fc2fc617b8f00df4aa12a0b

The first question I asked on IRC was what to describe what the places widget is. Apparently it's something that displays a bunch of things. What things? Federico said it's GFiles and that's all the developer needs to know about it. So a developer does not need to know if something is a shortcut or a generally shown thing, the only relevant thing is that when something happens with it, you get told the file.
Now of course, what do we do when there's multiple places pointing to the same direction (having a bookmark to a shortcut or even to the Desktop seems reasonably common, and people will definitely do it erroneously from time to time). The consensus was that this is the job of the places widget and not the developer using it. So there we go.

Another thing we shortly argued was if there need to be different modes for operation when the filesystem can be modified (ie in a file manager where moving files and creating/deleting folders is common) and when it can't (file chooser). The consensus was that we want to avoid it, so I'm assuming this from now on.

> gtk_places_sidebar_set_current_location
>
This seems pretty obvious. The only question is how to treat multiple places pointing to the same file. But see above.

> gtk_places_sidebar_set_accept_uri_drops
>
This is mostly about the modifiable file system. See above.

> gtk_places_sidebar_set_show_desktop
> gtk_places_sidebar_set_show_trash
> gtk_places_sidebar_set_trash_is_full
> gtk_places_sidebar_set_show_cwd
>
I'm not sure why an application should ever want to set that, even if it pretends to be a file manager. This should at least be a global per-desktop setting or be hardcoded in GTK. I'd prefer the hardcoding, otherwise we'd need to have to get an XSetting so the settings daemon can set it.
And why does the application decide if a trash is full? Shouldn't the trash know that?

> gtk_places_sidebar_set_show_properties
> gtk_places_sidebar_set_multiple_tabs_supported
> gtk_places_sidebar_set_multiple_windows_supported
>
This seems to about actions in the right click menu. I'd much rather have an API like gtk_places_sidebar_add_action (widget, action_name, action_label) and a simple "action-performed" signal that gets passed the action name. And then have some sort of convenience function gtk_places_sidebar_add_tab_actions() or so.

Also, could we (ab)use any of the new action APIs used in GtkApplication for this?

> gtk_places_sidebar_add_shortcut
> gtk_places_sidebar_remove_shortcut
>
As shortcuts are supposed to be application-provided files (like a photo editor adding a link to $HOME/Photos), those look good to me. What I don't like is that they return a GError. In particular because the error is about a thing the programmer should know in advance ("Did I add that shortcut before?"). I'd much prefer if they had a documented default behavior in those cases. I'd go with "don't add twice, ignore removal of shortcuts that don't exist", I'd also be happy with return_if_fail()ing.
Throwing a GError just makes the code harder to use for a developer for no gain.

> gtk_places_sidebar_list_shortcuts
> gtk_places_sidebar_get_nth_bookmark
>
This should either both be array access or both be list access. Array access implies an order. Federico claimed that the file chooser uses this for Alt-1 to Alt-9 to quick-jump to those bookmarks, so it seems we guarantee an order. If we do that, we should also guarantee one for shortcuts.

(In reply to comment #8)

Thanks for the review, Benjamin!

> > gtk_places_sidebar_set_current_location
> >
> This seems pretty obvious. The only question is how to treat multiple places
> pointing to the same file. But see above.

I think the correct policy has two parts:

1. If you click on a (duplicated) place, the selection stays there.

2. If something causes the file chooser to change to a place that is shown more than once (e.g. you navigate to Home and then double-click on the Documents folder, which happens to be shown twice in the list), then either:  a) we highlight the first one in the list; b) we highlight the one closer to having been specified by the user, the one in bookmarks, not in the XDG dirs.

(The other option is to always disallow duplicates - users wouldn't be able to bookmark Documents, etc. - but I think the user should be allowed to bookmark whatever he damn well pleases.)

(1) and (2) happen just fine right now, and they feel okay to me.
 
> > gtk_places_sidebar_set_accept_uri_drops
> >
> This is mostly about the modifiable file system. See above.

Yeah, I'm not comfortable with that.  Cosimo will chip in with his thoughts about making this consistent, I hope.
 
> > gtk_places_sidebar_set_show_desktop
> > gtk_places_sidebar_set_show_trash
> > gtk_places_sidebar_set_trash_is_full
> > gtk_places_sidebar_set_show_cwd
> >
> I'm not sure why an application should ever want to set that, even if it
> pretends to be a file manager.

show_desktop() - I don't like having this, and it's more a gesture to environments who want to use the places sidebar *and* use a Desktop metaphor.  I'd be more comfortable if Gnome at large said, Desktop is available unless disabled by the standard XDG-dirs means.  (Currently you can have a ~/Desktop directory with files that predate Gnome 3, which Gnome ignores, and that's pretty bad.)

set_show_trash() and set_trash_is_full().  As discussed on IRC, I want to make trash:/// show up always in the places sidebar, as it works fine if you type it manually in the location bar.  The only thing missing is to have something like NautilusTrashMonitor in GIO - in my branches, the *only* thing Nautilus does with respect to trash and the places sidebar is to hook it to NautilusTrashMonitor.

set_show_cwd() - We talked about this during the gtk-devel meeting:
https://mail.gnome.org/archives/gtk-devel-list/2012-November/msg00083.html

The file chooser wants to show $CWD if it is different from the default directory (i.e. $HOME) for apps launched from the desktop shell, so that terminal users will get something useful.  But Nautilus *is* part of the desktop shell, so showing $CWD there is meaningless.

I just had an idea:  what if the file chooser uses the mechanism in gtk_places_sidebar_add_shortcut() to add that $CWD if necessary, instead of having an explicit API for it.  I'll do that.

> > gtk_places_sidebar_set_show_properties
> > gtk_places_sidebar_set_multiple_tabs_supported
> > gtk_places_sidebar_set_multiple_windows_supported
> >
> This seems to about actions in the right click menu. I'd much rather have an
> API like gtk_places_sidebar_add_action (widget, action_name, action_label) and
> a simple "action-performed" signal that gets passed the action name. And then
> have some sort of convenience function gtk_places_sidebar_add_tab_actions() or
> so.
> 
> Also, could we (ab)use any of the new action APIs used in GtkApplication for
> this?

Hey, I like this idea of defining actions for the menu.  I guess we want a global add_action() function that works for all the places shown, or do we want to complicate things with per-place actions?

The set_multiple_{windows,tabs}_supported() are for the benefit of Nautilus.  There is a GtkPlacesOpenMode enum which gets passed in the ::location-selected signal, and depends on whether the location was left-clicked, middle-clicked, or ctrl-middle-clicked.  The open mode is always GTK_PLACES_OPEN_MODE_NORMAL if you don't enable set_multiple_*_supported().

(Maybe I could remove those APIs, keep on emitting the open mode, and just having the file chooser ignore it, while Nautilus would pay attention to it.)
 
> > gtk_places_sidebar_add_shortcut
> > gtk_places_sidebar_remove_shortcut
> >
> As shortcuts are supposed to be application-provided files (like a photo editor
> adding a link to $HOME/Photos), those look good to me. What I don't like is
> that they return a GError. In particular because the error is about a thing the
> programmer should know in advance ("Did I add that shortcut before?").

Yeah, good point.  I'll remove the GError and add the shortcut unconditionally.  Apps can be smart enough not to add duplicates themselves.

> > gtk_places_sidebar_list_shortcuts
> > gtk_places_sidebar_get_nth_bookmark
> >
> This should either both be array access or both be list access. Array access
> implies an order. Federico claimed that the file chooser uses this for Alt-1 to
> Alt-9 to quick-jump to those bookmarks, so it seems we guarantee an order. If
> we do that, we should also guarantee one for shortcuts.

Shortcuts are the app-specific shortcuts, like Clipart.

Bookmarks are the user-defined stuff, global across apps.

list_shortcuts() is to preserve the original file chooser API of gtk_file_chooser_list_shortcuts().

get_nth_bookmark() is to do the quick-jump to bookmarks.  I'm not just querying the new _gtk_bookmarks_manager directly, because the places sidebar may actually omit showing some of those bookmarks if they don't exist on disk anymore (and _gtk_bookmarks_manager doesn't know that - it doesn't stat() the bookmarks).

I'm getting more and more uncomfortable with a) having a private _gtk_bookmarks_manager; b) having it being not as smart as NautilusBookmark.  I'd love some help in moving this over to GTK+; it's not code that I particularly love :)


Summary of actions I'll take:

* Remove _set_show_cwd(); make the file chooser do that itself with _add_shortcut()

* Remove _set_multiple_windows/tabs_supported(), and make Nautilus handle that on its own.

* Remove the GError from add/remove_shortcut().


Summary of things we need to discuss:

* What to do with URI drops and the DnD inconsistencies between Nautilus and GtkFileChooser in general.

* What to do about showing Desktop or not.

* What to do about a trash monitor closer to GIO.

* Adding menu actions instead of an explicit _set_show_properties().

* What to do about having a bookmarks manager in GTK+.

Remove GError from shortcuts functions - done.

Remove _set_show_cwd() - will do it in a second.

If I remove _set_multiple_windows/tabs_supported(), how can I know whether to remove the corresponding menu items?

One of the things that just came to me:

Could displaying the context menu be handled by the application instead of the widget? There could be an open-context-menu signal that gets emitted. That way Nautilus and the file chooser could customize that menu any way they want.

(In reply to comment #11)
> Could displaying the context menu be handled by the application instead of the
> widget? There could be an open-context-menu signal that gets emitted. That way
> Nautilus and the file chooser could customize that menu any way they want.

Let's see.  The current items are:

* Open - easy to do in the sidebar or in the application (it's the default action when you click, anyway).  Could just remove this.  Potential for duplicated code if the sidebar doesn't handle it by itself.

* Open in new tab / Open in new window - easy to do for Nautilus.  Since the sidebar processes button clicks, it would still take care of figuring out Button1 / Button2 / Ctrl-button2 for activations without the menu.

* Add bookmark - only shown for volumes; easy to do either way.  Potential for duplication.

* Remove - Easy to do either way.  Potential for duplication.

* Rename - Hard to do outside the sidebar if you want inline editing of the bookmark's name.

* Mount / Unmount / Eject / Detect Media / Start / Stop - I'd prefer to have this code only *once*.  It's not tricky code, but it doesn't feel "close to the system" if every app does it differently.  Start/Stop get their actual strings changed depending on g_drive_get_start_stop_type(), and other details like that.

* Empty trash - As before for the trash monitor, we need trash machinery in GIO.

* Properties - We can move it to Nautilus, either by letting it generate the menu, or by letting it add actions to items in the sidebar.


So let's say we have a "populate-popup" signal, similar to the one in GtkEntry, and start the menu populated with

  Open
  Add bookmark
  Remove
  Mount/Unmount/etc.
  Empty trash (*)

Nautilus would have an easy time appending "Properties", and inserting "Open in new tab / open in new window" before the second item.  I guess this makes the ordering of items part of the ABI :)

I'll start work on the populate_popup signal tomorrow.

The trash stuff remains up for discussion - do we move it to GIO or what?

As I was asked to comment on this too. In Thunar we have a similar sidepane (since 1.6), but we have no included the special directories. Rationale behind this is that users can decided on their own what to add in their bookmarks.

I agree the popup menu should not be implemented by the widget.

Also, what about volume visibility? Is it required for an application/user to hide a volume?

(In reply to comment #13)
> As I was asked to comment on this too. In Thunar we have a similar sidepane
> (since 1.6), but we have no included the special directories. Rationale behind
> this is that users can decided on their own what to add in their bookmarks.

Thanks for CCing yourself on the bug :)

Any suggestions on how to go about this?  Just having an API to turn the XDG dirs on and off would not work well, as GTK+ would like them on by default (and would appear like that in the file chooser), but Thunar would like them off.

We don't have XSettings anymore, I think.  Not sure how to do this.

> I agree the popup menu should not be implemented by the widget.

Good.  What about the stock menu items I mentioned in comment #12?  The app would be handed a menu with those few items, and it would be free to add others.

> Also, what about volume visibility? Is it required for an application/user to
> hide a volume?

What does Thunar do?  (Why not show all volumes?)

(In reply to comment #14)
> (In reply to comment #13)
> > As I was asked to comment on this too. In Thunar we have a similar sidepane
> > (since 1.6), but we have no included the special directories. Rationale behind
> > this is that users can decided on their own what to add in their bookmarks.
> 
> Thanks for CCing yourself on the bug :)
> 
> Any suggestions on how to go about this?  Just having an API to turn the XDG
> dirs on and off would not work well, as GTK+ would like them on by default (and
> would appear like that in the file chooser), but Thunar would like them off.
> 
> We don't have XSettings anymore, I think.  Not sure how to do this.

Make an option in GtkSettings? XSettings still work there and applications/users have the option to override it.
 
> > I agree the popup menu should not be implemented by the widget.
> 
> Good.  What about the stock menu items I mentioned in comment #12?  The app
> would be handed a menu with those few items, and it would be free to add
> others.

The bookmark items can have a remote option i guess, but volume handing might be handled different between apps (sync or async, different mount operations).
Another option is to use signals for volume/mount operations, if not implemented by the app, fallback to the internal usage.

> > Also, what about volume visibility? Is it required for an application/user to
> > hide a volume?
> 
> What does Thunar do?  (Why not show all volumes?)

By default it shows all volumes, like the filechooser dialog, but you can optionally hide them. Think of volumes mounted in $HOME, partitions of other installations, etc.

I think the looks (the treeview and sorting) and user handling (dnd, device operations) are most important here.

(In reply to comment #12)

> So let's say we have a "populate-popup" signal, similar to the one in GtkEntry,
> and start the menu populated with
> 
>   Open
>   Add bookmark
>   Remove
>   Mount/Unmount/etc.
>   Empty trash (*)
> 
> Nautilus would have an easy time appending "Properties", and inserting "Open in
> new tab / open in new window" before the second item.  I guess this makes the
> ordering of items part of the ABI :)

I am not sure I really like this last part, especially since the items and the order will change according to what item was selected, though duplicating code to handle common actions such as "add bookmark" or the mount/unmount stuff seems highly undesirable (even the code that detects whether there were any additional keys pressed on click or the button that triggered the event - I don't think connecting to button-press/release event is a good solution for this).

I think it's fair to assume there are four "sections" in this menu:
- Open actions
- Bookmark actions (add, remove)
- Manipluation actions (mount, empty trash)
- Custom application provided actions (e.g. properties)

I believe all of these actions (except for the fourth case of course) should be provided directly by the widget which is in the best position to perform the generic action and make it sensitive according to selection.

This would leave out only the new tab/new window actions, which could have an apposite API (e.g. gtk_places_sidebar_set_open_flags and and enum GtkPlacesSidebarOpenFlags); the flags would be forwarded as a hint to the signal emitted to switch location, and clients would be free to decide whether or not to obey them.

> The trash stuff remains up for discussion - do we move it to GIO or what?

For reference, the Nautilus helper object is here [1]. No strong feelings about this, it seems simple enough that it could go into GIO indeed. Not sure it'd be very useful outside of a file chooser/file manager though.

[1] http://git.gnome.org/browse/nautilus/tree/libnautilus-private/nautilus-trash-monitor.c

(In reply to comment #15)
[Showing XDG dirs vs. not showing them for Thunar] 
> Make an option in GtkSettings? XSettings still work there and
> applications/users have the option to override it.

Oh, you are right, this got moved to GtkSettings in GTK+ 3.x.  I'll do it that way.  How about a property name of "gtk-places-sidebar-show-xdg-directories"?

> The bookmark items can have a remote option i guess, but volume handing might
> be handled different between apps (sync or async, different mount operations).
> Another option is to use signals for volume/mount operations, if not
> implemented by the app, fallback to the internal usage.

What's the "remote option" for bookmarks?

The places sidebar has a bit of smarts (copied from Nautilus) to handle volume mounting.  On IRC, Cosimo and I have discussed exposing the GtkMountOperation that is used, but never ended up with an actual need for it.

Can you please read a bit of gtkplacessidebar.c, grep for "gtk_mount_operation", and see what things you would need to do with it?  With any luck the current code will already serve your purposes; maybe we can avoid exposing the mount operation then.

> By default it shows all volumes, like the filechooser dialog, but you can
> optionally hide them. Think of volumes mounted in $HOME, partitions of other
> installations, etc.

I'm wondering how to handle this.  How about a

  gboolean should_show_place (drive, volume, mount)

signal, that is emitted for each drive/volume/mount when the sidebar is populating itself.

(In reply to comment #16)
> 
> I think it's fair to assume there are four "sections" in this menu:
> - Open actions
> - Bookmark actions (add, remove)
> - Manipluation actions (mount, empty trash)
> - Custom application provided actions (e.g. properties)
> 
> I believe all of these actions (except for the fourth case of course) should be
> provided directly by the widget which is in the best position to perform the
> generic action and make it sensitive according to selection.
> 
> This would leave out only the new tab/new window actions, which could have an
> apposite API (e.g. gtk_places_sidebar_set_open_flags and and enum
> GtkPlacesSidebarOpenFlags)

* Opening:

The idea of a GtkPlacesSidebarOpenFlags is more or less equivalent to the original API of gtk_places_sidebar_set_multiple_{tabs,windows}_supported(), which Benjamin didn't like.  Maybe that was due to entry-point explosion.  I'm leaning towards liking the idea of flags for this.  Benjamin?

(I've made Nautilus insert the "Open in new XXX" items in the menu, and it works, but it is indeed iffy even if the places sidebar guarantess that the plain "Open" item is always present in the first position.)

[NautilusTrashMonitor]
> > The trash stuff remains up for discussion - do we move it to GIO or what?
> 
> For reference, the Nautilus helper object is here [1]. No strong feelings about
> this, it seems simple enough that it could go into GIO indeed.

Upon looking at the NautilusTrashMonitor code, I see that it is indeed very simple.  It is literally just a GIO file monitor on "trash:///" that selects the right icon based on whether the trash has stuff in it or not.  This simple code can be weaved into the places sidebar with no problems - but see below.

What *is* more complicated is the "empty trash" machinery in nautilus_file_operations_empty_trash(), and the related stuff to check whether a mount that is to be unmounted has trash files in it.

So, for now, I'm happy with:

1. Putting the trash monitoring stuff in GtkPlacesSidebar, and removing gtk_places_sidebar_set_show_trash() and gtk_places_sidebar_set_trash_is_full().  The Trash item will always be shown.

2. Creating the "Empty trash" menu item in Nautilus as it is now, and leaving the empty-trash machinery there.

We can later discuss adding an API for trash operations in GIO and GTK+ as appropriate.

(In reply to comment #17)
> (In reply to comment #15)
> [Showing XDG dirs vs. not showing them for Thunar] 
> > Make an option in GtkSettings? XSettings still work there and
> > applications/users have the option to override it.
> 
> Oh, you are right, this got moved to GtkSettings in GTK+ 3.x.  I'll do it that
> way.  How about a property name of "gtk-places-sidebar-show-xdg-directories"?

Bit long,  but it does describe the purpose ;-).

> > The bookmark items can have a remote option i guess, but volume handing might
> > be handled different between apps (sync or async, different mount operations).
> > Another option is to use signals for volume/mount operations, if not
> > implemented by the app, fallback to the internal usage.
> 
> What's the "remote option" for bookmarks?

Nm, we already dropped this. Previously we kept local and "remote" bookmark locations separately.

> The places sidebar has a bit of smarts (copied from Nautilus) to handle volume
> mounting.  On IRC, Cosimo and I have discussed exposing the GtkMountOperation
> that is used, but never ended up with an actual need for it.
> 
> Can you please read a bit of gtkplacessidebar.c, grep for
> "gtk_mount_operation", and see what things you would need to do with it?  With
> any luck the current code will already serve your purposes; maybe we can avoid
> exposing the mount operation then.

An example would be notifications. Thunar shows a persistent notification if the umount/eject action is still running and eventually when its safe to remove the device. I know the pre-unmount signals from the volume monitor can be used as well, but directly feels a bit safer.

> > By default it shows all volumes, like the filechooser dialog, but you can
> > optionally hide them. Think of volumes mounted in $HOME, partitions of other
> > installations, etc.
> 
> I'm wondering how to handle this.  How about a
> 
>   gboolean should_show_place (drive, volume, mount)
> 
> signal, that is emitted for each drive/volume/mount when the sidebar is
> populating itself.

Duno if this is worth for the widget, only saying we added this. Else it probably easier to directly hook this in a gsetting and use a hidden-group. No developer api.

*** Bug 666795 has been marked as a duplicate of this bug. ***

GtkPlacesSidebar is in 3.9.0

*** Bug 520633 has been marked as a duplicate of this bug. ***