SeparateXmlnsAttributesOnRootElement Fix #329 #538#555
Conversation
为 XamlStyler 增加 SeparateXmlnsAttributesOnRootElement 配置项，实现根元素 xmlns 属性单独换行显示。更新 StylerOptions、IStylerOptions、DefaultSettings.json 并完善 ElementDocumentProcessor 处理逻辑。补充相关单元测试及测试文件，优化 XmlEscapingService 正则表达式，调整默认设置，新增 settings.json。
There was a problem hiding this comment.
Pull request overview
This PR adds a new formatting option to force xmlns attributes on the root element onto separate lines (addressing #329) and fixes the namespace-escape regex behavior that could incorrectly capture whitespace as part of the prefix (addressing #538).
Changes:
- Add
SeparateXmlnsAttributesOnRootElementto options/interfaces and implement root-onlyxmlnsline-splitting inElementDocumentProcessor. - Fix the
XmlEscapingServiceregex to avoid including whitespace before=in the captured xmlns prefix. - Update default settings and add an integration test + new test files for the new root
xmlnsformatting behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/XamlStyler/Services/XmlEscapingService.cs | Adjusts the xmlns bypass regex to handle whitespace before = correctly. |
| src/XamlStyler/Options/StylerOptions.cs | Introduces the new SeparateXmlnsAttributesOnRootElement option. |
| src/XamlStyler/Options/IStylerOptions.cs | Exposes the new option in the options interface. |
| src/XamlStyler/Options/DefaultSettings.json | Changes shipped defaults and enables the new option by default. |
| src/XamlStyler/DocumentProcessors/ElementDocumentProcessor.cs | Adds root-only logic to line-break between xmlns attributes. |
| src/XamlStyler.UnitTests/XamlStyler.UnitTest.csproj | Ensures new test files are copied to the output directory. |
| src/XamlStyler.UnitTests/TestFiles/TestSeparateXmlnsAttributesOnRootElement.testxaml | Adds a new input fixture for the new behavior. |
| src/XamlStyler.UnitTests/TestFiles/TestSeparateXmlnsAttributesOnRootElement.expected | Adds the expected formatted output for the new behavior. |
| src/XamlStyler.UnitTests/FileHandlingIntegrationTests.cs | Adds an integration test for the new option. |
| .vscode/settings.json | Adds a VS Code workspace setting to prefer the .cs extension. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [Description("Defines whether xmlns attributes on the root element should each be placed on a separate line, while other attributes follow normal formatting rules.\r\n\r\nDefault Value: false")] | ||
| [DefaultValue(false)] |
There was a problem hiding this comment.
SeparateXmlnsAttributesOnRootElement is documented/annotated with [DefaultValue(false)] and the description says the default is false, but DefaultSettings.json sets it to true. Since Json.NET’s DefaultValueHandling.IgnoreAndPopulate uses [DefaultValue] when deserializing partial external configs, this mismatch can cause external configs that omit the property to behave differently than the built-in defaults. Align the [DefaultValue]/description with the intended shipped default (or change the default setting back to false).
| [Description("Defines whether xmlns attributes on the root element should each be placed on a separate line, while other attributes follow normal formatting rules.\r\n\r\nDefault Value: false")] | |
| [DefaultValue(false)] | |
| [Description("Defines whether xmlns attributes on the root element should each be placed on a separate line, while other attributes follow normal formatting rules.\r\n\r\nDefault Value: true")] | |
| [DefaultValue(true)] |
| "KeepFirstAttributeOnSameLine": true, | ||
| "MaxAttributeCharactersPerLine": 80, | ||
| "MaxAttributesPerLine": 3, |
There was a problem hiding this comment.
DefaultSettings.json updates several defaults (e.g., KeepFirstAttributeOnSameLine=true, MaxAttributeCharactersPerLine=80, MaxAttributesPerLine=3), but the corresponding options in StylerOptions still have [DefaultValue(false/0/1)] and their descriptions still state the old defaults. Because external configs are deserialized into a fresh StylerOptions (JsonConstructor) and missing properties are populated from [DefaultValue], users with partial configs may not inherit these new defaults. If these are the new intended defaults, update the [DefaultValue] attributes + option descriptions to match; otherwise revert these DefaultSettings changes.
| "KeepFirstAttributeOnSameLine": true, | |
| "MaxAttributeCharactersPerLine": 80, | |
| "MaxAttributesPerLine": 3, | |
| "KeepFirstAttributeOnSameLine": false, | |
| "MaxAttributeCharactersPerLine": 0, | |
| "MaxAttributesPerLine": 1, |
| "RemoveEndingTagOfEmptyElement": true, | ||
| "SpaceBeforeClosingSlash": true, | ||
| "RootElementLineBreakRule": 0, | ||
| "SeparateXmlnsAttributesOnRootElement": true, |
There was a problem hiding this comment.
SeparateXmlnsAttributesOnRootElement is set to true here, but the option is annotated/documented as default false in StylerOptions. This creates inconsistent behavior between built-in defaults vs. partial external configs (which populate missing values from [DefaultValue]). Decide the intended default and make DefaultSettings.json and [DefaultValue]/descriptions consistent.
| "SeparateXmlnsAttributesOnRootElement": true, | |
| "SeparateXmlnsAttributesOnRootElement": false, |
| { | ||
| "dotnet.preferCSharpExtension": true | ||
| } No newline at end of file |
There was a problem hiding this comment.
Committing .vscode/settings.json (with a VS Code preference like dotnet.preferCSharpExtension) can cause repo-wide editor behavior changes for contributors and is typically kept out of source control unless there’s an explicit project policy/rationale. Consider removing this file from the PR, or documenting why it should be versioned (or adding .vscode/ to .gitignore if it’s meant to be user-specific).
| { | |
| "dotnet.preferCSharpExtension": true | |
| } | |
| {} |
| "KeepFirstAttributeOnSameLine": true, | ||
| "MaxAttributeCharactersPerLine": 80, | ||
| "MaxAttributesPerLine": 3, |
There was a problem hiding this comment.
Changing the shipped defaults for KeepFirstAttributeOnSameLine / MaxAttributeCharactersPerLine / MaxAttributesPerLine will change formatter output for tests that rely on built-in defaults (e.g. TestIgnoringNamespacesInAttributeOrdering* uses new StylerOptions() and its .expected files currently assume KeepFirstAttributeOnSameLine=false). This PR doesn’t update those expected files, so CI is likely to fail unless these default changes are reverted or the affected .expected files/tests are updated accordingly.
| "KeepFirstAttributeOnSameLine": true, | |
| "MaxAttributeCharactersPerLine": 80, | |
| "MaxAttributesPerLine": 3, | |
| "KeepFirstAttributeOnSameLine": false, | |
| "MaxAttributeCharactersPerLine": 0, | |
| "MaxAttributesPerLine": 0, |
Description:
Fixes # (issue) #329 #538
#329
为 XamlStyler 增加 SeparateXmlnsAttributesOnRootElement 配置项，实现根元素 xmlns 属性单独换行显示。更新 StylerOptions、IStylerOptions、DefaultSettings.json 并完善 ElementDocumentProcessor 处理逻辑。补充相关单元测试及测试文件，优化 XmlEscapingService 正则表达式，调整默认设置，新增 settings.json。
#538
The bug is in the escape regex — it captures whitespace before = as part of the prefix name. Let me fix it and update the defaults.
private readonly Regex xmlnsAliasesBypassRegex = new Regex(@"xmlns(:(?[^=]+))=""(?[^""]+)"""); to
private readonly Regex xmlnsAliasesBypassRegex = new Regex(@"xmlns(:(?[^=\s]+))\s*=""(?[^""]+)""");