-
Notifications
You must be signed in to change notification settings - Fork 226
Fix(ish) formatting of RenderFragments (C# templates) #12397
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
Fix(ish) formatting of RenderFragments (C# templates) #12397
Conversation
This means we do a much better, but not perfect, job of formatting render fragments.
This is an unfortunate regression, but IMO the PR still makes for a better overall formatting experience
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses formatting issues for multi-line RenderFragment expressions in Razor code by improving how the C# formatter generates lambda expressions for these constructs. The key changes involve:
- Adding special handling for multi-line RenderFragments to emit block-bodied lambda expressions instead of null statements
- Tracking whether Roslyn places opening braces on new lines in lambda expressions based on formatting options
- Enhancing the logic to skip opening braces that may be placed on separate lines by Roslyn
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| HtmlFormattingTest.cs | Updated test expectation to align with improved Caption attribute indentation |
| DocumentFormattingTest.cs | Added semicolons to RenderFragment variable declarations and added 7 new formatting tests covering multi-line RenderFragment scenarios with various brace placement options |
| CSharpFormattingPass.cs | Enhanced brace-skipping logic to check for first non-whitespace character and handle lambda opening braces, with updated comment explaining the behavior |
| CSharpFormattingPass.CSharpDocumentGenerator.cs | Added core logic to detect multi-line RenderFragments, emit lambda expressions instead of null statements, track formatting options for brace placement, and properly close lambda blocks |
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpFormattingPass.cs
Outdated
Show resolved
Hide resolved
...eAnalysis.Razor.Workspaces/Formatting/Passes/CSharpFormattingPass.CSharpDocumentGenerator.cs
Outdated
Show resolved
Hide resolved
...eAnalysis.Razor.Workspaces/Formatting/Passes/CSharpFormattingPass.CSharpDocumentGenerator.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
| formattedCSharpText.Lines[iFormatted + 1] is { Span.Length: > 0 } nextLine && | ||
| nextLine.CharAt(0) == '{') | ||
| nextLine.GetFirstNonWhitespaceOffset() is { } firstNonWhitespace && | ||
| nextLine.Start + firstNonWhitespace == nextLine.End - 1 && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good call, thanks. I think I just added this because I noticed it was missing, clearly didn't look far enough down the page :)
ToddGrun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
Fixes #12310
Fixes #6632
This doesn't do a perfect job of formatting these, because there is an extra level of indentation in some (all?) cases, because the Html formatter doesn't see the
@<tag>as a tag. It's a lot better than what we did previously (see screenshot in #12310), and likely needs #11916 for us to be able to do anything better, so I think this is still a net positive for users.