On Wed, Nov 21, 2018 at 12:26 PM Nikolay Sivov nsivov@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. :-)