Created attachment 7856 patch Please could you take a look to the attached patch that adds "folder.jpg" support in Thunar. Also posting a screenshot. I believe it's useful to manage a music collection, and to easily customize folders icons... patch must be improved to match Thunar code quality policy, and take .folder.jpg, png... etc.. as xfdesktop already does. All the best :) JP
Created attachment 7857 screenshot
Thanks for the contribution, looks like a nice feature to me ! As Andre MIranda already told in the mail: - Lets check if there is no standard already on how to do this - Lets check how other file managers implement it - Lets make the file hidden
Here's the xfdesktop way (xfdesktop-regular-file-icon.c) : /* builds a folder/file path and then tests if that file is a valid image. * returns the file location if it does, NULL if it doesn't */ static gchar * xfdesktop_check_file_is_valid(const gchar *folder, const gchar *file) { gchar *path = g_strconcat(folder, "/", file, NULL); if(gdk_pixbuf_get_file_info(path, NULL, NULL) == NULL) { g_free(path); path = NULL; } return path; } static gchar * xfdesktop_load_icon_location_from_folder(XfdesktopFileIcon *icon) { gchar *icon_file = g_file_get_path(xfdesktop_file_icon_peek_file(icon)); gchar *path; g_return_val_if_fail(icon_file, NULL); /* So much for standards */ path = xfdesktop_check_file_is_valid(icon_file, "Folder.jpg"); if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "folder.jpg"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "Folder.JPG"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "folder.JPG"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "folder.jpeg"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "folder.JPEG"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "Folder.JPEG"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "Folder.jpeg"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "Cover.jpg"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "cover.jpg"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "Cover.jpeg"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "cover.jpeg"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "albumart.jpg"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "albumart.jpeg"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "fanart.jpg"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "Fanart.jpg"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "fanart.JPG"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "Fanart.JPG"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "FANART.JPG"); } if(path == NULL) { path = xfdesktop_check_file_is_valid(icon_file, "FANART.jpg"); } g_free(icon_file); /* the file *should* already be a thumbnail */ return path; }
Created attachment 7859 Thunar as Music Library Manager An exemple with Thunar as Music Library Manager : look at the shortcuts on the left ;)
Created attachment 7860 image If I may make a couple of recommendations: 1. Add png support so that we can also have transparency in case we want to do something like attached. 2. Add hidden file support so that the file is not visible in the folder if hidden files are not shown (e.g. .folder.png). Note: If we commit https://bugzilla.xfce.org/show_bug.cgi?id=12041 to tumbler, we'll also get album image art for audio files as well.
Created attachment 7861 more serious patch attached a more serious patch with support for png and hidden .folder.xxx files. Feature can be disabled with the "disable thumbnails" setting maybe worth a commit ... jp
Created attachment 7862 backtrace I just took a look into nautilus, nemo and dolphin, which all already have this feature. IMO the GUI approach there to select a custom icon is very self-explaining and natural, and I think we should do the same. You just have to open the "properties" dialog and click the folder icon there to select a custom icon. What I am missing on all 3 file managers: simple way to reset icon to default. So far I did not look into the related code, I dont know yet where the icon-path per folder is stored, seems to be no hidden file in the folder. Will check the bugtrackers and the git histories to see how they did it. IMO it would be great if e.g. a switch from nautilus/nemo/dolphin to thunar would preserve the custom folder icons ! Regarding your patch, something seems to be wrong , I get a seg.fault from it, a second right after starting thunar (applied patch on current master ) ... attached the backtrace.
Just one thing I forgot: For dolphin the feature is only available for folders. However nautilus/nemo allow to have custom icons for any single file. IMO it would not hurt to allow custom icons for every file.
It's not the same feature : having folder.jpg icons inside the directories allows to deal with music libraries : if you have 2000 directories (1 per album), each one with a cover artwork : setting custom images manually is not an option ;) You can add an option to set custom icons manually, but still the folder.jpg is another feature.
I am not saying that setting the icon manually should be the only option. Just think it should be the default option. But you are right that this is only a nice add .. .the cherry on the cake. Primary goal should be to allow custom-icons at all. Meanwhile I looked a bit further into the nautilus way of doing things. In "nautilus-file.c" there is a method "get_custom_icon(..)". A bit more digging reveals that nautilus uses the metadata of gvfs( https://en.wikipedia.org/wiki/GVfs or https://wiki.gnome.org/Projects/gvfs ) to store the path to the custom icon. I think that is a very nice approach. Thunar anyhow relies on gvfs if it comes to trash-support, removable media or remote file systems .. so IMO good to use the same standard to save file-specific meta information. You can read all metadata e.g with "gvfs-info -a metadata::*". After setting a custom-icon with nautilus it is shown there. And there is "gvfs-set-attribute", so I guess your use-case should be covered, you can script it for huge folder-trees. So I would strongly prefer to use gvfs to store file-specific metadata over using folder.jpg
Created attachment 7864 new patch, shouldn't segfault new patch, can't reproduce segfault here, so I g_freed some gchar just in case... :/ some help ?
thanks, seems like you fixed it, now the patch works fine for me. Some further improvement suggestions for your patch to prevent memory leaking: - the return value of g_strconcat always need to be freed. Currently there is the risk that only one string is freed (if thunar_icon_factory_check_folder_icon returns on the last "if" statement) - as well when thunar_icon_factory_check_folder_icon return NULL, the srings build by g_strconcat need to be freed ( or when icon == NULL ) If you add a new patch to the issue which replaces old patches, you can select all your older patches in the "Obsoletes:" list, to mark them as not valid any more. However as said above, I would prefer a patch which makes use of gvfs metadata. We could recycle most of the nautilus code for that I think.
Regarding the feature in dolphin: The dolphin solution is more like the way you propose: A file called ".directory" is added to a folder which stores the path to the used image. here an example: > [Desktop Entry] > Icon=/home/schwinn/Bilder/misc/me/kopf.png Pro: It is not required to add another gvfs dependency Con: Only possibly to have custom icons for folders
Thanks for the tips, I will improve the code in this way. About having custom icons : I believe that it's really useful specially for folders, and a bit less interesting for regular files which already have mimetypes, thumbnails, and emblems.. My initial goal was to support natively music collection albums covers, so that Thunar could be used as a music library manager (uca allows to add enqueue/play right click action on album ( audacious --play --enqueue-to-temp %f/* ). All the best jp
Created attachment 7866 improved patch (memory cleaning)
Comment on attachment 7866 improved patch (memory cleaning) >diff -rNaud Thunar-1.8.1-OLD/thunar/thunar-icon-factory.c Thunar-1.8.1/thunar/thunar-icon-factory.c >--- Thunar-1.8.1-OLD/thunar/thunar-icon-factory.c 2018-06-12 03:47:24.000000000 +0200 >+++ Thunar-1.8.1/thunar/thunar-icon-factory.c 2018-08-06 00:42:05.485135360 +0200 >@@ -792,7 +792,50 @@ > return thunar_icon_factory_lookup_icon (factory, name, size, wants_default); > } > >- >+/** >+ * thunar_icon_factory_check_folder_icon: >+ * @file : a #ThunarFile. >+ * >+ * Return value: gchar pointer containing full path to >+ * folder.xxx image or NULL if no image found. >+ **/ >+gchar * >+thunar_icon_factory_check_folder_icon ( ThunarFile *file) >+{ >+ >+ gchar *folder_icon; >+ gchar *path; >+ >+ path = g_file_get_path (thunar_file_get_file (file)); >+ >+ folder_icon = g_strconcat (path, "/", ".folder.jpg", NULL); >+ if ( g_file_test(folder_icon, G_FILE_TEST_EXISTS)) >+ { >+ g_free (path); >+ return folder_icon; >+ } >+ folder_icon = g_strconcat (path, "/", "folder.jpg", NULL); >+ if ( g_file_test(folder_icon, G_FILE_TEST_EXISTS)) >+ { >+ g_free (path); >+ return folder_icon; >+ } >+ folder_icon = g_strconcat (path, "/", ".folder.png", NULL); >+ if ( g_file_test(folder_icon, G_FILE_TEST_EXISTS)) >+ { >+ g_free (path); >+ return folder_icon; >+ } >+ folder_icon = g_strconcat (path, "/", "folder.png", NULL); >+ if ( g_file_test(folder_icon, G_FILE_TEST_EXISTS)) >+ { >+ g_free (path); >+ return folder_icon; >+ } >+ g_free (path); >+ folder_icon = NULL; >+ return folder_icon; >+} > > /** > * thunar_icon_factory_load_file_icon: >@@ -819,8 +862,9 @@ > GIcon *gicon; > const gchar *icon_name; > const gchar *custom_icon; >+ const gchar *folder_icon; > ThunarIconStore *store; >- >+ > _thunar_return_val_if_fail (THUNAR_IS_ICON_FACTORY (factory), NULL); > _thunar_return_val_if_fail (THUNAR_IS_FILE (file), NULL); > _thunar_return_val_if_fail (icon_size > 0, NULL); >@@ -836,7 +880,7 @@ > return g_object_ref (store->icon); > } > >- /* check if we have a custom icon for this file */ >+ /* check if we have a custom icon for this file */ > custom_icon = thunar_file_get_custom_icon (file); > if (custom_icon != NULL) > { >@@ -846,6 +890,21 @@ > return icon; > } > >+ /* check if thumbnails are enabled and we have a "?folder.xxx" icon for this folder */ >+ if (thunar_icon_factory_get_show_thumbnail (factory, file) >+ && thunar_file_is_directory (file)) >+ { >+ folder_icon = thunar_icon_factory_check_folder_icon (file); >+ if (folder_icon != NULL) >+ { >+ /* try to load the icon */ >+ icon = thunar_icon_factory_lookup_icon (factory, folder_icon, icon_size, FALSE); >+ g_free(folder_icon); >+ if (G_LIKELY (icon != NULL)) >+ return icon; >+ } >+ } >+ > /* check if thumbnails are enabled and we can display a thumbnail for the item */ > if (thunar_icon_factory_get_show_thumbnail (factory, file) > && thunar_file_is_regular (file))
Created attachment 7867 small improvement sorry for the spam, small mod to "always" gfree folder_icon ;)
Okay, late to the party, I'll to summarize my point of view after a couple of talks with Alexander: I like this feature, the way it was proposed is really simple, however since we are already using gvfs, optionally as a matter of fact, however I dare say the majority of Thunar users have gvfs installed (I might be very mistaken), thus probably it's not a good idea to introduce a new standard[1]. Alex's proposal is to use g_file_info_get_attribute_stringv as it's already used for emblems[2]. Then have an option in the folder's properties dialog to set/clear the path to an image. Power users could automate this with a script using gvfs-set-attribute or "gio set". All in all it's still quite simple, easy for users to discover and interop with Nautilus is a plus. By the way, I think this should be restricted to folders, it's not wise to freely change the icon of files, e.g. an executable that looks like a picture. Anyway, for that we already have .desktop files. @hyperion, does this approach satisfy your needs? Would you be kind enough to rework your patch with at least the chuck that reads the image path with g_file_info_get_attribute_stringv (please)? 1 - https://xkcd.com/927/ 2 - https://git.xfce.org/xfce/thunar/tree/thunar/thunar-file.c#n3366
Hi, Thanks for the feedback, before going further, let me clarify again the purpose of my patch : -> primary goal is to make Thunar behave the way XFDESKTOP already do for music folders you put on the desktop. "folder.jpg", or "cover.jpg" is a de facto standard for music collections : so IMHO a user should be able to see albums covers as folders icons without the need of setting anything : should be automatically detected. Being able to setup a folder custom icon path manually : is another feature ;) (and I like it) In the "folder.jpg" idea : path is always the same : folder.jpg inside a directory should be detected, just like XFDESKTOP do it actually. (btw : xfdesktop search for many more ***names***.jpg, and I chose to restrict to "folder.jpg" for speed and simplicity, and it's really simple to rename all cover.jpg to folder.jpg) Let me know your thoughts :) JP
Created attachment 7869 Another exemple of Thunar as a Music Library File Manager Another exemple of Thunar as a Music Library File Manager
or maybe as a thunar plugin ? (ie : "Thunar Music Library Plugin"
I just had another chat with Andre Miranda .. for some strange coincidence we both so far overlooked that xfdesktop already implements this feature :X Sorry for that , you already pointed out by your first post, but I failed to recognize :/ So actually this changed our opinion, we are now ok to support the feature the way you proposed. One concern is, that it will slow down loading for folder with many subfolders, specially on old hardware. If this should upset somebody, we could later introduce a toggle-switch for xfce4-setting-editor later. And we are still discussion if only the hidden ".folder.jpg" / ".folder.png" should supported, or as well the non-hidden ones. Just give me some time to properly review and test your patch.
Created attachment 7871 New patch with Gfile custom-icon metadata Not familiar with this API, but it seems that I managed to use Gfile metadata to store the folder icon as custom-icon... and it loads faster once it has been loaded once.. Tell me ... ;)
sorry for previous patch with bad API usage
Created attachment 7874 Attached the simplest and fastest patch so far ;) Attached the simplest and fastest patch so far ;)
Hi HYPERION, The latest patch looks very simple, so we may support both ways of loading folders images. I would like to point out a couple of things: - pay attention to indentation and white spaces, Xfce has no standard source format guidelines , but we strive follow the style the project is already using. - if possible, provide patches generated by git diff, or even better, make commits and use git format-patch so we can easily give you credits for the commit once merged. We appreciate your effort, the patch is getting better and better, just give us a few days so we can properly test and review it.
Created attachment 7879 correct indentation Hi Andre, Here's the patch with small memory improvement and correct indentation ;) sorry if GIT is not my cup of tee ... old school... also it seems that master is not building at the time, so my patch is on latest stable. All the best jp
Master build fine for me here .. did you try to build without the patch / a clean checkout ? Latest patch does not work for me, when applied to master: File to patch: thunar/thunar-icon-factory.c patching file thunar/thunar-icon-factory.c Hunk #1 FAILED at 847. Which commit is the latest stable for you ? Last modification of thunar-icon-factory.c on master was 11 days ago by this commit: https://git.xfce.org/xfce/thunar/commit/?id=95a5c2559f45f662e0a4c1826cf77f863286c46f .. line numbers changed, I guess you need to update your patch according to the changes.
latest stable = tag Thunar-1.8.1
Created attachment 7880 thunar-icon-factory.c (master) try to build master with this file ... (not tested) build fails here...
line 906 if (thunar_icon_factory_get_show_thumbnail (factory, file))
Created attachment 7883 Patch to apply to git Attached is a modified version of Hyperion's last patch that applies against thunar git. I made a few corrections to get it to work. I also removed the g_free of const gchar * that were added (as I understand it, you don't need to free const gchar). I'm sad to see that hidden files and png format have been removed. I think this is the way to go.
My last sentence is confusing. We should add support for hidden and png files.
It's a matter of performance impact : my second patch did it, but too much impact imho. We have to choose JP
1 remark : you added a test for custom_icon != NULL : custom_icon = g_strconcat (g_file_get_path (thunar_file_get_file (file)), "/", "folder.jpg", NULL); + if (custom_icon != NULL && g_file_test(custom_icon, G_FILE_TEST_EXISTS)) you can avoid testing custom_icon != NULL : custom_icon can't be NULL as g_strconcat can't fail and output is at least "/", "folder.jpg" JP
git contrib ;) https://github.com/H1p8r10n/thunar/commit/aeee54171efcfda194258860645a5dbf780d32c7
ps : I totally agree with the hidden ".folder.jpg" idea, instead of shown "folder.jpg" But : - png is not usual for cover thumbs - music albums are often provided with "folder.jpg" , never with ".folder.jpg"
@Hyperion, without the test for custom_icon, thunar was spitting out alot of "GLib-CRITICAL **: g_file_test: assertion 'filename != NULL' failed" messages similar to this bug report: https://bugzilla.xfce.org/show_bug.cgi?id=13663. The fix was similar.
ok :) clear !
github.com/H1p8r10n/thunar/commit/949d184590d18369747b9d63954c244a2004f457
Thanks alot for your work, the feature works fine for me when I compile your master branch ! A fork on github is nice ! Regarding the code, I will directly comment into the commit there. ( Hope we will have our new xfce infra soon, so we have official support for pull requests ) Regarding performance vs. multiple file-name support: How about a configuration option string, e.g. FOLDER_THUMBNAIL_FILES in xfce4-settings-editor ? We could have a string of supported file-names. Default could be "folder.jpg" ... but you may add others into the string, like "folder.jpg,cover.jpg,.folder.jpg". An example on how to add preferences can be found here: https://git.xfce.org/xfce/thunar/commit/?id=6e2b1d762ba177fba19e0bb4d466f5a1ed524df4 .. you will not need to add it to thunar-preferences-dialog.c , only to thunar-preferences.c is sufficient to see it in xfce4-settings-editor.
I need some help to understand the way exo_binding ; Object properties ; and all these API work. For the moment it's obscure, and unfortunately I have not enough time to investigate :/
Here some doc regarding gtk3 Object Properties: [0] .... not taht much, I did not find a better doc for it so far. Here is some doc. regarding exo bindings: [1] Think you will only need one exo_binding between the property "PROP_FOLDER_THUMBNAIL_FILES" of thunar-icon-factory.c and the "PROP_MISC_FOLDER_THUMBNAIL_FILES" of thunar-preferences.c Take your time, we are not in a hurry ;) [0] https://python-gtk-3-tutorial.readthedocs.io/en/latest/basics.html#properties [1] https://git.xfce.org/xfce/exo/tree/exo/exo-binding.c#n32
I think we are overthinking this feature, ["folder.png", ".folder.png", "folder.jpg", ".folder.jpg"] should be enough, not need to customize it via a hidden property. As performance is a concern, I guess an option to enable/disable it in preferences dialog wouldn't hurt, something like "Show custom icons for folders", a tooltip explaining the supported files and (maybe) the GFile Metadata (the latter has precedence over the former IMO). This could be disabled by default until a proper analysis is run to assess the performance hit, i.e. check with a profile, for instance gnome builder, for hot spots on thousands of folders with custom icons.
As it is now : the code is fast, simple, and already triggered by an option : enabling thumbnails : with a few lines of code the result is really great. Taking distance with the feature : it's just another kind of thumbnailing for music folders. The convention for music albums is to use "folder.jpg", and also some variants of "cover.jpg" (that can easily be renamed).... but never hidden, nor png files... This said : switching to hidden .folder.jpg doesn't hurt me... but it's less user friendly. JP
simple is beautiful !
ready for merge ? https://github.com/H1p8r10n/thunar/commit/d6d8d7c042bba84befe8f843957f9d1b8c290188
Yes, it is simple, but IMO no, that does not make it beautiful. And no, I would not call it fast, since, it will slow down thunar for all users who do not use the feature. - IMO it should be possible to disable a hidden feature which is just used by a minority of users, specially when it possibly adds slowness. - To enable it together with enabling thumbnails IMO is not a good solution. Thats not intuitive. It's just another hidden switch on top. And even worse, if I want to disable the feature, I need to quit using thumbnails. Give me some time, than I will provide a proper patch which adds a checkbox into the (advanced?)menu and which adds support for ["folder.png", ".folder.png", "folder.jpg", ".folder.jpg"] Since it would be opt in , we could even support cover.jpg / cover.png
ok :)
Here is my first shot: https://github.com/alexxcons/thunar/commit/50d1f709f7a2ccf3a5ecd42901095abce1e49c93 Please let me know if it would be fine for you like that. ( And I would be happy if you could check my grammar on the preference item and the tooltip :P )
The patch works. To enable the hidden setting: "xfconf-query -c thunar -p /misc-folder-custom-icons --create -t bool -s true". However, there is a noticeable impact when first loading a folder and a smaller performance impact when scrolling in a window with many folders (160 in my tests). The initial load of icons is understandable, but the scrolling is going to be problematic (it feels like each file is being re-read as opposed to cached somewhere) when off-screen or out of viewport. It is significant enough that the mouse and window scroll move what looks like independently (delayed) - hard to control. In addition, the "(thunar:4282): GLib-CRITICAL **: 19:31:49.169: g_file_test: assertion 'filename != NULL' failed" messages are back - but that can be easily fixed. Thanks for providing support for multiple files.
Scrolling results in thunar cpu usage increase to @30-40% while scrolling.
@ToZ I couldn't test the patch yet (looks simple and correct to me), but this is a considerable performance hit, we can't afford this even if the feature is opt-in. @alexxcons Now I think a tumbler plugin for mime type "inode/directory" makes more sense, tumbler was designed for this purpose, its asynchronous nature allows it to scale much better than the proposed approach. The API seems as easy as thunarx, I can put together a PoC, but not anytime soon, feel free go ahead of me if interested.
Agreed, tumbler is probably better for the purpose.
Thanks for testing all ! @ToZ - With this patch there is no need for xfconf-query. I added a checkbox for it in preferences-->display ... I used the tooltip to give info on the supported files-names ... We as well could make it hidden, than we could add the tooltip info into the Wiki. I dont have strong preference for one or the other. - Will try to test with your setup to reproduce the scroll-delay / CPU load. Which size did your used picture have ? jpg /png ? @All To use thumbnails instead of the file itself is a good idea I think. Do we really need a new plugin for that ? The thumbnailer will already provide thumbnails for all pics inside a folder, as well for folder.jpg, etc. The only problem: Afaik currently these thumbnails are loaded when the folder is opened. This would be too late for our purpose. If possible I would try to keep using the generic fd API for picture thumbnailers, not explicitly tumbler. Without looking into the related thunar-thumbnailer code my plan would be: 1. request thumbnails for folder.jpg, etc. already when the containing folder icon is requested 2. load + display the thumbnail when available + store the path of the thumbnail of interest in the related "ThunarFile" of the folder ( should be the member: thumbnail_path) 3. Only request a new thumbnail for the folder if e.g. cover.jpg is added When I have time I'll further read the thumbnailer part to see if it could be done like that.
Hi All, Tumbler has the capability of loading "xxxx.thumbnailer" files placed in the /usr/share/thumbnailers folder (aka Desktop Thumbnailer Plugin, see : plugin https://docs.xfce.org/xfce/thunar/tumbler ). This way I managed to make Thunar show folder icons, with a "/usr/share/thumbnailers/folder.thumbnailer" as follow : [Thumbnailer Entry] Version=1.0 Encoding=UTF-8 Type=X-Thumbnailer Name=Folder Thumbnailer MimeType=inode/directory; Exec=/usr/bin/convert -thumbnail %s %i/folder.jpg %o AND Small mod in thunar-thumbnailer.c (line 331) : - /* the icon factory only loads icons for regular files */ - if (!thunar_file_is_regular (lp->data)) + /* the icon factory only loads icons for regular files & folders */ + if (!thunar_file_is_regular (lp->data) && !thunar_file_is_directory (lp->data)) { thunar_file_set_thumb_state (lp->data, THUNAR_FILE_THUMB_STATE_NONE); continue; } AND Small mod in thunar-icon-factory.c (line 850) : - if (thunar_icon_factory_get_show_thumbnail (factory, file) - && thunar_file_is_regular (file)) - { + if (thunar_icon_factory_get_show_thumbnail (factory, file)) + { + if (thunar_file_is_regular (file) || thunar_file_is_directory (file)) Then it behaves like tumbler usually do for videos or images : slow at first run, and no lag at all once the thumbnailer cache is populated. JP
Created attachment 7895 just enable folder thumbnails just enable folder thumbnails in Thunar : thumbnailing itself is left on tumbler side.
Works fine for me. Congratz, looks like you found another nice solution ! :P Some minor thing: - Instead of adding another indention in this already huge and deep if/else block in thunar-icon-factory.c , code would be more readable when just adding an "or" into the condition: if (thunar_icon_factory_get_show_thumbnail (factory, file) && ( thunar_file_is_regular (file) || thunar_file_is_directory (file)) ) Some remaining things: - What if I want to recover my default folder icon ? Renaming folder.png seems to be not sufficient. Do I need to clear the thumbnail folder, or is there a more elegant way ? - Open ab bug on thumblr to add this into plugins ? - When released, add some Info about it to the thunar Wiki -
To recover the origin theme icon, two solutions, so far : 1) remove folder.jpg and then rename the folder twice (to be back to original name) 2) remove folder.jpg and then clear the thumbnail cache Not very user friendly... I know.. :/
@Alex : up to you to commit the small change to Thunar code ? ok ? I will send the folder.thumbnailer solution to tumbler team ;) JP
The performance is much better with this solution. Plus, I like the fact that you can specify the file to use for the folder icon (folder.jpg, .folder.png, etc) directly in the thumbnailer file. Reverting to the default icon might be an issue (if someone doesn't want to clear their whole cache for one folder and then deal with having it re-created). Renaming the folder twice doesn't seem to work for me. For other thumbnails, eg. images, the thumbnail is updated if the mtime (modification time?) of the file changes. Changing the mtime of the folder (touch -m) or changing the folder file itself doesn't seem to initiate the refresh. Perhaps something can be added to tumbler to watch for this as well?
@Toz : not watching for mtime for folders may be related to the way Thunar handles thumbnails... (maybe I forgot to remove some test statement, someone knows ?). @All Anyway : the icon rollback problem doesn't prevent Thunar team to commit the small change to support for folder thumbs : as long as there's no folder.thumbnailer Freedesktop file : no handler for inode/directory : no impact on Thunar. JP
To support more images files : "/usr/bin/folder-thumbnailer" as follow : #!/bin/bash convert -thumbnail "$1" "$2/folder.jpg" "$3" ||\ convert -thumbnail "$1" "$2/.folder.jpg" "$3" ||\ convert -thumbnail "$1" "$2/folder.png" "$3" ||\ convert -thumbnail "$1" "$2/cover.jpg" "$3" ||\ exit 1 AND "/usr/share/thumbnailers/folder.thumbnailer" as follow : [Thumbnailer Entry] Version=1.0 Encoding=UTF-8 Type=X-Thumbnailer Name=Folder Thumbnailer MimeType=inode/directory; Exec=/usr/bin/folder-thumbnailer %s %i %o
@HYPERION If there is a change that thunar is involved in the mtime problem, I would prefer to wait a bit with the commit. I guess there is a third if(..) which needs to get modified ( Trying to keep the number of commits per bug at a minimum )
@alex : I looked at the Thunar "file monitor" mechanism, and found nothing specific to folders. I believe that the problem of persistant thumb seems to come from the way tumbler desktop plugin works : once a thumbnail has been created : it doesn't handles updating it.
Created attachment 7899 patch @HYPERION I think I know what the problem is: For usual thumbnails, rename, move, etc works fine. A removal of the picture results in a removal of the thumbnail. For thumbnails of folders the situation is different. On first load, it looks like a copy of the "folder.jpg" thumbnail is saved with a different thumbnail-name. This copy is used for the folder. This copy is not updated on move / rename / delete. It will only be overwritten if "folder.jpg" is replaced by a different file. I guess this is a thumbnailer bug. It should be the job of the thumbnailer to as well update the thumbnail of the folder according to the file-thumbnail. (or possibly use the same thumbnail for both ) If there is no other objection, I would commit your fix to master and the 4.14 branch with the proposed small change. I added you as author, if that is ok for you (See attached patch) ?
Patch works well. @HYPERION, thank you very much for this contribution. This is something I've heard a lot of requests for.
@Alex, LGTM. I just wonder if this thumbnailer has to be 1) shipped by tumbler 2) shipped by distributions 3) created manually by users. Thank you all for patiently investigating and testing, specially HYPERION.
@All Seems that I found the reason for persistant thumbs, and a workaround :) The reason : each thumb file in the cache is named after the md5 hash of the URI of the thumbed object (here : the folder). folder change signal is handled correctly by Thunar and Tumbler on deletion of the folder.jpg (thumbed file), but the thumb is not refreshed because the thumbed file is note the same as the thumbed object. The workaround : "/usr/bin/folder-thumbnailer" as follow : #!/bin/bash convert -thumbnail "$1" "$2/folder.jpg" "$3" ||\ convert -thumbnail "$1" "$2/.folder.jpg" "$3" ||\ convert -thumbnail "$1" "$2/folder.png" "$3" ||\ convert -thumbnail "$1" "$2/cover.jpg" "$3" ||\ convert -thumbnail "$1" "/usr/share/icons/$(xfconf-query -c xsettings -p /Net/ThemeName)/128x128/places/folder.png" "$3" ||\ exit 1
@All This is cleaner, seems to work better, and if confirmed : is not a workaround anymore ;) "/usr/bin/folder-thumbnailer" as follow : #!/bin/bash convert -thumbnail "$1" "$2/folder.jpg" "$3" ||\ convert -thumbnail "$1" "$2/.folder.jpg" "$3" ||\ convert -thumbnail "$1" "$2/folder.png" "$3" ||\ convert -thumbnail "$1" "$2/cover.jpg" "$3" ||\ rm -f "$HOME/.cache/thumbnails/normal/$(echo -n "file://$2" | md5sum | cut -d " " -f1).png" ||\ rm -f "$HOME/.thumbnails/normal/$(echo -n "file://$2" | md5sum | cut -d " " -f1).png" ||\ rm -f "$HOME/.cache/thumbnails/large/$(echo -n "file://$2" | md5sum | cut -d " " -f1).png" ||\ rm -f "$HOME/.thumbnails/large/$(echo -n "file://$2" | md5sum | cut -d " " -f1).png" ||\ exit 1 @Andre : I think it could be provided in Tumbler @Alex : thanks for the patch, commit, and OK to be mentioned in authors list (my mail : h1p8r10n [[[AT]]] yandex [[[DOT]]] com )
HYPERION referenced this bugreport in commit 58b7da73dcee057ba4ed2756ee989c09c1e9c358 Add support for folder.jpg (Bug #14576) https://git.xfce.org/xfce/thunar/commit?id=58b7da73dcee057ba4ed2756ee989c09c1e9c358
HYPERION referenced this bugreport in commit 543c23b0f011dde5a571bca3525e80bb7e273561 Add support for folder.jpg (Bug #14576) https://git.xfce.org/xfce/thunar/commit?id=543c23b0f011dde5a571bca3525e80bb7e273561
Ok pushed the patch to both branches. Thanks alot for the fix, the the script and all the testing! For now I added the script and the config file into the thunar Wiki: https://docs.xfce.org/xfce/thunar/tumbler#customized_thumbnailers I am not sure if both files should be automatically provided by Tumbler or, shipped by distributions .. best open a new bug on Tumbler to further discuss it there. So I think finally this bug came to an end ;)
@alexxcons, the patch doesn't include the setting in the dialog. Is the intent to keep it a hidden setting now (which we should document), or will you push the dialog change as well? Also, I've found that the convert command in the script is quite noisy. I suggest appending "> /dev/null > 2&1" to each command to quiet it down in the script. I will make the edit to the wiki page for this.
@Toz OK for standard outputs redirects, thanks ;) About dialog : see my above : as long as the freedesktop plugin "folder.thumbnailer" is not in place : Tumbler has no handler for inode/directory : so no impact on Thunar. For now Thunar has an GUI option to enable thumbnails : IMHO it's enough : the folder thumbnailer is just another thumbnailer between the others. Anyway : I think that enabling / disabling certain thumbnailer plugins : should be handled by Tumbler. A small GUI is needed there. Just my 2 cents... JP
Something needs to be improved to handle folder names containing spaces : ________________________________________________________________ #!/bin/bash convert -thumbnail "$1" "$2/folder.jpg" "$3" ||\ convert -thumbnail "$1" "$2/.folder.jpg" "$3" ||\ convert -thumbnail "$1" "$2/folder.png" "$3" ||\ convert -thumbnail "$1" "$2/cover.jpg" "$3" ||\ rm -f "$HOME/.cache/thumbnails/normal/$(echo -n "$4" | md5sum | cut -d " " -f1).png" ||\ rm -f "$HOME/.thumbnails/normal/$(echo -n "$4" | md5sum | cut -d " " -f1).png" ||\ rm -f "$HOME/.cache/thumbnails/large/$(echo -n "$4" | md5sum | cut -d " " -f1).png" ||\ rm -f "$HOME/.thumbnails/large/$(echo -n "$4" | md5sum | cut -d " " -f1).png" ||\ exit 1 ________________________________________________________________ [Thumbnailer Entry] Version=1.0 Encoding=UTF-8 Type=X-Thumbnailer Name=Folder Thumbnailer MimeType=inode/directory; Exec=/usr/bin/folder-thumbnailer %s %i %o %u ________________________________________________________________ How could I get authoring access to the xfce wiki ?
#!/bin/bash convert -thumbnail "$1" "$2/folder.jpg" "$3" 1>/dev/null 2>&1 ||\ convert -thumbnail "$1" "$2/.folder.jpg" "$3" 1>/dev/null 2>&1 ||\ convert -thumbnail "$1" "$2/folder.png" "$3" 1>/dev/null 2>&1 ||\ convert -thumbnail "$1" "$2/cover.jpg" "$3" 1>/dev/null 2>&1 ||\ rm -f "$HOME/.cache/thumbnails/normal/$(echo -n "$4" | md5sum | cut -d " " -f1).png" ||\ rm -f "$HOME/.thumbnails/normal/$(echo -n "$4" | md5sum | cut -d " " -f1).png" ||\ rm -f "$HOME/.cache/thumbnails/large/$(echo -n "$4" | md5sum | cut -d " " -f1).png" ||\ rm -f "$HOME/.thumbnails/large/$(echo -n "$4" | md5sum | cut -d " " -f1).png" ||\ exit 1
@ToZ Regarding the preferences dialog I would agree with HYPERION. It's already configurable via the config file and the "show thumbnails" enum, should be sufficient. Even if we would provide a checkbox, in thunar we could not make sure that enabling it would have an effect. However I dont think we need tumbler a GUI for it, but that's a different story ;) @HYPERION ok, I replaced the WIki scripts. "exit 1" means ERROR, so I replaced it by "exit 0" = success
@Alex : I really meant ERROR : at this point of exit : all the tests have failed, so we have to inform the caller program that we failed ;) Not sure Tumbler uses the exit code from child thumbnailer anyway... JP
uh, ok ... changed back to 1.
Hi, Regression in tumbler 0.2.2 : folder thumbnails show and vanish :( JP
@HYPERION, see: https://bugzilla.xfce.org/show_bug.cgi?id=14693
*** Bug 3508 has been marked as a duplicate of this bug. ***