GNOME Bugzilla – Bug 679580
add bookmarks to the file chooser for open windows
Last modified: 2012-07-10 14:31:43 UTC
It might be nice to add bookmarks to the file chooser that is used for copy/move to for the current directory of other open nautilus windows.
Created attachment 218334 [details] [review] Add locations for open windows to the destination selection dialog These locations are identified as interesting by the fact that they are in use and it makes sense to offer them as quick options for explicit copies and moves.
Review of attachment 218334 [details] [review]: Looks mostly good; some comments below. ::: src/nautilus-view.c @@ +5797,3 @@ + + application = gtk_window_get_application (GTK_WINDOW (view->details->window)); + GtkApplication *application; I wonder if we should also connect to the window-added and window-removed signals and update the bookmarks when those get emitted. @@ +5802,3 @@ + GList *s; + + GList *w; I think I would like it better to have a nautilus_window_get_slots() method and avoid including the private header here. @@ +5812,3 @@ + &error); + if (error != NULL) { + NautilusWindow *window = w->data; You can use the DEBUG() macro instead
Created attachment 218368 [details] [review] Add locations for open windows to the destination selection dialog These locations are identified as interesting by the fact that they are in use and it makes sense to offer them as quick options for explicit copies and moves.
Created attachment 218370 [details] [review] Add locations for open windows to the destination selection dialog These locations are identified as interesting by the fact that they are in use and it makes sense to offer them as quick options for explicit copies and moves. Fix error clearing. Noticed that this still doesn't track the locations of secondary tabs/slots so more work is needed.
Review of attachment 218370 [details] [review]: Looks good so far, just some minor comments below. ::: src/nautilus-window-slot.c @@ +311,3 @@ + 0, + NULL, NULL, + nautilus_marshal_VOID__STRING_STRING, You can just pass NULL here and rely on the (recently introduced) generic marshaller...that will allow to get rid of the marshal.list automake stuff. ::: src/nautilus-window.h @@ +86,3 @@ void (* close) (NautilusWindow *window); + void (* slot_added) (NautilusWindow *window, NautilusWindowSlot *slot); + void (* slot_removed) (NautilusWindow *window, NautilusWindowSlot *slot); No need for this and the similar snippet in nautilus-window-slot.h, since we don't have any default class handlers to override for these signals.
Created attachment 218378 [details] [review] Add locations for open windows to the destination selection dialog These locations are identified as interesting by the fact that they are in use and it makes sense to offer them as quick options for explicit copies and moves.
Created attachment 218414 [details] [review] Add locations for open windows to the destination selection dialog These locations are identified as interesting by the fact that they are in use and it makes sense to offer them as quick options for explicit copies and moves. Fixes some errors with the last rebase and tries harder to disconnect all signals.
Review of attachment 218414 [details] [review]: Thanks, looks almost good to go. ::: src/Makefile.am @@ -71,3 @@ glib-compile-resources --target=$@ --sourcedir=$(srcdir) --generate-header --c-name nautilus $(srcdir)/nautilus.gresource.xml - Can you also rebase this out of the patch please? ::: src/nautilus-view.c @@ +5784,3 @@ + + if (slot->location == NULL) + GError *error = NULL; You can use nautilus_window_slot_get_location_uri() (also below) @@ +5910,3 @@ + windows = gtk_application_get_windows (application); + g_signal_handlers_disconnect_by_func (application, G_CALLBACK (on_app_window_added), data); + add_bookmarks_for_window_slot (data, slot); These are disconnected twice ::: src/nautilus-window.h @@ +86,3 @@ void (* close) (NautilusWindow *window); + void (* slot_added) (NautilusWindow *window, NautilusWindowSlot *slot); + void (* slot_removed) (NautilusWindow *window, NautilusWindowSlot *slot); No need for these.
Created attachment 218418 [details] [review] Add locations for open windows to the destination selection dialog These locations are identified as interesting by the fact that they are in use and it makes sense to offer them as quick options for explicit copies and moves.
Review of attachment 218418 [details] [review]: Thanks, looks good.