Nikolay Sivov : comctl32/taskdialog: Initial implementation of a minimal task dialog.

Nikolay Sivov bunglehead at gmail.com
Thu Mar 23 00:07:46 CDT 2017


On 23.03.2017 3:40, Fabian Maurer wrote:
> 
>> static const WCHAR nulW;
> Shouldn't this be set 0?

It's static, so it's already initialized.

> 
>> if (radio_button) *radio_button = taskconfig->nDefaultButton;
> This probably should have been changed, it was already wrong in the old 
> implementation.

Maybe it is, but I'd rather keep it as is, until we have actual support
for radio buttons.

> 
>>    /* write control entries */
>>    LIST_FOR_EACH_ENTRY_SAFE(control, control2, &desc.controls, struct 
> taskdialog_control, entry)
>  >   {
>  >       ALIGN_POINTER(ptr, 3);
>>
>>        template_write_data(&ptr, control->template, control->template_size);
>>
>>        /* list item won't be needed later */
>>        list_remove(&control->entry);
>>        Free(control->template);
>>        Free(control);
>>    }
> Wouldn't it be shorter to also use "taskdialog_clear_controls" here? Just 
> curious on why you chose to write it again.

I didn't want to do another loop just to clear the list.

> 
> Also, wouldn't it make sense to keep the template generating code separated 
> from the taskdialog specific bits? I intentionally split it, so we could use 
> those function in another place, if needed. In my opinion, that also makes the 
> code easier readable.

We can split it up once it's useful. Right now I don't see where we
could need it in comctl32.

> 
> Finally, are you adding the rest from my latest patch-set too, or should I 
> adapt the missing parts?

No, please go ahead.

> 
> Regards,
> Fabian Maurer
> 




More information about the wine-devel mailing list