Skip to content

Conversation

@davidwengier
Copy link
Member

Fixes the other reported issue in developercommunity.visualstudio.com/t/Format-Document-in-Razor-pages-no-longer/10903159
Fixes devdiv.visualstudio.com/DevDiv/_workitems/edit/2471065

Allows users to have Roslyn configured for K&R style braces (ie, on the same line), even though its ugly 😛

@davidwengier davidwengier requested a review from a team as a code owner June 4, 2025 05:27
}
}
""",
// I'm so sorry, but I could not find any way to change Roslyn formatting options from our test infra. Forgive my hacky sins
Copy link
Contributor

Choose a reason for hiding this comment

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

Forgive my hacky sins

This does feel particularly nasty. Is there no way to pass over some data to the EA layer in Roslyn that indicates that CSharpFormattingOptions2.NewLineBeforeOpenBrace should be false?

Copy link
Member Author

Choose a reason for hiding this comment

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

EA would definitely be able to solve this, and in fact we already have EA code that we use to tweak settings in formatting, but I'm planning to service this so I don't need the complexity of dual insertions. At least this way I can port the tests back to 17.14 too, and be sure we have coverage.

The frustrating thing is that it shouldn't even be necessary, I just couldn't get the things that should work working.

Copy link
Contributor

@ToddGrun ToddGrun left a comment

Choose a reason for hiding this comment

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

:shipit:

@davidwengier davidwengier enabled auto-merge June 5, 2025 00:30
@davidwengier davidwengier merged commit 0ad37fe into dotnet:main Jun 5, 2025
11 checks passed
@davidwengier davidwengier deleted the FormatKandR branch June 5, 2025 01:39
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Jun 5, 2025
@davidwengier
Copy link
Member Author

/backport to release/dev17.14

@github-actions
Copy link
Contributor

github-actions bot commented Jun 5, 2025

Started backporting to release/dev17.14: https://github.com/dotnet/razor/actions/runs/15456468743

@github-actions
Copy link
Contributor

github-actions bot commented Jun 5, 2025

@davidwengier backporting to "release/dev17.14" failed, the patch most likely resulted in conflicts:

$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Handle when Roslyn removes newlines while formatting
Using index info to reconstruct a base tree...
M	src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Extensions/TextLineExtensions.cs
M	src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/New/CSharpFormattingPass.CSharpDocumentGenerator.cs
M	src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs
M	src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs
M	src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingTestBase.cs
M	src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/TestRazorFormattingService.cs
M	src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/FormattingTestBase.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/FormattingTestBase.cs
CONFLICT (content): Merge conflict in src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/FormattingTestBase.cs
Auto-merging src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/TestRazorFormattingService.cs
CONFLICT (content): Merge conflict in src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/TestRazorFormattingService.cs
Auto-merging src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingTestBase.cs
CONFLICT (content): Merge conflict in src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingTestBase.cs
Auto-merging src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs
Auto-merging src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs
CONFLICT (content): Merge conflict in src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs
Auto-merging src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/New/CSharpFormattingPass.CSharpDocumentGenerator.cs
Auto-merging src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Extensions/TextLineExtensions.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Handle when Roslyn removes newlines while formatting
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

phil-allen-msft added a commit that referenced this pull request Jun 6, 2025
Backport of #11908 and #11911 to release/dev17.14

Fixes
[developercommunity.visualstudio.com/t/Format-Document-in-Razor-pages-no-longer/10903159](https://developercommunity.visualstudio.com/t/Format-Document-in-Razor-pages-no-longer/10903159)
Fixes
[devdiv.visualstudio.com/DevDiv/_workitems/edit/2471065](https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2471065)

/cc @davidwengier

## Customer Impact

## Regression

- [ ] Yes
- [ ] No

[If yes, specify when the regression was introduced. Provide the PR or
commit if known.]

## Testing

[How was the fix verified? How was the issue missed previously? What
tests were added?]

## Risk

[High/Medium/Low. Justify the indication by mentioning how risks were
measured and addressed.]
davidwengier added a commit that referenced this pull request Jun 18, 2025
Razor side of dotnet/roslyn#78949

Two followups from previous PRs:
* Export formatting options for Razor testing:
#11911 (comment)
* Share const for complex edit command name:
#11938 (comment)

Won't build, of course, without the above PR merged and packages
referenced
@RikkiGibson RikkiGibson modified the milestones: Next, 18.0 P1 Aug 20, 2025
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.

4 participants