Created attachment 309707 [details] [review]
view: add interface

Nautilus is in the proccess of receiving a places view,
based on GtkFileChooser's one.

To be able to handle that, an abstraction layer is needed
between NautilusFilesView and NautilusWindowSlot, so we factor
out the common data between views.

Add the NautilusView interface, and make NautilusFilesView a
NautilusView implementation. Because of the new way we handle
search on the view side, the search logic is rewritten to match
the new expected behavior.

https://bugzilla.gnome.org/show_bug.cgi?id=753777

Created attachment 309708 [details] [review]
places-view: implement a view for Other Locations

GtkFileChooser received a Other Locations view that lists
persistent devices, as well as networks and the root location
for the computer's hard drive.

Since Nautilus is a file management tool too, it should keep
consistency between Gtk+ file chooser, something that doesn't
happen since it doesn't display Other Locations.

To fix that, add NautilusPlacesView, a NautilusView implementation
that displays the GtkPlacesView widget. In order to implement that,
update window-slot to correctly display the places-view whenever
Other Locations is clicked.

https://bugzilla.gnome.org/show_bug.cgi?id=753777

Created attachment 309709 [details] [review]
connect-server: remove legacy code

nautilus-connect-server is a legacy application from
GNOME 2 era, where we had no consistent way to manage
servers.

With the introduction of NautilusPlacesView, this code
becomes obsolete, as the network management logic is
moved to the places view.

Remove the legacy code.

https://bugzilla.gnome.org/show_bug.cgi?id=753778

Review of attachment 309708 [details] [review]:

So two little changes here and I think it will look fine!

::: libnautilus-private/nautilus-file-utilities.c
@@ +271,3 @@
+
+                if (nautilus_file_is_in_other_locations (file)) {
+                        title = g_strdup (_("Other Locations"));

For now this works because you will onyl call this with the actual other locations file.
So either you take into account childrens here (which is not necesary right now) or make clear that what you want to say is: nautilus_file_is_other_locations, so in any case that in the future we add children, at least the code wont confuse the developer that is doing it

::: src/nautilus-application.c
@@ -390,1 @@
-	if (g_file_query_file_type (location, G_FILE_QUERY_INFO_NONE, NULL) != G_FILE_TYPE_DIRECTORY) {

Can't believe we were doing blocking I/O like this.... thanks for the change. this even deserves its own commit =)

::: src/nautilus-places-view.c
@@ +189,3 @@
+static void
+nautilus_places_view_set_location (NautilusView *view,
+                                   GFile        *location)

I think this has to be windows slot enough smart to know it should call the other locations view with a random location. As we said, windows slot is the controller for the views, and the views expects correct locations for them, so the communication is one way, not two ways (what we want is get rid of these ugly two ways communication we had until now, which makes everything confusing and difficult to follow, and is a can of worms for async operations)

@@ +210,3 @@
+                        if (slot) {
+                                nautilus_window_slot_open_location (NAUTILUS_WINDOW_SLOT (slot), location, 0);
+                        }

And if not what? silently fail? I think a warning if this happens, but it shouldn't...

::: src/nautilus-window-slot.c
@@ +219,3 @@
+
+        return g_object_ref (view);
+                                        slot->details->view_mode_before_search = g_strdup (nautilus_files_view_get_view_id (NAUTILUS_FILES_VIEW (slot->details->content_view)));

Seems you are actually doing the right thing here, use this function along the code to get the correct view instead of letting the views to call window slot some times.

@@ +1199,1 @@
+        if (!error || (error && error->domain == G_IO_ERROR && error->code == G_IO_ERROR_NOT_SUPPORTED)) {

g_error_matches

@@ +1326,3 @@
+                selection = NULL;
+
+                if (NAUTILUS_IS_FILES_VIEW (slot->details->content_view)) {

Probably we want this transparent as well... in the iface, since we could want this for other views.
For example, we could want to show the network or a partition selected from a different app. This is done tru dbus with a standrad call to FileManager1.ShowItems. So selection is actually necesary.
Currently gtkplacesview doesnt support it, but probably we will need to do it. So if it is not too much effort (I guess even less than doing the if/elses here) can you add it and do nothing for now for gtkplacesview implementation?

@@ +1394,1 @@
+                g_list_free_full (selection_copy, g_object_unref);

why is a copy needed? if it is needed, why does not set_selection does one?

@@ +1581,3 @@
+                selection = nautilus_files_view_get_selection (NAUTILUS_FILES_VIEW (slot->details->content_view));
+
+                if (g_list_length (selection) == 0) {

In this case is fine special casing, since by design we dont want to select the first item in other-places, but you could special casing here, not when getting the selection.

Created attachment 309765 [details] [review]
application: don't block UI if not necesary

we were asking for the file type blocking the UI. Use
nautilus file instead which catch the values.

Created attachment 309766 [details] [review]
view: add interface

Nautilus is in the proccess of receiving a places view,
based on GtkFileChooser's one.

To be able to handle that, an abstraction layer is needed
between NautilusFilesView and NautilusWindowSlot, so we factor
out the common data between views.

Add the NautilusView interface, and make NautilusFilesView a
NautilusView implementation. Because of the new way we handle
search on the view side, the search logic is rewritten to match
the new expected behavior.

Created attachment 309767 [details] [review]
places-view: implement a view for Other Locations

GtkFileChooser received a Other Locations view that lists
persistent devices, as well as networks and the root location
for the computer's hard drive.

Since Nautilus is a file management tool too, it should keep
consistency between Gtk+ file chooser, something that doesn't
happen since it doesn't display Other Locations.

To fix that, add NautilusPlacesView, a NautilusView implementation
that displays the GtkPlacesView widget. In order to implement that,
update window-slot to correctly display the places-view whenever
Other Locations is clicked.

Created attachment 309768 [details] [review]
connect-server: remove legacy code

nautilus-connect-server is a legacy application from
GNOME 2 era, where we had no consistent way to manage
servers.

With the introduction of NautilusPlacesView, this code
becomes obsolete, as the network management logic is
moved to the places view.

Remove the legacy code.

Created attachment 309769 [details] [review]
window-slot: use g_error_matches

And add a comment to track this...

Review of attachment 309765 [details] [review]:

avanti...

Review of attachment 309766 [details] [review]:

a toda...

Review of attachment 309767 [details] [review]:

vela!!

Review of attachment 309768 [details] [review]:

good

Review of attachment 309769 [details] [review]:

job!!! :)

and congratulations!

It's finally over. Thanks.