GNOME Bugzilla – Bug 756589
Improve the heuristics for external drives
Last modified: 2015-11-23 17:36:53 UTC
Following https://bugzilla.gnome.org/show_bug.cgi?id=755654 we want to hide all external drives in the sidebar and show them on gtkplacesview. However checking for can_eject and is_media_removable is not enough, since some external drives report that they are not. However we can check if they can be stopped, that I believe internal drives can not. These serie of patches do it for the sidebar and the gtkplacesview, alongside with two patches for gtkplacesview that clean up some code.
Created attachment 313326 [details] [review] gtkplacessidebar: improve heuristics for external drives Since the change to use GtkPlacesView we don't want to show internal storage on the sidebar. In our case we were checking for drive_can_eject and drive_is_media_removable. However for some external hard drives it's reported that they are not ejectable nor the have removable media. So the only attribute that they have different from internal drives is that they can be stopped. So check for if the drive can be stopped to decide if it is external or internal. On the way realized we don't need to check for the mounts associated with the volume to know if the volume can be ejected or not. So remove that code.
Created attachment 313327 [details] [review] gtkplacesview: remove dead code This is checked on is_removable_volume
Created attachment 313328 [details] [review] gtkplacesview: remove dead code This is checked on add_volume inside the loop.
Created attachment 313329 [details] [review] gtkplacesview: improve heuristics for external drives Following the sidebar on commit b0989b190df, improve the way we check when a drive is external or not.
Review of attachment 313326 [details] [review]: Might have been nicer as three separate commits: 1) remove the unused mount 2) rename from removable to external 3) add the extra condition But I'm not picky, so just push it as-is.
Review of attachment 313327 [details] [review]: I would say "checked in", not "checked on".
Review of attachment 313328 [details] [review]: Here too: "checked in"
Review of attachment 313329 [details] [review]: same patch as the first ?!
Review of attachment 313329 [details] [review]: ah, my bad. This is view, not bar
thanks for the reviews. Agree would look better, will do next time. Attachment 313326 [details] pushed as 0cd4e7e - gtkplacessidebar: improve heuristics for external drives Attachment 313327 [details] pushed as 569be9f - gtkplacesview: remove dead code Attachment 313328 [details] pushed as 569be9f - gtkplacesview: remove dead code Attachment 313329 [details] pushed as 1dbcce7 - gtkplacesview: improve heuristics for external drives
*** Bug 757034 has been marked as a duplicate of this bug. ***
*** Bug 756782 has been marked as a duplicate of this bug. ***
Created attachment 315653 [details] nautilus screencast Is this really fixed? On Arch Linux, since gtk 3.18.4 and nautilus 3.18.2 things got worse (see duplicate bug #756782) - now even external drives that used to be properly shown on 'Other Locations' aren't anymore, even though mounted properly...
*** Bug 756542 has been marked as a duplicate of this bug. ***
(In reply to Luis Henrique Mello from comment #13) > Created attachment 315653 [details] > nautilus screencast > > Is this really fixed? On Arch Linux, since gtk 3.18.4 and nautilus 3.18.2 > things got worse (see duplicate bug #756782) - now even external drives that > used to be properly shown on 'Other Locations' aren't anymore, even though > mounted properly... you need gtk 3.18.5 (sorry I didn't bump gtk+ requirement)
(In reply to Carlos Soriano from comment #15) > (In reply to Luis Henrique Mello from comment #13) > > Created attachment 315653 [details] > > nautilus screencast > > > > Is this really fixed? On Arch Linux, since gtk 3.18.4 and nautilus 3.18.2 > > things got worse (see duplicate bug #756782) - now even external drives that > > used to be properly shown on 'Other Locations' aren't anymore, even though > > mounted properly... > > you need gtk 3.18.5 (sorry I didn't bump gtk+ requirement) thank you, it's ok now :)