After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 736285 - Improve display name for root
Improve display name for root
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: mtp backend
1.21.x
Other Linux
: Normal enhancement
: ---
Assigned To: Philip Langdale
gvfs-maint
Depends on:
Blocks:
 
 
Reported: 2014-09-08 19:44 UTC by Ross Lagerwall
Modified: 2014-10-17 11:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
mtp: Improve root dir name handling (6.88 KB, patch)
2014-09-08 19:48 UTC, Ross Lagerwall
needs-work Details | Review
Build shared code for gphoto2 and mtp as a convenience lib (17.87 KB, patch)
2014-09-09 18:08 UTC, Ross Lagerwall
committed Details | Review
mtp: Improve root dir name and icon handling (5.93 KB, patch)
2014-09-09 18:08 UTC, Ross Lagerwall
committed Details | Review
gphoto2: Improve root dir name and icon handling (3.59 KB, patch)
2014-09-09 18:08 UTC, Ross Lagerwall
committed Details | Review
gphoto2: include gvfsgphoto2utils.h conditionally (997 bytes, patch)
2014-10-17 07:29 UTC, Ondrej Holy
committed Details | Review

Description Ross Lagerwall 2014-09-08 19:44:09 UTC
The display name for the root dir comes from LIBMTP_GetFriendlyname or shows "Unnamed Device" if there is no friendly name. In this situation, the MTP volume still has a name (which comes from udev).

The result is that the folder name showed by Nautilus is "Unnamed Device", yet the device is called "Ascend G300" in the sidebar.  I think we should fallback to the udev-derived name.
Comment 1 Ross Lagerwall 2014-09-08 19:48:48 UTC
Created attachment 285673 [details] [review]
mtp: Improve root dir name handling

If the device has no friendly name, set the display name of the device
from the udev info to match the name shown by the volume monitor.

This is visible when opening the device in Nautilus (or using gvfs-info).
Comment 2 Philip Langdale 2014-09-08 20:32:03 UTC
All the name processing logic is copied verbatim from the monitor. Any chance we can share it? and I think it was originally copied from gphoto2's backend, so another de-dup opportunity. Even simple .c file re-use would be a big step up.
Comment 3 Ross Lagerwall 2014-09-08 21:52:26 UTC
(In reply to comment #2)
> All the name processing logic is copied verbatim from the monitor. Any chance
> we can share it? and I think it was originally copied from gphoto2's backend,
> so another de-dup opportunity. Even simple .c file re-use would be a big step
> up.

Yeah, that is a good idea.  There doesn't seem to be any shared code between the daemons and the monitors other than libgvfscommon and we probably shouldn't be sticking mtp (or gphoto2) specific code in there so I'll try some C code re-use.
Comment 4 Ross Lagerwall 2014-09-09 18:08:36 UTC
Created attachment 285763 [details] [review]
Build shared code for gphoto2 and mtp as a convenience lib

Deduplicate code between gphoto2 and mtp by creating a convenience
library call libgvfscommon-gphoto2 which is only built if either libmtp
is used or gphoto2 with gudev is used.
Comment 5 Ross Lagerwall 2014-09-09 18:08:44 UTC
Created attachment 285764 [details] [review]
mtp: Improve root dir name and icon handling

If the device has no friendly name, set the display name of the device
from the udev info to match the name shown by the volume monitor.

This is visible as the folder title when the root dir of the device is
open in Nautilus (or using gvfs-info).
Comment 6 Ross Lagerwall 2014-09-09 18:08:51 UTC
Created attachment 285765 [details] [review]
gphoto2: Improve root dir name and icon handling

Set the name of the root dir and the icon to match the way that the
volume monitor sets them.

This is visible as the folder title when the root dir of the device is
open in Nautilus (or using gvfs-info).
Comment 7 Ross Lagerwall 2014-09-09 18:09:49 UTC
Right, the attached series de-duplicates the code and uses it for both daemons and both monitors.
Comment 8 Philip Langdale 2014-09-09 18:15:17 UTC
Review of attachment 285763 [details] [review]:

Looks fine, but presumably you want to set the symbolic icon for gphoto2 as well while you're there?
Comment 9 Ross Lagerwall 2014-09-09 20:12:31 UTC
(In reply to comment #8)
> Review of attachment 285763 [details] [review]:
> 
> Looks fine, but presumably you want to set the symbolic icon for gphoto2 as
> well while you're there?

Yeah, will do.  I'll commit these after the freeze since they're a bit invasive and change strings.
Comment 10 Ross Lagerwall 2014-10-13 21:30:17 UTC
Pushed to master as 9ac1c5c. Thanks for the review!
Comment 11 Ting-Wei Lan 2014-10-17 06:11:00 UTC
The commit 60f96c8 added #include "gvfsgphoto2utils.h" to daemon/gvfsbackendgphoto2.c, which causes build failure on FreeBSD because gudev/gudev.h cannot be found.
Comment 12 Ondrej Holy 2014-10-17 07:29:22 UTC
Created attachment 288721 [details] [review]
gphoto2: include gvfsgphoto2utils.h conditionally

Does the attached patch fix it for you?
Comment 13 Ting-Wei Lan 2014-10-17 08:07:06 UTC
Yes, it can be fixed using the patch.
Comment 14 Ross Lagerwall 2014-10-17 11:13:15 UTC
Review of attachment 288721 [details] [review]:

Looks good, thanks for this.
Comment 15 Ondrej Holy 2014-10-17 11:59:35 UTC
Comment on attachment 288721 [details] [review]
gphoto2: include gvfsgphoto2utils.h conditionally

Thanks for reporting this and thanks for reviews!

Fix has been committed to master as:

commit e3fab1313117d8fefc98976b8c97d0f98cbb1819