Skip to content

Conversation

ThomasGoulet73
Copy link
Contributor

@ThomasGoulet73 ThomasGoulet73 commented Feb 8, 2025

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 force-pushed the run-MilCodeGen-for-PresentationCore branch from 8f44ef0 to 8d86bf3 Compare February 11, 2025 05:57
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 :

[[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

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.

Labels

Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants