Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto generate projections for all referenced winmd files #1262

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lukasf
Copy link

@lukasf lukasf commented Oct 25, 2022

This is to solve #780.

This PR allows generating projections for all referenced WinRT projects, without any manual editing of the csproj file. Just add a reference to CsWinRT, add one or more references to C++/WinRT component projects, and you are ready to compile. All projections are generated and compiled. This should make dev's lives easier (and also simplify documentation for the most common case).

Tested with the sample project, and also with multiple WinRT references.

@manodasanW
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@brabebhin
Copy link

It would be pretty cool to have this functionality built into cs/winRT. Having to manually create nuget packages as part of the development and debugging loop is a nightmare.

@dongle-the-gadget
Copy link
Contributor

There might be some cases where the namespaces don't match the file name though.

@lukasf
Copy link
Author

lukasf commented Nov 6, 2022

True. But in the vast majority of cases, the .winmd filename will be the root namespace of a project. It that is not the case, it's always possible to manually specify CsWinRTIncludes. I am just setting this to a sensible default, which should work for most cases.

@brabebhin
Copy link

I think the pattern coming from uwp is to have the namespace match the file name, so I'd say the cases in which it does not match are very few and far in between.

@j0shuams
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@j0shuams
Copy link
Contributor

Hi @lukasf , there seems to be something with this change that is affecting the CsWinRT Embedded support. I am taking a look into this now.

@lukasf
Copy link
Author

lukasf commented Nov 30, 2022

Hi @j0shuams, thank you for looking into it. I wonder how this could affect embedded support...

@dongle-the-gadget
Copy link
Contributor

@lukasf, I think a better option (maybe?) is to modify the cswinrt.exe generator to generate everything not excluded by default?

@lukasf
Copy link
Author

lukasf commented Sep 13, 2023

@dongle-the-gadget That might be even better, since it would work without namespace restrictions.

I just peeked over the code a bit. It seems that it's the winmd::reader::filter class which decides if a member is to be included or not (in the prive includes method). The filter class is used in cswinrt::settings.

We'd still need to respect the excludes (which get added by default), so we cannot just omit using the filter. Instead we need to modify the filter class. Which in turn means, we'd first need a PR in the winmd repo before we can solve this in cswinrt.

My idea would be this: We extend the filter class constructor with an additional, optional boolean parameter. This parameter allows to specify the default choice if no mathing filter is found (see last line of the includes method). It defaults to "false", so existing code continues to work. But in cswinrt.exe, we would check the length of the includes before creating the filter. If we have zero includes specified, then we override the default to "true", so any class or namespace that is not explicitly excluded will be generated.

What do you think about this approach @dongle-the-gadget? Franky, I only had a short peek over the code, so I am not 100% sure about this. But to me it looks like this could be pretty easy to solve, if we can get a PR into the winmd repo.

@dongle-the-gadget
Copy link
Contributor

For the WinMD code, I would say that how about we add an optional parameter for the initializer?

@lukasf
Copy link
Author

lukasf commented Sep 14, 2023

Yes, that's what I meant

@jlaanstra
Copy link
Collaborator

You probably want to check $(CsWinRTIncludesPrivate) as well and make sure it's empty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants