Re: [3/5] comctl32: Implement TaskDialogIndirect as a custom dialog box

[Joachim Priesner]

Hi Nikolay,

thanks for the awesomely quick feedback. I'll submit a v2 of the patch soon.
> > - /* Destroy unused common buttons. */
> > - TRACE("dwCommonButtons=%x\n", config->dwCommonButtons);
> > - if (!(config->dwCommonButtons & TDCBF_YES_BUTTON))
> > -
> >    DestroyWindow(GetDlgItem(hwnd, IDYES));
>
> Did you check that it's actually destroyed and not just hidden?

You mean on Windows? Yes, the buttons are destroyed (or not created at all) there, not just hidden.
> > -
> >        WCHAR buttonText[1024];
> > -
> >        int w, h;
> > -
> >        if (GetWindowTextW(hItem, buttonText, 1024))
>
> Where does this limit come from?

The code comes from user32/msgbox.c. I think it's supposed to be a safe limit to hold the text of any of the default buttons (Yes/No/Cancel/...) in any language.
> > - /* Query parent window/desktop size and center window. */
> > - monitor = MonitorFromWindow(config->hwndParent ? config->hwndParent : GetActiveWindow(),
> > -
> >                            MONITOR_DEFAULTTOPRIMARY);
> > - monitorInfo.cbSize = sizeof(monitorInfo);
> > - GetMonitorInfoW(monitor, &monitorInfo);
> > - windowLeft = (monitorInfo.rcWork.left + monitorInfo.rcWork.right
> > -
> >              - (windowClientWidth + windowBorderWidth)) / 2;
> > - windowTop = (monitorInfo.rcWork.top + monitorInfo.rcWork.bottom
> > -
> >             - (windowClientHeight + windowBorderHeight)) / 2;
> > - SetWindowPos(hwnd, 0, windowLeft, windowTop,
> > -
> >             windowClientWidth + windowBorderWidth, windowClientHeight + windowBorderHeight,
> > -
> >             SWP_NOZORDER | SWP_NOACTIVATE | SWP_NOREDRAW);
>
> This looks overcomplicated, isn't it enough to use desktop window if no 
> parent was specified?

I don't know. This comes from user32/msgbox.c as well, and seems to work there :) Do you still want me to change it?
> > - DeleteObject(mainInstructionFont);
> > - Free(contentBuffer);
> > - Free(mainInstructionBuffer);
>
> It's better to free right when you don't need it anymore, and please use 
> simpler/shorter names.

contentBuffer and mainInstructionBuffer are actually gone now. As for the other names, could you give an example of an shortening of e.g. "mainInstructionFont" that's acceptable to you? (I deliberately chose some longer variable names in favor of readability -- I don't want to chose "miFont" for example, because I want to keep consistency with the member names of TASKDIALOGCONFIG. The very short variable names in user32/msgbox.c bugged me very much.)

Joachim