GNOME Bugzilla – Bug 787128
Re-add FUSE network mounts in local-only mode
Last modified: 2018-08-28 19:43:22 UTC
Created attachment 358936 [details] [review] Patch against git master Hi, The attached patch re-adds the network mounts that are accessible locally (via FUSE) to GtkFileChooser when in local-only mode. A lot of people miss this a lot while major applications (Firefox, Chromium, etc) gradually switch to GTK+3. The patch applies to today's git master. Hope this helps, Colin
Review of attachment 358936 [details] [review]: I can't speak for whether the behaviour is desired, but I *can* post style nitpicks so no one else has to. ::: gtk/gtkplacessidebar.c @@ +894,3 @@ + if (path != NULL) + { + base_file = g_mount_get_root (mount); missing space before parenthesis @@ +1300,3 @@ + mount = l->data; + + if (sidebar->local_only) ditto @@ +1308,3 @@ + } + + mount = l->data; I'm not sure we need a 3-line comment when the code explains the same thing pretty unambiguously already. @@ +1347,3 @@ mount = l->data; + + if (sidebar->local_only && !is_mount_locally_accessible(mount)) another missing space before parenthesis @@ +1348,3 @@ + + if (sidebar->local_only && !is_mount_locally_accessible(mount)) + continue; This should be 2 spaces of indentation, not 4
Thanks again to Bugzilla/Splinter for showing my additions below the wrong lines in comments in 50% of cases. That's really helpful. Check the review for what I was really commenting on.
Created attachment 358969 [details] [review] Updated patch with coding styles issues fixed Hi Daniel, Thanks for your comments. I have updated the patch accordingly :)
Thanks for that!
Created attachment 359361 [details] [review] Better patch to fix the problem Hello, Here is an updated patch. I based my first one on a patch I did against GTK+3.14, which did not have an "Other Locations" entry in the sidebar, and it did add the remote shares to the sidebar. Apparently the new way is to put them in Places View. This patch does that. Hope this helps.
*** Bug 727538 has been marked as a duplicate of this bug. ***
Review of attachment 359361 [details] [review]: This should be git-formatted with an explanation as to why you're making this change. ::: gtk/gtkplacesview.c @@ +1905,3 @@ + base_file = g_mount_get_root (mount); + + if (base_file != NULL) Why not carry on with the "return early" way? if (base_file == NULL) return FALSE; if (path == NULL) ... @@ +1907,3 @@ + if (base_file != NULL) + { + char *path = g_file_get_path (base_file); Don't assign results of a function when declaring. @@ +1912,3 @@ + if (path != NULL) + { + g_free(path); Space before parenthesis. @@ +1948,3 @@ + g_object_get(G_OBJECT (placesviewrow), "mount", &mount, NULL); + + if (mount != NULL) Is this necessary? You already do the check in the function. @@ +1951,3 @@ + { + is_local = is_mount_locally_accessible (mount); + g_object_unref(mount); Space before parenthesis as well. You could also use g_clear_object and avoid the NULL check.
Created attachment 359630 [details] [review] patch v3 addressing Bastien's remarks Hi Bastien, Thanks for the review. Here is an updated patch.
Hi, Any news on this patch? Thanks in advance, Colin
Hi, I'm not a linux guru, sorry. Can someone please explain to me how I can apply this patch? And does it also work with Linux Mint 18.2 Cinnamon 64bit? Thank you very much! Kind regards, Erwin ter Hofstede
It's not Linux-specific. This is a source patch, so to apply it, you need to use a compatible program (in this case Git) to incorporate its changes, then recompile a new version of library including it. That'll work on any OS that can compile/run this version of GTK+ 3. Google for JHbuild for an easy way to build. As for whether there's any news on the patch getting into GTK+ upstream, not until a maintainer OKs it. You can ping IRC or the mailing list with a reminder.
Review of attachment 359630 [details] [review]: looks fine to me
Good afternoon. I'm using Debian Stretch, it have the same problem. Could you give me a step by step how to use this patch to fix the problem?