> Hey MCR1. Excellent work, here's some comments so far.
> 
Hi Sam. First of all thanks for the review. It is very appreciated.

> 8       +    CompOption::Vector o(0);
> 9            dndWindow->ungrabNotify ();
> 10
> 11      +    screen->handleCompizEvent ("expo", "start_viewport_switch", o);
> 
> I believe there is a constant somewhere, either as a static member of
> CompOption or just as a global called "noOptions". I'd suggest using that.
>
I am not sure where to find that, but I'll search...
 
> 43      -       <_long>Warp and resize windows to fit an imaginary
> grid.</_long>
> 44      +       <_long>Warp and resize windows to fit an imaginary
> grid</_long>
> 45              <category>Window Management</category>
> 46              <deps>
> 47                  <requirement>
> 48      @@ -83,7 +83,7 @@
> 49                              <_long>Window resize action</_long>
> 50                              <option name="top_left_corner_action"
> type="int">
> 51                                  <_short>Upper Left Corner</_short>
> 52      -                           <_long>Action to be performed when window
> is dropped on the top left corner</_long>
> 53      +                           <_long>Action to be performed when window
> is dropped on the top left corner.</_long>
> 
> Try to avoid mixing formatting changes with substantive changes.
>
I am sorry - I just can't help myself. I want to harmonize how plugins and their
tooltips look & feel, so if I am fixing stuff in the xml - I automatically just
have to fix those also...
 
> 319     -                * computing what the 33% and 66% offsets would be
> 320     -                */
> 321     +                    * computing what the 33% and 66% offsets would be
> 322     +                    */
> 
> I think that's probably an undesired formatting change.
> 
Not sure, have to check...

> Statements like this:
> 
> 377     +               if (where & GridLeft || where & GridRight ||
> 378     +                   where & GridTop || where & GridBottom)
> 
> And this:
> 
> 308     +           (where & ~(GridMaximize) ||
> 309     +            (where & ~(GridLeft | GridRight | GridTop | GridBottom)
> &&
> 
> Can be combined, eg
> 
> unsigned int VerticalGrid = GridLeft | GridRight;
> unsigned int HorizontalGrid = GridTop | GridBottom;
> unsigned int ConstrainedGrid = VerticalGrid | HorizontalGrid | GridMaximize;
> 
> if (where & ~ConstrinedGrid)
> 
> ...
> 
> if (where & VerticalGrid)
> {
> }
> else if (where & HorizontalGrid)
> {
> }
> 
Yeah, I did not touch that any further for the sake of the diffs' size and 
readability, but if you want me to fix it in this MP, I'll do it...

> 530     +    else if (!gw->isGridResized &&
> 531     +            gw->isGridHorzMaximized &&
> 532     +            !gw->isGridVertMaximized)
> 533     +    {
> 534     +       /* Window has been horizontally maximized by grid. We only
> need
> 535     +        * to restore Y and height - core handles X and width. */
> 536     +       if (gw->sizeHintsFlags)
> 537     +           gw->window->sizeHints ().flags |= gw->sizeHintsFlags;
> 538     +       xwcm |=  CWY | CWHeight;
> 539     +    }
> 540     +    else if (!gw->isGridResized &&
> 541     +            !gw->isGridHorzMaximized &&
> 542     +            gw->isGridVertMaximized)
> 543          {
> 544             /* Window has been vertically maximized by grid. We only need
> 545     -        * to restore the X and width - core handles Y and height. */
> 546     +        * to restore X and width - core handles Y and height. */
> 547             if (gw->sizeHintsFlags)
> 548                 gw->window->sizeHints ().flags |= gw->sizeHintsFlags;
> 549             xwcm |= CWX | CWWidth;
> 550          }
> 551     -
> 552     -    else if (gw->isGridResized && !gw->isGridSemiMaximized)
> 553     -       /* Window is just gridded (top, bottom, center, corners). We
> 554     -        * need to handle everything. */
> 555     +    else if (gw->isGridResized &&
> 556     +            !gw->isGridHorzMaximized &&
> 557     +            !gw->isGridVertMaximized)
> 558     +       /* Window is just gridded (center, corners).
> 559     +        * We need to handle everything. */
> 
> It might make sense to nest these blocks.
> 
> if (!isGridResized)
> {
>     assert (isGridHorzMaximized != isGridVertMaximized);
> 
>     if (isGridHorzMaximized)
>     {
>         ...
>     }
>     else if (isGridVertMaximized)
>     {
>         ...
>     }
> }
> else
> {
>     assert (!isGridHorzMaximized);
>     assert (!isGridVertMaximized);
>     ...
> }
> 
> Though the use of all these boolean flags has made me wonder if it would be
> better to use an enum instead:
> 
> enum CurrentGridState
> {
>     None = 0,
>     Resized = 1,
>     VerticallyMaximized = 2,
>     HorizontallyMaximized = 3
> };

I'll see what I can do about that one as well.
Thanks a lot for the review. I am happy we have all currently known Grid bugs
eliminated finally...