Code review comment for lp:~mhr3/unity-lens-files/folder-fixes

Revision history for this message
Michal Hruby (mhr3) wrote :

> I can confirm that it works :-) However, I'd like to refactor this a bit. The
> idea so far has been to keep all bookmarks logic contained in the Bookmarks
> class in folders.vala.
>
> I suggest that the filtering be moved into that class an automatically taken
> care of when you call bookmarks.prefix_search() or bookmarks.list().
>

Fair enough, I'll fix that.

> Also - there's no reason to do async IO here since we have no UI to block :-).
> In fact async IO is to be taken cautiously inside the lens daemons because it
> warrants the need for reentrancy safety for fast changing queries (the daemons
> protect somewhat against this at a higher level, but still).
>
> Otherwise good!

TBH, I'm not sure doing everything synchronously is such a good idea, true we do not have a UI to block, but we make the subsequent queries execute slower, imo the correct solution would be to add a Cancellable instance to each query and call cancel on it if the lens gets a new query. But perhaps this would be something for P.

« Back to merge proposal