Skip to content

Run MilCodeGen for PresentationCore #10434

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 11, 2025
Merged

Run MilCodeGen for PresentationCore #10434

merged 1 commit into from
Feb 11, 2025

Conversation

ThomasGoulet73
Copy link
Contributor

@ThomasGoulet73 ThomasGoulet73 commented Feb 8, 2025 •
edited
Loading

Contributes to #10429

Description

Generates the code in PresentationCore using the changes from #10430.

I recommend reviewing only the last commit and with whitespace changes hidden, use this link.

Customer Impact

None, shouldn't affect customers.

Regression

No.

Testing

Local build + CI + I manually validated the generated code.

Risk

Low, there shouldn't be any functional changes.

Microsoft Reviewers: Open in CodeFlow

@ThomasGoulet73 ThomasGoulet73 requested review from a team as code owners February 8, 2025 06:10
@dotnet-policy-service dotnet-policy-service bot added PR metadata: Label to tag PRs, to facilitate with triage Community Contribution A label for all community Contributions labels Feb 8, 2025
@ThomasGoulet73
Copy link
Contributor Author

This PR is blocked by #10430.

@ThomasGoulet73 ThomasGoulet73 mentioned this pull request Feb 8, 2025
Merged
@ThomasGoulet73 ThomasGoulet73 force-pushed the run-MilCodeGen-for-PresentationCore branch from 8f44ef0 to 8d86bf3 Compare February 11, 2025 05:57
dipeshmsft
dipeshmsft approved these changes Feb 11, 2025
View reviewed changes
Copy link
Member

@dipeshmsft dipeshmsft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No doubt there are spacing issues and in TextDecorationCollection we have [index] converted to [ index ], but I guess it should be handled in a separate PR.

@ThomasGoulet73 are you planning to work on the spacing and other issue ?

@h3xds1nz
Copy link
Member

That is to reduce the diff. TextDecorationCollection is like one of the few that got re-formatted because most of them are actually as [ index ].

@dipeshmsft
Copy link
Member

TextDecorationCollection is like one of the few that got re-formatted because most of them are actually as [ index ].

I thought this was reverted back to this state because of the limitation of the generator. This code gets generated from this place :

wpf/src/Microsoft.DotNet.Wpf/src/WpfGfx/codegen/mcg/helpers/CollectionHelper.cs

Line 192 in 5f713e2

[[type]] oldValue = _collection[ [[index]] ];

and if we remove the spaces, the generator crashes throwing a syntax error.

@h3xds1nz
Copy link
Member

Eitherway is possible tbh, I know where its coming from but I didn't try to modify it.

@dipeshmsft dipeshmsft merged commit 687e100 into dotnet:main Feb 11, 2025
8 checks passed
@ThomasGoulet73 ThomasGoulet73 deleted the run-MilCodeGen-for-PresentationCore branch February 11, 2025 23:41
@ThomasGoulet73
Copy link
Contributor Author

ThomasGoulet73 commented Feb 11, 2025 •
edited
Loading

I did hit the bracket limitation but the main reason I reverted TextDecorationCollection instead of fixing the code generator was because, as @h3xds1nz said, it was the only 1 of many files generated from a template that was formatted manually so either I reverted this file or changed a bunch of other files and I chose the option with the smaller diff.

and if we remove the spaces, the generator crashes throwing a syntax error.

I hit this problem also but there's way to workaround this limitation, you just need to concat the string with the [ and ] in a variable instead of inline and it will fix the parser error.

I do plan on working on formatting the generated code in the future.

@dipeshmsft dipeshmsft added this to the .NET 10 milestone Feb 19, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Mar 22, 2025
@dipeshmsft dipeshmsft modified the milestones: .NET 10, 10.0.0 Apr 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dipeshmsft dipeshmsft dipeshmsft approved these changes

@singhashish-wpf singhashish-wpf Awaiting requested review from singhashish-wpf

@Kuldeep-MS Kuldeep-MS Awaiting requested review from Kuldeep-MS

Assignees

@ThomasGoulet73 ThomasGoulet73

Labels
Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Milestone
10.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@ThomasGoulet73 @h3xds1nz @dipeshmsft