Hello! PCMan has written on Saturday, 26 November, at 22:52: >On Sat, Nov 26, 2016 at 9:48 PM, Andrej N. Gritsenko <and...@rep.kiev.ua> >wrote: >> You have written on Saturday, 26 November, at 21:02: >>>On Sat, Nov 26, 2016 at 6:52 PM, Andrej N. Gritsenko <and...@rep.kiev.ua> >>>wrote: >> >>>> I have an objection on including new features and string changes into >>>> libfm 1.2.x release, new features (especially those that produce new >>>> issues, as it is with emblems feature) should come into next release, >> >>>The emblem change is backward compatible and there are no UI string or >>>ABI changes. >>>It's safe to include that in the current release. >>>If you have concerns, it's still safe to just cherry-pick that commit. >> >> Yes, I have concerns, as that change: >> 1) changes libfm behavior on local files (it will now query GVFS for >> extra data, that was never done before);
>The query is done inside the gvfs "client" module, not the gvfs daemon. >https://git.gnome.org/browse/gvfs/tree/client/gdaemonfileenumerator.c#n314 >What it does is just like other parts of gio itself. If gvfs is not >installed, this won't be called. >The gvfs "metadata" implementation uses in-memory cache and binary >file format which is mapped into memory using mmap. >Examine the code here yourself. https://git.gnome.org/browse/gvfs/tree/metadata >So reading from the metadata is not time-consuming since it's an >in-process, in-memory operation. Well, the cache isn't filled itself, like a magic, it is always filled on first access. Otherwise it would not have a sense to have FmJob things at all, if all file infos were cached in system, right? :) So that mentioned thing will be also time consuming, at least on first access. >Writing to the data is more time-consuming, but we don't do it so it's >not an issue. It is done by underlaying glib functions, so anyway it is, just hidden from us. >> 2) produces an issue https://github.com/lxde/pcmanfm-qt/issues/397 which >Are you sure that the bug is introduced by the emblem support? >What I did is just replacing the GIcon of the file info with an emblemed icon. Not just replacing but also creating GFile and then quering that info from gio's VFS layer, to be exact. :) >How can this simple operation introduce such bug? >If the bug is indeed triggered by the commit, then that means there's >a bug somewhere in the underlying libfm which is trigerred. You might forget it all is done in a thread so it might require revision of underlaying processes. Anyway, bug is triggered by that commit so unless it is reliably fixed (not workarounded) end ensured that there are no additional gvfs interactions unless explicitly requested, it is not suitable for 1.2.x releases. Note that I'm speaking only about official stable release, GIT master would and should have all and any improvements (which are classified unstable until next feature release). >> I would classify as a blocker and https://github.com/lxde/libfm/pull/19 >> is not even a workaround but a dirty hack, as it isn't thread-safe >> while rest of that class APIs are (and should be). >> >> So in short, that change isn't suitable for 1.2.5, I'm sorry. In 1.3.0 I >> plan to add fm_enable_features() call so this and similar time-consuming >> features can be individually enabled, so it's where this change really >> belongs. And also I had a little bit different approach for this feature, >> not an enforsed (not every widget would want emblemed icon, e.g. menu). >This emblem support only touch file icons and does not affect menu icons. I'm sorry to tell you but you are wrong - LXPanel menu uses FmFileInfo, so it is already affected by your change. With best regards, Andriy. ------------------------------------------------------------------------------ _______________________________________________ Lxde-list mailing list Lxde-list@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxde-list
