[PATCH 2/6] comctl32/listview: Move LVN_ODSTATECHANGED notification to function.

[Angelo Haller]

On 11/05/2022 01.00, Nikolay Sivov wrote:
> On 5/10/22 01:17, Angelo Haller wrote:
>> +/***
>> + * DESCRIPTION:
>> + * Sets the state of multiple items for LVS_OWNERDATA listviews.
>> + * Make sure to also disable per item notifications via the 
>> notification mask.
>> + *
>> + * PARAMETER(S):
>> + * [I] infoPtr : valid pointer to the listview structure
>> + * [I] nFirst : first item index
>> + * [I] nLast : last item index
>> + * [I] item  : item or subitem info
>> + *
>> + * RETURN:
>> + *   SUCCESS : TRUE
>> + *   FAILURE : FALSE
>> + */
>
> Please remove this header.

I can remove that if so desired. I however do believe a remark to 
disable per item notifications is important, as this needs to be 
manually done on all call sites.

>> +static BOOL LISTVIEW_SetOwnerDataState(LISTVIEW_INFO *infoPtr, INT 
>> nFirst, INT nLast, const LVITEMW *item)
>> +{
>> +    NMLVODSTATECHANGE nmlv;
>> +
>> +    if (!item) return FALSE;
>> +
>> +    ZeroMemory(&nmlv, sizeof(nmlv));
>> +    nmlv.iFrom = nFirst;
>> +    nmlv.iTo = nLast;
>> +    nmlv.uOldState = 0;
>> +    nmlv.uNewState = item->state;
>
> Are state fields always like that? E.g. old is always 0? Can "item" be 
> legitimately NULL?
>
I did not look into the legitimacy of the existing code of old always 
being 0. I believe that is the case though. LVN_ODSTATECHANGED seems to 
be only used  for "notify about new selection". Deselection is seemingly 
always done via "deselect all" (item = -1), as can be seen in the tests 
I submitted. I have not found any other instances were windows sends 
LVN_ODSTATECHANGED yet.

Regarding "item" being NULL, I do not know if there is a legitimate case 
for that. I just mimicked the defensive? coding practices I found in 
LISTVIEW_SetItemState, which does check "item" for NULL.

>> +
>> +    notify_hdr(infoPtr, LVN_ODSTATECHANGED, (LPNMHDR)&nmlv);
>> +
>> +    return TRUE;
>> +}
> If return value is never going to be used it's better to have it as void.
>
Again, I am happy to change that. This is, again, just adhering to the 
existing coding practices found within the file.