Re: [try 2] comctl32: Implement highlighting (marquee) selection support in listview

[Nikolay Sivov]

Owen Rudge wrote:
> This patch adds support for selecting multiple items in a 
> multi-selection listview using the mouse (by dragging and 
> highlighting). It has been reimplemented in a more efficient manner 
> compared to the previous patches.
>
> ---
>  dlls/comctl32/listview.c |  135 
> ++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 120 insertions(+), 15 deletions(-)
> Hi again.

Didn't test it actually but it looks definitely better without extra 
cycles and state caching, thanks for that.
I suggest another simple naming improvement: you're mixing different 
things in comments and naming,
it looks confusing now.

---
+ BOOL bHighlighting;
+ RECT highlightRect;
---

I suggest something like marqueeRect name here cause item itself has a 
state flag called LVIS_DROPHILITED
which has nothing to do with this rectangle obviously.

---
+ /* Cancel out the old focus rectangle and draw the new one */
+ LISTVIEW_InvalidateRect(infoPtr, &infoPtr->highlightRect);
---

Focus rectangle is an item focus bounds in current naming, so comment is 
a bit confusing here.

---
+ /* Draw focus rect if highlighting */
+ if (infoPtr->bHighlighting)
+ DrawFocusRect(hdc, &infoPtr->highlightRect);
...

+        /* Remove the focus rectangle */
+        if (infoPtr->bHighlighting)
+            LISTVIEW_InvalidateRect(infoPtr, &infoPtr->highlightRect);

---

Same here, it isn't a focus rectangle.

Regards.

[try 2] comctl32: Implement highlighting (marquee) selection support in listview

[Owen Rudge]

Hi Nikolay,
> Focus rectangle is an item focus bounds in current naming, so comment is 
> a bit confusing here.

Ah, yes, that seems sensible - I named it "focus rectangle" as that's 
what the API function was called, but of course the way it's used would 
lend itself better to different nomenclature. I'll amend that and send 
in an updated patch.

Regards,

-- 
Owen Rudge
http://www.owenrudge.net/