[Add options to] skip formatting in script and style blocks with complex Razor expressions - #85142
davidwengier wants to merge 7 commits into
Conversation
…th complex Razor expressions/statements.
…azor expressions or statements Unfortunately, due to how we emit Html from the Razor document, the downstream formatters have no choice but to make formatting decisions that are not great. Without fundamentally re-writing how we do that emitting, and/or more like without having to parse CSS and JavaScript ourselves, the best we can do for now is simply avoid formatting and hopefully not break users code.
|
Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟡 Changes recommended
A newly added regression test does not currently prove the option toggle has an effect because its inputs/outputs are identical.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces a new Razor formatting option to preserve source indentation inside <script>/<style> blocks when they contain “complex” Razor constructs, wiring the option through VS/VS Code settings, formatting endpoints, and the remote formatting passes. It also adds/updates tests and localizes the new setting’s UI strings.
Changes:
- Add
IgnoreIndentationInScriptOrStyleBlocksWithComplexRazorto client/formatting option models and propagate it through cohost/LSP formatting entry points. - Update remote HTML/C# formatting passes to detect “complex Razor” within script/style bodies and preserve indentation accordingly.
- Add regression tests and register/localize the setting for Visual Studio (Unified Settings + resx/xlf), plus VS Code configuration mapping.
File summaries
| File | Description |
|---|---|
| src/Razor/src/Razor/test/Microsoft.VisualStudioCode.RazorExtension.UnitTests/RemoteClientSettingsServiceTest.cs | Updates test settings construction to include the new advanced setting. |
| src/Razor/src/Razor/test/Microsoft.VisualStudioCode.RazorExtension.UnitTests/CohostConfigurationChangedServiceTest.cs | Extends JSON parsing tests for the additional config value and default. |
| src/Razor/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.UnitTests/Settings/ClientSettingsManagerTest.cs | Updates change-detection test data for the new setting. |
| src/Razor/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.UnitTests/Cohost/Formatting/FormattingLogTest.LegacyRazorFormattingOptions.cs | Adds legacy option surface and forwards it into RazorFormattingOptions. |
| src/Razor/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.UnitTests/Cohost/Formatting/FormattingLogTest.cs | Threads the new option into the formatting test harness call. |
| src/Razor/src/Razor/test/Microsoft.CodeAnalysis.Remote.Razor.UnitTests/ClientSettingsJsonSerializationTest.cs | Validates JSON round-tripping includes the new advanced setting name/value. |
| src/Razor/src/Razor/test/Microsoft.CodeAnalysis.Razor.CohostingShared.UnitTests/Formatting/HtmlFormattingTest.cs | Updates helper invocation to include the new option parameter. |
| src/Razor/src/Razor/test/Microsoft.CodeAnalysis.Razor.CohostingShared.UnitTests/Formatting/DocumentFormattingTestBase.cs | Adds a test harness parameter and flows it into ClientSettingsManager for formatting. |
| src/Razor/src/Razor/test/Microsoft.CodeAnalysis.Razor.CohostingShared.UnitTests/Formatting/DocumentFormattingTest.cs | Adds new formatting regression tests for complex script scenarios (and tweaks an existing expected indent). |
| src/Razor/src/Razor/src/Microsoft.VisualStudioCode.RazorExtension/Services/CohostConfigurationChangedService.cs | Fetches the new VS Code setting and updates JSON array indexing/parsing accordingly. |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/xlf/VSPackage.zh-Hant.xlf | Adds new setting display/description resources (untranslated “new” entries). |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/xlf/VSPackage.zh-Hans.xlf | Adds new setting display/description resources (untranslated “new” entries). |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/xlf/VSPackage.tr.xlf | Adds new setting display/description resources (untranslated “new” entries). |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/xlf/VSPackage.ru.xlf | Adds new setting display/description resources (untranslated “new” entries). |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/xlf/VSPackage.pt-BR.xlf | Adds new setting display/description resources (untranslated “new” entries). |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/xlf/VSPackage.pl.xlf | Adds new setting display/description resources (untranslated “new” entries). |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/xlf/VSPackage.ko.xlf | Adds new setting display/description resources (untranslated “new” entries). |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/xlf/VSPackage.ja.xlf | Adds new setting display/description resources (untranslated “new” entries). |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/xlf/VSPackage.it.xlf | Adds new setting display/description resources (untranslated “new” entries). |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/xlf/VSPackage.fr.xlf | Adds new setting display/description resources (untranslated “new” entries). |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/xlf/VSPackage.es.xlf | Adds new setting display/description resources (untranslated “new” entries). |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/xlf/VSPackage.de.xlf | Adds new setting display/description resources (untranslated “new” entries). |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/xlf/VSPackage.cs.xlf | Adds new setting display/description resources (untranslated “new” entries). |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/VSPackage.resx | Adds display name + description resources for the new setting. |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/UnifiedSettings/razor.registration.json | Registers the new Unified Settings key and default value. |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.RazorExtension/Microsoft.VisualStudio.RazorExtension.Custom.pkgdef | Updates CacheTag due to settings manifest change. |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Options/SettingsNames.cs | Adds new unified setting name constant and includes it in the known list. |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Options/OptionsStorage.cs | Reads the new unified setting with default true. |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostInlineCompletionEndpoint.cs | Threads the new option into RazorFormattingOptions.From(...) construction. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/Passes/HtmlFormattingPass.cs | Conditionally excludes complex script/style bodies from script/style span processing based on the new option. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/Passes/CSharpFormattingPass.CSharpDocumentGenerator.cs | Adds logic to preserve indentation for complex script/style bodies (and uses a distinct marker line for ignored lines). |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Settings/ClientSettings.cs | Maps the new advanced setting into RazorFormattingOptions. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/RazorSyntaxFacts.cs | Adds detection logic to determine whether a script/style body contains “complex Razor”. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingOptions.cs | Adds the option to the formatting options payload and updates factory methods. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/OnAutoInsert/CohostOnAutoInsertEndpoint.cs | Threads the new option into formatting options passed to OOP. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/Formatting/CohostRangeFormattingEndpoint.cs | Threads the new option into formatting options passed to OOP. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/Formatting/CohostOnTypeFormattingEndpoint.cs | Threads the new option into formatting options passed to OOP. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/Formatting/CohostDocumentFormattingEndpoint.cs | Threads the new option into formatting options passed to OOP. |
| .github/instructions/Razor.instructions.md | Documents that CacheTag must be updated when razor.registration.json changes. |
Review details
- Files reviewed: 39/39 changed files
- Comments generated: 1
- Review effort level: Lite
# Conflicts: # src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/Passes/CSharpFormattingPass.CSharpDocumentGenerator.cs
There was a problem hiding this comment.
🔵 Needs a closer look
One or more issues must be addressed before approval.
Review details
Suppressed comments (3)
src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/RazorSyntaxFacts.cs:185
- This classifier is now the gate for both
<script>and<style>blocks and has separate paths for comments/statements versus allowlisted single-line inline expressions, but the added formatting regressions only exercise script content. Please add focused style-block cases (including a Razor statement/comment) and a simple inline expression case so changes to either classification path cannot silently re-enable indentation or suppress normal formatting in one embedded language.
internal static bool ContainsComplexRazorInScriptOrStyleBody(BaseMarkupElementSyntax element, SourceText sourceText)
{
if (!IsScriptOrStyleBlock(element) ||
element.StartTag is not { } startTag ||
element.EndTag is not { } endTag ||
src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/Passes/HtmlFormattingPass.cs:349
- This new branch is shared by both
<script>and<style>formatting, but the added end-to-end cases only exercise complex scripts; the existing style cases contain CSS only and never executeContainsComplexRazorInScriptOrStyleBodyfor a style element. Please add a complex<style>regression case (including the enabled and disabled option behavior) so a style-specific regression cannot pass unnoticed.
if (!ignoreIndentationInScriptOrStyleBlocksWithComplexRazor ||
!RazorSyntaxFacts.ContainsComplexRazorInScriptOrStyleBody(element, sourceText))
{
src/Razor/src/Razor/test/Microsoft.CodeAnalysis.Razor.CohostingShared.UnitTests/Formatting/DocumentFormattingTest.cs:16705
- The new behavior is shared by both
<script>and<style>elements, but all of the added end-to-end preservation cases here exercise only<script>. Existing style tests cover plain CSS or escaped at-rules, not a complex Razor body whose indentation must be preserved, so a style-specific regression in eitherBuildSpansor the C# document generator could pass unnoticed. Add a<style>case with a multiline Razor statement or comment and intentionally different source/HTML-formatted indentation.
- Files reviewed: 39/39 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
Ping @dotnet/razor-tooling if anyone wants to review. |
|
Ping @dotnet/roslyn-ide @dotnet/razor-tooling for review |


Fixes #85661
Shamelessly repeating what I wrote in one of the commits, unfortunately due to how we emit Html from the Razor document, the downstream formatters have no choice but to make formatting decisions that are not great. Without fundamentally re-writing how we do that emitting, and/or more like without having to parse CSS and JavaScript ourselves, the best we can do for now is simply avoid formatting and hopefully not break users code.
Commit-at-a-time review would be simplest I imagine.
Microsoft Reviewers: Open in CodeFlow