From: "Rémi Bernon" <rbernon@codeweavers.com>
Subject: Re: [PATCH 03/11] include/windows.globalization.idl: Add ILanguage interface and Language class stubs.
Message-Id: <731ce5a5-83ee-23a6-518d-0c961eff7af9@codeweavers.com>
Date: Wed, 19 Jan 2022 19:15:43 +0100
In-Reply-To: <20220119132819.18340-4-besentv@gmail.com>
References: <20220119132819.18340-1-besentv@gmail.com> <20220119132819.18340-4-besentv@gmail.com>

Hi Bernhard,

On 1/19/22 14:28, Bernhard Kölbl wrote:
> Signed-off-by: Bernhard Kölbl <besentv@gmail.com>
> ---
>   include/windows.globalization.idl | 38 +++++++++++++++++++++++++++++++
>   1 file changed, 38 insertions(+)
> 
> diff --git a/include/windows.globalization.idl b/include/windows.globalization.idl
> index c08f7bd27d9..c6b7b96f894 100644
> --- a/include/windows.globalization.idl
> +++ b/include/windows.globalization.idl
> @@ -29,6 +29,9 @@ import "windows.foundation.idl";
>   namespace Windows {
>       namespace Globalization {
>           typedef enum DayOfWeek DayOfWeek;
> +        interface ILanguage;
> +        interface ILanguageExtensionSubtags;
> +        runtimeclass Language;
>       }
>   }
>   
> @@ -45,5 +48,40 @@ namespace Windows {
>               Friday    = 5,
>               Saturday  = 6
>           };
> +
> +        [
> +            contract(Windows.Foundation.FoundationContract, 1.0),
> +            exclusiveto(Windows.Globalization.Language),
> +            uuid(ea79a752-f7c2-4265-b1bd-c4dec4e4f080)
> +        ]
> +        interface ILanguage : IInspectable
> +        {
> +            /* Stub! */
> +        }
> +
> +        [
> +            contract(Windows.Foundation.FoundationContract, 1.0),
> +            exclusiveto(Windows.Globalization.Language),
> +            uuid(7d7daf45-368d-4364-852b-dec927037b85)
> +        ]
> +        interface ILanguageExtensionSubtags : IInspectable
> +        {
> +            /* Stub! */
> +        }
> +

Any reason to keep these empty? As far as I can see they are simple 
enough to not pull anything for their full declaration. It'd be better 
than just a comment that we may easily forget about.

> +        [
> +            activatable(Windows.Foundation.UniversalApiContract, 1.0),

This activatable attribute should actually have three arguments, and the 
first one be an interface. This is also apparently not supported by widl 
at the moment, so it would be nice to make it support it first, before 
adding this class (sorry).

> +            contract(Windows.Foundation.FoundationContract, 1.0),
> +            marshaling_behavior(agile),
> +            /* Fixme: ILanguageStatics, ILanguageStatics2 not defined yet:
> +            static(Windows.Globalization.ILanguageStatics, Windows.Foundation.UniversalApiContract, 1.0),
> +            static(Windows.Globalization.ILanguageStatics2, Windows.Foundation.UniversalApiContract, 1.0), */

For such cases, and for the activatable attribute above, it's also only 
required to have a declaration of the interface, not an actual 
definition, so it would be better to just forward declare the 
ILanguageStatics at the top of the file, and uncomment this.

Also in my version of the SDK I can see that there's ILanguageStatics3 
too, starting with contract version 10.0.

> +            threading(both)
> +        ]
> +        runtimeclass Language
> +        {
> +            [contract(Windows.Foundation.UniversalApiContract, 1.0), default] interface Windows.Globalization.ILanguage;
> +            [contract(Windows.Foundation.UniversalApiContract, 1.0)] interface Windows.Globalization.ILanguageExtensionSubtags;
> +        }
>       }
>   }

Same here, you only need the interfaces to be forward declared, it would 
be nice to have them all already I think, as it's not much effort.

Cheers,
-- 
Rémi Bernon <rbernon@codeweavers.com>