I noticed several problems with the tree browser:
1. The green toolbar icon with my theme corresponds to a document icon but it should be an action icon <img width="479" alt="Screen Shot 2019-10-12 at 20 53 41" src="https://user-images.githubusercontent.com/713965/66708120-d9910200-ed4b-11e9...; 2. Expanded folder icon is wrong and corresponds to document open action icon instead of a file type icon <img width="479" alt="Screen Shot 2019-10-12 at 20 54 03" src="https://user-images.githubusercontent.com/713965/66708125-fcbbb180-ed4b-11e9...; 3. On MacOS folder icons are themed differently than e.g. ProjectOrganizer or File browser plugins.
This pull request addresses these issues and makes some cleanups of the code in addition. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany-plugins/pull/927
-- Commit Summary --
* treebrowser: Use GIcon instead of pixbuf for tree icons * treebrowser: Use better icons * treebrowser: introduce a helper function and eliminate some #if's
-- File Changes --
M treebrowser/src/treebrowser.c (192)
-- Patch Links --
https://github.com/geany/geany-plugins/pull/927.patch https://github.com/geany/geany-plugins/pull/927.diff
LarsGit223 requested changes on this pull request.
Apart from the two little remarks, it looks good to me.
- GtkIconSet *icon_set;
- - icon_set = gtk_icon_factory_lookup_default(stock_id); - - if (icon_set) - return gtk_icon_set_render_icon(icon_set, gtk_widget_get_default_style(), - gtk_widget_get_default_direction(), - GTK_STATE_NORMAL, GTK_ICON_SIZE_MENU, NULL, NULL); - return NULL; -} -#endif - -static GdkPixbuf * -utils_pixbuf_from_path(gchar *path) +static GIcon * +utils_gicon_from_path(gchar *path)
The parameter path could be const.
ctype = g_content_type_guess(path, NULL, 0, NULL); icon = g_content_type_get_icon(ctype); g_free(ctype);
if (icon != NULL) { - gtk_icon_size_lookup(GTK_ICON_SIZE_MENU, &width, NULL); - info = gtk_icon_theme_lookup_by_gicon(gtk_icon_theme_get_default(), icon, width, GTK_ICON_LOOKUP_USE_BUILTIN); - g_object_unref(icon); + info = gtk_icon_theme_lookup_by_gicon(gtk_icon_theme_get_default(), icon, 16, 0);
What is usually used for icon sizes? You entered 16 but what about using a gtk constant like before?
techee commented on this pull request.
ctype = g_content_type_guess(path, NULL, 0, NULL); icon = g_content_type_get_icon(ctype); g_free(ctype);
if (icon != NULL) { - gtk_icon_size_lookup(GTK_ICON_SIZE_MENU, &width, NULL); - info = gtk_icon_theme_lookup_by_gicon(gtk_icon_theme_get_default(), icon, width, GTK_ICON_LOOKUP_USE_BUILTIN); - g_object_unref(icon); + info = gtk_icon_theme_lookup_by_gicon(gtk_icon_theme_get_default(), icon, 16, 0);
Notice that the constant GTK_ICON_LOOKUP_USE_BUILTIN was used for the last parameter which is flags - I believe this is why the icons looked differently on macOS so I used 0.
The width was determined by https://developer.gnome.org/gtk3/stable/gtk3-Themeable-Stock-Images.html#gtk... but based on the documentation this function shouldn't be needed and GTK_ICON_SIZE_MENU is 16px. I use the same in ProjectOrganizer and it seems to work fine.
techee commented on this pull request.
- GtkIconSet *icon_set;
- - icon_set = gtk_icon_factory_lookup_default(stock_id); - - if (icon_set) - return gtk_icon_set_render_icon(icon_set, gtk_widget_get_default_style(), - gtk_widget_get_default_direction(), - GTK_STATE_NORMAL, GTK_ICON_SIZE_MENU, NULL, NULL); - return NULL; -} -#endif - -static GdkPixbuf * -utils_pixbuf_from_path(gchar *path) +static GIcon * +utils_gicon_from_path(gchar *path)
OK, will do.
@techee pushed 3 commits.
e6c1df585ec5c38aa02db2f3b0796707476f1f32 treebrowser: Use GIcon instead of pixbuf for tree icons 99ef6527ed6a93c6b799ea6a6b110b445963f63d treebrowser: Use better icons ab1172ac79e369a5e450eaf8b93bb99eec765a72 treebrowser: introduce a helper function and eliminate some #if's
LarsGit223 commented on this pull request.
ctype = g_content_type_guess(path, NULL, 0, NULL); icon = g_content_type_get_icon(ctype); g_free(ctype);
if (icon != NULL) { - gtk_icon_size_lookup(GTK_ICON_SIZE_MENU, &width, NULL); - info = gtk_icon_theme_lookup_by_gicon(gtk_icon_theme_get_default(), icon, width, GTK_ICON_LOOKUP_USE_BUILTIN); - g_object_unref(icon); + info = gtk_icon_theme_lookup_by_gicon(gtk_icon_theme_get_default(), icon, 16, 0);
Ok, then we leave it as-is. I didn't doubt that it works I just wondered if there is a coding style for stuff like this which says 'use constants or literals.
@techee pushed 3 commits.
ccd4e1c9b389d2b0e5f8c07e9788784ea99f227f treebrowser: don't use ui_image_menu_item_new() 95929a531824398b24b4f7505bc274c1ef3f57ad treebrowser: don't use gtk_tool_button_new_from_stock() 84a58b226e75f4cadd6f086de681e81983164abc treebrowser: use slightly better icons for some actions
I've added a few more commits because icons weren't shown in the popup menu on GTK3 - see the new commits for more info.
github-comments@lists.geany.org