Fix NOTES section formatting in comment-based help#26512
Conversation
- Remove extra blank line after NOTES header - Fix double indentation (8 spaces to 4 spaces) - Add Pester tests to verify the fix The issue was caused by: 1. Two consecutive .AddNewline() calls creating extra blank line 2. Nested .StartFrame(leftIndent: 4) causing double indentation (4+4=8) Fixes PowerShell#26102
There was a problem hiding this comment.
Pull request overview
This PR fixes formatting issues in the comment-based help NOTES section that caused inconsistent spacing and indentation compared to other help sections. The fix removes an extra newline and eliminates double indentation by removing a nested frame.
Key Changes:
- Removed duplicate
.AddNewline()and nested.StartFrame(leftIndent: 4)in Help_format_ps1xml.cs - Added comprehensive Pester tests to verify NOTES section formatting matches other sections
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/System.Management.Automation/FormatAndOutput/DefaultFormatters/Help_format_ps1xml.cs | Fixed control16 definition by removing extra newline and nested indentation frame |
| test/powershell/Language/Scripting/ScriptHelp.NotesFormatting.Tests.ps1 | Added tests to verify NOTES section has single blank line and 4-space indentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…Tests.ps1 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
21ef920 to
3c25e01
Compare
|
@sdwheeler Could you please look the change? Can we accept it? |
| $lines[$notesIndex + 1] | Should -Match '^\s*$' -Because "There should be a blank line after NOTES header" | ||
|
|
||
| # The second line after NOTES header should have content (not another blank line) | ||
| # This was the bug: there were TWO blank lines instead of one |
There was a problem hiding this comment.
I'd prefer to remove line 25 at all.
| $lines[$notesIndex + 2] | Should -Match '\S' -Because "Content should start on the second line after NOTES header (bug was extra blank line)" | ||
| } | ||
|
|
||
| It 'NOTES content should have 4-space indentation, not 8 (double indentation bug)' { |
There was a problem hiding this comment.
The test duplicates code from previous test. So I suggest to merge these two tests.
There was a problem hiding this comment.
Done in 35b51dc. Merged both tests into one.
|
|
||
| # Count leading spaces - should be 4, not 8 | ||
| # Store capture group immediately to avoid issues with $matches being overwritten | ||
| if ($contentLine -match '^(\s+)') { |
There was a problem hiding this comment.
We could simply check that the line start with 4 space string $contentLine.StartsWith(" ").
There was a problem hiding this comment.
StartsWith 4 would match StartsWith 8
There was a problem hiding this comment.
@sdwheeler is right. I'm using -match '^[ ]{4}\S' instead, which verifies exactly 4 spaces followed by a non-whitespace character. (35b51dc)
PR Summary
Fixes #26102 - Comment-based help NOTES section had extra blank line and double indentation.
PR Context
The NOTES section in comment-based help had two formatting issues:
This made the NOTES section visually inconsistent with other help sections like INPUTS, OUTPUTS, etc.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerChanges
Code Changes
Help_format_ps1xml.csin thecontrol16definition:.AddNewline()call that created the second blank line.StartFrame(leftIndent: 4)and.EndFrame()that caused double indentationTest Changes
ScriptHelp.NotesFormatting.Tests.ps1with two Pester tests:Technical Details
Root Cause:
In
Help_format_ps1xml.cs, thecontrol16definition had:.AddNewline()callsStartFrame(leftIndent: 4)causing 4+4=8 space indentationBefore:
After:
Testing
Manual Testing:
Verified NOTES section now matches formatting of other sections.
Pester Tests:
Both tests pass with the fix: