Merge lp://qastaging/~dandrader/unity/lp940612 into lp://qastaging/unity

Proposed by Daniel d'Andrada
Status: Superseded
Proposed branch: lp://qastaging/~dandrader/unity/lp940612
Merge into: lp://qastaging/unity
Diff against target: 232 lines (+45/-61)
4 files modified
plugins/unityshell/src/GeisAdapter.cpp (+10/-10)
plugins/unityshell/src/GeisAdapter.h (+10/-10)
plugins/unityshell/src/GestureEngine.cpp (+24/-40)
plugins/unityshell/src/GestureEngine.h (+1/-1)
To merge this branch: bzr merge lp://qastaging/~dandrader/unity/lp940612
Reviewer Review Type Date Requested Status
Tim Penhey (community) Needs Fixing
Review via email: mp+99726@code.qastaging.launchpad.net

This proposal has been superseded by a proposal from 2012-03-29.

Description of the change

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

Please keep the variable type and variable name separate:
  const CompWindowVector &client_list_stacking
should be
  CompWindowVector const& client_list_stacking
(or
  const CompWindowVector& client_list_stacking
   - but my preference is for the first)

> if (client_list_stacking.size() == 0)

can just be:

  if (client_list_stacking.empty())

> int pos_x = (int) fpos_x;

Please don't use C style casts. It isn't needed here (mostly
certain about that).

The do/while loop misses the first element of the vector if there
is more than one element.

A traditional for loop is more idiomatic and understandable.
Also, can remove the window and result temporaries from outside the loop.
And... if using the iterators, you don't need the empty check first
as the initial check makes sure of that too.

for (auto iter = client_list_stacking.rbegin(),
          end = client_list_stacking.rend();
     iter != end; ++iter)
{
    CompWindow* window = *iter;

    if (pos_x >= window->x() && pos_x <= (window->width() + window->x())
        &&
        pos_y >= window->y() && pos_y <= (window->height() + window->y()))
      return window;
}

return nullptr;

review: Needs Fixing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.