Code review comment for lp:~mhr3/unity-lens-files/fix-841847

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

> Tested against Unity trunk and works very well.
>
> 460 +/*
> 461 private string get_current_type ()
> 462 {
> 463 - /* Get the current type to filter by */
> 464 + // Get the current type to filter by
> 465 var filter = scope.get_filter ("type") as RadioOptionFilter;
> 466 Unity.FilterOption? option = filter.get_active_option ();
> 467 return option == null ? "all" : option.id;
> 468 }
> 469 -
> 470 +*/
>
> Any specific reason to keep this in?
>

I kept it so if design changed its mind we could revert back to using it. OTOH we could just use the current functions and they'd return single element arrays - I'll remove it.

> Apart from the that the code looks great.
>
> On the behavior: One change from the current ways is that the Folders category
> now lists recent folders after the Gtk bookmarks. Is that intentional? (fwiw,
> I found use of it already while testing this branch - so if it wasn't on
> purpose we might want to check with John if we can keep it... :-))

Yes, before this change the files lens wasn't really searching folders (unless you explicitly selected the Folder filter), so that got fixed.

« Back to merge proposal