[PATCH v2 07/10] comctl32/listbox: Use a helper to set the selected item in single-selection listboxes

[Huw Davies]

On Mon, Feb 11, 2019 at 07:03:10PM +0200, Gabriel Ivăncescu wrote:
> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
> ---
>  dlls/comctl32/listbox.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c
> index 48b5b78..633e7c6 100644
> --- a/dlls/comctl32/listbox.c
> +++ b/dlls/comctl32/listbox.c
> @@ -160,6 +160,18 @@ static BOOL is_item_selected( const LB_DESCR *descr, UINT index )
>      return descr->items[index].selected;
>  }
>  
> +static void set_item_selected( LB_DESCR *descr, UINT index )
> +{
> +    INT oldsel = descr->selected_item;
> +
> +    if (!(descr->style & LBS_NODATA))
> +    {
> +        if (oldsel != -1) descr->items[oldsel].selected = FALSE;
> +        if (index != -1) descr->items[index].selected = TRUE;
> +    }
> +    descr->selected_item = index;
> +}

I had more in mind:

static void set_item_selected_state( LB_DESCR *descr, UINT index, BOOL state )
{
    if (!(descr->style & LBS_NODATA)) descr->items[index].selected = state;
}

The idea is that the helpers are simple getters/setters for the item
'object'.  It's in these helpers that you'll hide any storage
differences.

> +
>  static ULONG_PTR get_item_data( const LB_DESCR *descr, UINT index )
>  {
>      return (descr->style & LBS_NODATA) ? 0 : descr->items[index].data;
> @@ -1476,10 +1488,8 @@ static LRESULT LISTBOX_SetSelection( LB_DESCR *descr, INT index,
>      {
>          INT oldsel = descr->selected_item;
>          if (index == oldsel) return LB_OKAY;
> -        if (oldsel != -1) descr->items[oldsel].selected = FALSE;
> -        if (index != -1) descr->items[index].selected = TRUE;
> +        set_item_selected(descr, index);
>          if (oldsel != -1) LISTBOX_RepaintItem( descr, oldsel, ODA_SELECT );
> -        descr->selected_item = index;
>          if (index != -1) LISTBOX_RepaintItem( descr, index, ODA_SELECT );
>          if (send_notify && descr->nb_items) SEND_NOTIFICATION( descr,
>                                 (index != -1) ? LBN_SELCHANGE : LBN_SELCANCEL );

So it would get called twice in the above code, and the above would still set
->selected_item itself.

This should also be used in _SelectItemRange() which will help when
you get to nodata multi-select.

Huw.