Re: [PATCH v7 0/6] MR8890: windows.devices.enumeration: Add support for parsing AQS filter strings.

[Rémi Bernon (＠rbernon)]

Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.y:
> - case OP_TYPE_BOOLEAN:
> -
>    if (expr->u.boolean.op < ARRAY_SIZE( bool_op ))
> -
>        return wine_dbg_sprintf( "{%s %p %p}", bool_op[expr->u.boolean.op], expr->u.boolean.lhs,
> -
>                                 expr->u.boolean.rhs );
> -
>    return wine_dbg_sprintf( "{(unknown %d) %p %p}", expr->u.boolean.op, expr->u.boolean.lhs, expr->u.boolean.rhs );
> - case OP_TYPE_COMPARE:
> - {
> -
>    const struct expr_compare *cmp = &expr->u.compare;
> -
>    if (expr->u.compare.op < ARRAY_SIZE( compare_op ))
> -
>        return wine_dbg_sprintf( "{%s %p %s}", compare_op[cmp->op], cmp->prop_desc, debugstr_propvar( &cmp->val ) );
> -
>    return wine_dbg_sprintf( "{(unknown %d) %p %s}", cmp->op, cmp->prop_desc, debugstr_propvar( &cmp->val ) );
> - }
> - default:
> -
>    return wine_dbg_sprintf( "{(unknown %d)}", expr->op_type );
> - }
>
> +}

I think you should move these to aqs.c (or to aqs.h for `debugstr_propvar`), would let you hide most of the expression structures, and reduce friction between the .y and .c. Keeping the .y minimal is better IMO, they often don't benefit from the usual C source editor features.