Unity

Merge lp://qastaging/~azzar1/unity/fix-850984 into lp://qastaging/unity

fix-850984
Merge into trunk

Proposed by Andrea Azzarone on 2012-03-09

Status:

Merged

Approved by:

Andrea Azzarone on 2012-03-12

Approved revision:

no longer in the source branch.

Merged at revision:

2085

Proposed branch:

lp://qastaging/~azzar1/unity/fix-850984

Merge into:

lp://qastaging/unity

Diff against target:

475 lines (+329/-28)

9 files modified

plugins/unityshell/src/AbstractPlacesGroup.cpp (+44/-0)
plugins/unityshell/src/AbstractPlacesGroup.h (+51/-0)
plugins/unityshell/src/LensView.cpp (+10/-12)
plugins/unityshell/src/LensViewPrivate.cpp (+48/-0)
plugins/unityshell/src/LensViewPrivate.h (+40/-0)
plugins/unityshell/src/PlacesGroup.cpp (+3/-12)
plugins/unityshell/src/PlacesGroup.h (+3/-4)
tests/CMakeLists.txt (+5/-0)
tests/test_lensview_impl.cpp (+125/-0)

To merge this branch:

bzr merge lp://qastaging/~azzar1/unity/fix-850984

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Marco Trevisan (Treviño)		2012-03-09	Approve on 2012-03-10
Review via email: mp+96843@code.qastaging.launchpad.net

Description of the change

The problem
===
Dash - Missing category separator line in dash

The fix
===

17 - bool found_one = false;
18 -
19 - for (rit = children.rbegin(); rit != children.rend(); ++rit)
20 - {
21 - PlacesGroup* group = static_cast<PlacesGroup*>(*rit);
22 -
23 - if (group->IsVisible())
24 - group->SetDrawSeparator(found_one);
25 -
26 - found_one = group->IsVisible();
27 - }

If the next group is not visibile the separator is not shown in the prev group.
This branch fixes it moving the logic side in another (testable) function. I'd love to
remove the static_cast too but I can't find an easy way right now.

The test
===
Adds a unit test for the logic side.

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2012-03-10:

Overall looks very good to me.

133 + std::list<AbstractPlacesGroup*> groups;
134 +
135 + std::transform(children.begin(), children.end(), std::back_inserter(groups),
136 + [](Area* obj) -> AbstractPlacesGroup*
137 + {
138 + return static_cast<AbstractPlacesGroup*>(obj);
139 + });

I this really much more optimized, than using a normal loop? I mean, it's a nice C++ code, but it could look more cryptic than it really is :)

review: Approve

Revision history for this message

Andrea Azzarone (azzar1) wrote on 2012-03-10:

> Overall looks very good to me.
>
> 133 + std::list<AbstractPlacesGroup*> groups;
> 134 +
> 135 + std::transform(children.begin(), children.end(),
> std::back_inserter(groups),
> 136 + [](Area* obj) -> AbstractPlacesGroup*
> 137 + {
> 138 + return static_cast<AbstractPlacesGroup*>(obj);
> 139 + });
>
> I this really much more optimized, than using a normal loop? I mean, it's a
> nice C++ code, but it could look more cryptic than it really is :)