GNOME Bugzilla – Bug 682101
Provide a way to get a symbolic icon for a device
Last modified: 2012-09-05 10:06:21 UTC
At various times we would like to be able to get a symbolic icon for a device. Specifically, we need them for the Nautilus sidebar. See bug 681457. It doesn't seem right for the gvfs backends to return symbolic icons when a caller asks for just an icon since there are still many uses for the full color icons. Perhaps we need a G_FILE_ATTRIBUTE_STANDARD_SYMBOLIC_ICON ?
Sounds good to me. You'd also need to add g_drive_get_symbolic_icon() g_volume_get_symbolic_icon() g_mount_get_symbolic_icon() methods to the API since that's what GVfs' computer:// backend would use to populate the GFile attributes in question. (You'd also need to add a lot of glue in GVfs including mount daemons and remote volume monitors to handle the g_{drive,volume,mount}_get_symbolic_icon() data... Cosimo recently did that for another API addition - it's a lot of work.)
Created attachment 222708 [details] [review] Add symbolic icon support to drive, volume, and mount We need symbolic icon support for display in Nautilus.
Created attachment 222709 [details] [review] Add symbolic icon support to gfileinfo
Created attachment 222710 [details] [review] Add ability to get symbolic icon for content type
Review of attachment 222708 [details] [review]: Looks generally good to me ::: gio/gunixmounts.c @@ +1936,2 @@ else + icon_name = use_symbolic ? "media-removable-symbolic" : "media-flash"; Is it worth filing a bug requesting a media-flash-symbolic for this? ::: gio/gwin32mount.c @@ +204,3 @@ + case DRIVE_REMOTE : return use_symbolic ? "folder-remote-symbolic" : "folder-remote"; + case DRIVE_CDROM : return use_symbolic ? "drive-optical-symbolic" : "drive-optical"; + default : return use_symbolic ? "folder-symbolic" : "folder"; Not completely sure about changing these icon names for win32 - ideally these would be provided inside GTK+ itself, but it looks like even previously we weren't using available stock icons, so maybe it's fine.
Review of attachment 222709 [details] [review]: Looks good to me
Review of attachment 222710 [details] [review]: ::: gio/gcontenttype.c @@ +472,3 @@ + */ +GIcon * + * Returns: (transfer full): symbolic #GIcon corresponding to the content type. Not a huge fan of duplicating all of the code between g_content_type_get_icon() and this, since the bulk of the function seems to be the same. Maybe you can factor out a get_icon_internal private method? ::: gio/glocalfileinfo.c @@ +1695,2 @@ } } Could get_icon_name() become get_gicon() and all these condition checks be moved into that shared function? (since they appear to be basically the same for symbolic and non-symbolic cases).
Review of attachment 222708 [details] [review]: ::: gio/gunixmounts.c @@ +1936,2 @@ else + icon_name = use_symbolic ? "media-removable-symbolic" : "media-flash"; Jakub didn't want to add that. ::: gio/gwin32mount.c @@ +204,3 @@ + case DRIVE_REMOTE : return use_symbolic ? "folder-remote-symbolic" : "folder-remote"; + case DRIVE_CDROM : return use_symbolic ? "drive-optical-symbolic" : "drive-optical"; + default : return use_symbolic ? "folder-symbolic" : "folder"; Yeah those names made no sense anyway. I think we should use the correct names and copy them into the build for Win32 if we think that is necessary. But I'd rather do that separately from this patch since their is no regression here since those icons don't exist already.
Created attachment 222814 [details] [review] Add ability to get symbolic icon for content type
Review of attachment 222814 [details] [review]: Looks good to me
Review of attachment 222708 [details] [review]: This looks good to commit then, with the following fix ::: docs/reference/gio/gio-sections.txt @@ +1131,3 @@ g_drive_get_name g_drive_get_icon +g_drive_get_symbolic_icon Should also add the new methods for g_unix_mount* here
Review of attachment 222709 [details] [review]: Good to commit, with the following fixes ::: gio/gfileinfo.h @@ +172,3 @@ + * Since: 2.34 + **/ + * Corresponding #GFileAttributeType is %G_FILE_ATTRIBUTE_TYPE_OBJECT. Should also add this, and the two methods below to gio-sections.txt ::: gio/gio.symbols @@ +423,3 @@ g_file_info_get_edit_name g_file_info_get_icon +g_file_info_get_symbolic_icon Missing _set_symbolic_icon here
Review of attachment 222814 [details] [review]: Good to commit with the following fix ::: gio/gio.symbols @@ +162,3 @@ g_content_type_get_mime_type g_content_type_get_icon +g_content_type_get_symbolic_icon Should also add this to gio-sections.txt
Created attachment 222880 [details] [review] Add symbolic icon support to drive, volume, and mount We need symbolic icon support for display in Nautilus.
Created attachment 222881 [details] [review] Add symbolic icon support to gfileinfo
Created attachment 222882 [details] [review] Add ability to get symbolic icon for content type
Review of attachment 222880 [details] [review]: ++
Review of attachment 222881 [details] [review]: ++
Review of attachment 222882 [details] [review]: ++
Attachment 222880 [details] pushed as a2dca48 - Add symbolic icon support to drive, volume, and mount Attachment 222881 [details] pushed as a15a071 - Add symbolic icon support to gfileinfo Attachment 222882 [details] pushed as 40b4fae - Add ability to get symbolic icon for content type
These patches do not appear to include any changes to monitor/udisks2/*.c ... can anyone tell me what the symbolic icons for G{Drive,Volume,Mount} instances for e.g. USB sticks and hard disks are? (gvfs-mount(1) output, please)
(In reply to comment #21) > These patches do not appear to include any changes to monitor/udisks2/*.c ... > can anyone tell me what the symbolic icons for G{Drive,Volume,Mount} instances > for e.g. USB sticks and hard disks are? (gvfs-mount(1) output, please) Gathered on IRC that this will trigger the fallback in the interface wrappers. Good enough for now, we can always implement this in the udisks2 volume monitor if needed. Thanks.