[PATCH v2 1/3] comctl32/pager: Support toolbar notification conversion.

[Nikolay Sivov]

On 09/11/2018 03:26 PM, Zhiyi Zhang wrote:

> +/* Text field conversion test behavior flags. */
> +enum test_conversion_flags
> +{
> +    CONVERT_SEND = 0x01,
> +    NOT_CONVERT_SEND = 0x02,
> +    CONVERT_RECEIVE = 0x04,
> +    NOT_CONVERT_RECEIVE = 0x08,
> +    NULL_SEND_EMPTY = 0x10,
> +    NOT_NULL_SEND_EMPTY = 0x20
> +};
I think those should be called DONT_*, or changed to SEND_NOT_CONVERTED, 
or DONT_CONVERT_ON_SEND. or similar. And null case something like 
SEND_EMPTY_IF_NULL,
otherwise it's hard to understand without looking at what it does. By 
the way, why is it necessary to have both flags in tests and not in 
implementation?

> +/* Send notify to test text field conversion. In parent proc notify_generic_text_handler() handles these messages */
> +static void test_notify_generic_text_helper(HWND pager, void *ptr, size_t size, UINT *mask, UINT required_mask,
> +                                            WCHAR **text, INT *text_max, UINT code_unicode, UINT code_ansi, DWORD flags)
> +{
> +    const struct notify_test_send_data *send_datas;
> +    const struct notify_test_receive_data *receive_datas;
I don't think 'datas' is the word, you can drop _data from structure 
names, and use send_data for variable.

> +static void _send_notify(HWND pager, UINT unicode, UINT ansi, LPARAM lParam, BOOL code_change)
> +{
> +    NMHDR *hdr = (NMHDR *)lParam;
> +
> +    notify_test_info.unicode = unicode;
> +    notify_test_info.id_from = 1;
> +    notify_test_info.hwnd_from = child1_wnd;
> +    notify_test_info.ansi = ansi;
> +    notify_test_info.received = FALSE;
> +
> +    hdr->code = unicode;
> +    hdr->idFrom = 1;
> +    hdr->hwndFrom = child1_wnd;
> +
> +    SendMessageW(pager, WM_NOTIFY, hdr->idFrom, lParam);
> +    ok(notify_test_info.received, "Expect notification received\n");
> +    ok(hdr->code == code_change ? ansi : unicode, "Expect 0x%08x, got 0x%08x\n", hdr->code,
> +       code_change ? ansi : unicode);
> +}
> +
> +#define send_notify(a, b, c, d) _send_notify(pager, a, b, c, d)

Saving one argument is not worth it, it's usually done like that if you 
want to pass line number to ok_(), otherwise I don't see a point.

> +    for (i = 0; i < ARRAY_SIZE(paras); i++)
> +    {
> +        const struct generic_text_helper_para *p = paras + i;
> +        test_notify_generic_text_helper(pager, p->ptr, p->size, p->mask, p->required_mask, p->text, p->text_max,
> +                                        p->code_unicode, p->code_ansi, p->flags);
> +    }
Is it possible to pass structure pointer here?