[PATCH v3 3/4] comctl32/listbox: Implement LBS_NODATA for single-selection listboxes

[Gabriel Ivăncescu]

On 2/18/19 2:34 PM, Gabriel Ivăncescu wrote:
> On 2/18/19 2:19 PM, Nikolay Sivov wrote:
>> On 2/18/19 3:09 PM, Gabriel Ivăncescu wrote:
>>
>>> +static BOOL is_singlesel_NODATA(const LB_DESCR *descr)
>>> +{
>>> +    return (descr->style & (LBS_NODATA | LBS_MULTIPLESEL | 
>>> LBS_EXTENDEDSEL)) == LBS_NODATA;
>>> +}
>>> +
>>> +static BOOL is_multisel_NODATA(const LB_DESCR *descr)
>>> +{
>>> +    return (descr->style & LBS_NODATA) && (descr->style & 
>>> (LBS_MULTIPLESEL | LBS_EXTENDEDSEL));
>>> +}
>>> +
>>
>> This has to be consistent with existing IS_MULTISELECT(), both for 
>> naming and usage throughout the code.
>>
>> Could the same be achieved with IS_MULTISELECT() and direct style 
>> check for LBS_NODATA?
>>
>> Shortcuts with is_singlesel_NODATA() also seem avoidable, if we don't 
>> need to allocate anything in this case, can we just check for 
>> descr->items != NULL?
>>
>> That's basically what I was trying to suggest since first version of 
>> such helpers appeared.
>>
>>
>>
> 
> Hi Nikolay,
> 
> So you want me to use a macro? Or just all-caps naming? Or all lowercase?
> 
> And we can't check for descr->items != NULL since it's used in places 
> where it can be NULL, in this case for normal listbox it will allocate 
> the space (e.g. resize_storage).
> 
> I think I had it with manual checks originally but was told to move it 
> into a helper.

Ok so, I will turn them to inline functions in all-caps naming, i.e. 
IS_SINGLESEL_NODATA and IS_MULTISEL_NODATA instead of macros. 
IS_MULTISELECT checks for LBS_NOSEL, which I want to avoid (the usage of 
them is based on the storage of items, so I don't think that should 
factor into it), so I will keep the actual functions as they are now.

And in insert_item_data and remove_item_data I will replace with check 
for NULL because it works there.

Does that sound reasonable?