[PATCH v2 1/2] comctl32/listbox: Implement LBS_NODATA for single-selection listboxes

[Gabriel Ivăncescu]

On Wed, Nov 21, 2018 at 12:26 PM Nikolay Sivov <nsivov at codeweavers.com> wrote:
>
> > This is not new behavior though. I did it because that's what happens
> > with DeleteItem when it removes the last item, so that it keeps the
> > same behavior as before. If this isn't based on a test then the normal
> > listbox code is wrong too.
> Yes, but now it will be duplicated in two places.

Ok. If tests show that it's not sent, can I just call the function(s)
directly or do you have some other approach in mind?

> >
> > But, as I mentioned in the comments of the patch, this will make the
> > multi-selection patch smaller, since it will share this code path.
> > This will become a check for NODATA rather than just single-selection
> > and anything else will be identical (the helper itself will be
> > extended).
> >
> > If I change it for single-selection, I then have to add *another*
> > check for LBS_NODATA in the next patch, and also keep the normal code
> > path. IMO let's just keep it like this.
> I don't see why NODATA would be special here.

Of course it is special, since it has no data and we can't dereference
the item struct (and for multi-selection it will be stored differently
too, so again another "special case").

But, if we leave it like it is currently, all this implementation
detail will happen only within the NODATA helpers, so it's separated,
which is better IMO.

I mean, when multi-selection LBS_NODATA is implemented (in any way),
we *will* have to add a check here for it, there's literally no way
around it, if we don't do it now. Might as well just share the code
path with all LBS_NODATA and keep it inside those helpers, as an
implementation detail (so that, if it's changed in the future, it will
be easily done so only within the helpers).

And of course it doesn't make this patch any worse off by itself anyway...

> >
> >> Speaking of optimizations, having additional field for a number of
> >> selected items could improve LB_GETSELCOUNT and LB_GETSELITEMS.
> >>
> > Yeah, but it will require some changes in existing cases (not just
> > LBS_NODATA) to keep track of them (like when setting the range of
> > selections), I think we should leave that for later. I don't know if
> > it's very worth it since getting the count should be fast anyway on
> > multi-selection listboxes (and usually it's not spammed by apps).
> I think it's a very small change, and yes, it's separate and unrelated
> to NODATA.

Well that's only for multi-selection listboxes, I'll look into it
after at least the single-selection LBS_NODATA is done. :-)