Hello!
PCMan has written on Tuesday, 18 September, at 10:49:
>On Tue, Sep 18, 2012 at 8:19 AM, Andrej N. Gritsenko <andrej@...> wrote:
>> Have a look into 'search-vfs' branch, please. I've made a VFS draft
>> out of FmSearchJob class which will be eliminated now together with all
>> extra functions in FmDirListJob that you've added to support it. All the
>> work will be done by unmodified(!) FmDirListJob. And other search support
>> in FmFolder and FmPath should be reverted back as well, of course. It's a
>> draft still since I just rearranged it but never tried yet to compile.
>> But please, don't merge all those branches together yet as it will be
>> complete mess. I'll do that in designated order carefully after 1.0.1 is
>> released. Thank you!
I've sent it in private since that branch is just a draft, raw and
never compiled. But if you want to continue in ML then OK. :)
>Generally it looks OK but it's slightly more complicated than the
>original design.
It's not more complicated really, it only looks so since it have all
handlers for GFile interface that return "Not supported" and that takes
space of course. It's even simpler in some ways since it doesn't use any
other conversions into libfm data and back (which are used in FmSearchJob
design) - just look into diff and you'll see more lines deletions than
lines additions in the working code itself. And also it certainly _is_
reenterable where FmSearchJob may be not (you know it uses Glib main loop
internal locks a lot). And also it doesn't use internal data from inside
of FmDirListJob (which looks still dirty, I'm sorry).
>Another problem is, attributes passed by the caller of
>g_file_enumerate_children may not
>contain file attributes required by the search and could break it.
>I still have some work in progress on my branch now and will commit them later.
Attributes passed by the caller of g_file_enumerate_children in the
search:// VFS will _always_ contain attributes passed from FmDirListJob.
In FmSearchJob implementation you took them from FmDirListJob illegally
and that is dirty and you know that yourself.
>Another issue is, I haven't optimize FmSearchJob now.
>The best way to do the file enumeration is to do it without
>"standard::content-type" attribute.
>When the parameters received by FmSearchJob only does file matching by
>names, there is no need to do mime-type recognition.
>Mime type recognition is only needed when a file matching our rules is found.
>Eliminating unneeded mime-type recognition should greatly speed up the search.
>This optimization, however, is not possible with your new API.
Why not? The VFS API is just an interface and you can do anything
with it. You can add test if URI is search and pass another list of
attributes to g_file_enumerate_children() inside of FmDirListJob so VFS
will apply them recursively as requested.
>The newly added APIs to FmDirListJob should NOT be removed.
>It's also used by FmDirList itself and does much work to optimize
>signal emission internally.
>If you emit the signals too frequently, the UI will flicker and become slow.
>That's why I group the newly added files and emit the signals periodically.
It may be some misunderstanding here though. I meant the APIs in it
such as fm_dir_list_job_set_dir_info(). I just saw them as a hack to
allow FmSearchJob mimic a FmDirListJob. If they not then let them stay. I
just don't see real purpose for them and I'm sorry for hasty assumptions.
>Please also check "search" branch of pcmanfm as well, it uses some new
>APIs in libfm search branch already.
I've looked into it already and yes, it uses "incremental" attribute
which we discussed already - it isn't related to FmSearchJob you know. :)
>I understand your desire to unify programming interfaces as I want to
>do that, too.
Not only unify but simplify - instead of using hacks and tricks for
some scheme in many places it does all it in one place.
>However, some hard-coded things are needed for making the UI better.
>For example, the UI should be consistent among most operations, but
>should have some differences for different operations, too.
>For applications:// folders, it's reasonable to add a "uninstall"
>option to the popup menu.
Of course, some hardcoded things are inevitable. But this one that
you mention have no relation to implementation of menu:// folder since
it's GUI-related-only thing and nothing in libfm will help.
Remember, I meant to exclude hardcoded things which are dependent on
folder implementations, not GUI extensions for some kinds of files. The
latter is fine for me.
>For search:// folders, it's reasonable to add a column showing
>location of the files.
That isn't related to search at all but it's about details of the
detailed view since it's always possible to get path of any file in the
view, it's just not included yet in the view you know. And this is GUI
related thing again.
>Though a generic UI works well with a unified API, some special UIs
>are needed for different cases to make the program more user-friendly.
>So there must still be some hard-coded things to check types of
>folders and provide best GUIs for each of them.
Of course it's inevitable sometimes (just count checks for trash: in
libfm code for example) but I want to stop all the implementation being
scattered over libfm but get it gathered in one place where it can be
maintained. It's a hell to find a problem if some thing is hardcoded in
ten of places. Some of those places may even conflict with other things
so require workarounds. That is dirty and not good you know. I want the
code be simpler and easier to understand.
>Thanks for the hard work, but I think before widely adopting your new
>design, some discussions are still needed.
>See the "attributes" problems and the lack of optimization I mentioned
>previously.
>Do you have a better idea for it?
If you want some optimizations then let's make them. The VFS variant
isn't worse than FmJob variant - it just uses another interface but the
same workflow for search and representation into libfm objects. And the
mentioned problem isn't a problem but just an implementation question.
The FmSearchJob _will_ have problems in the future when you'll want to
extend interface - you'll have to make adaptations everywhere what isn't
good at all. VFS solution is much more extendible as it doesn't depend on
FmJob abilities and API but only on GIO API. The less of cross-API calls
we have the simpler and better understandable the code is and you know
that yourself. And also the FmFile extension can add any (optional!)
functionality concerning of any path-like object without any dirty hack
in libfm code. Otherwise you should add a lot of hacks and workarounds
(as it's done for the trash: for example). I hate that very much. I'm
sorry.
Andriy.
|