GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||
GeezerVison is showing github.com — Fix PlaceOpenBrace/PlaceCloseBrace corrections leaving stray whitespace and misindented braces by Copilot · Pull Request #2262 · PowerShell/PSScriptAnalyzer Conversation
|
There was an error while loading. Please reload this page.
| Before | After |
|---|---|
if ($RegEx) { |
if ($RegEx) { |
About the issue's expected output: these settings run only the two brace rules. PSPlaceCloseBrace.NewLineAfter defaults to $true, so it puts else on its own line, and none of the enabled rules indents the body (that's PSUseConsistentIndentation). The "After" column is therefore the correct result. The CodeFormattingOTBS preset with IgnoreOneLineBlock = $false now produces exactly the output the issue expected:
if ($RegEx) { '-match' } else { '-like' }
PlaceOpenBrace, NewLineAfter correction: the correction extent now runs from { to the next token on the same line, so it also removes the whitespace after the brace.PlaceCloseBrace, BraceShouldBeOnNewLine correction: when the preceding token is on the same line, the extent now starts at that token's end, so it also removes the whitespace before }.PlaceCloseBrace.GetIndentation: the loop condition was k > 0, so it never checked the first token in the script. When the open brace was on the first line, } took its indentation from the 2nd token (( → 3 spaces). Changed to k >= 0. This also affects the NoEmptyLineBefore correction, which uses the same helper.PlaceCloseBrace, NewLineAfter correction: the extent now runs from } up to the branch keyword (else/elseif/catch/finally). The replacement is } + newline + the brace's indentation, so the keyword lines up with } and no leading space remains.ViolationText to be '} ' (brace plus the following space) because the extent now includes that space.} and else keep their indentation;IgnoreOneLineBlock = $false;
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…ce and wrong indentation Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
The correction can misalign else after a multiline condition by using the opening-brace line’s indentation instead of preserving the closing brace’s leading whitespace, and regression coverage is needed.
This PR addresses #2218 by refining brace-formatting corrections so splitting one-line blocks removes stray whitespace and aligns closing braces and branch keywords.
Changes:
| File | Description |
|---|---|
| Tests/Rules/PlaceOpenBrace.tests.ps1 | Verify that opening-brace corrections consume following whitespace. |
| Tests/Rules/PlaceCloseBrace.tests.ps1 | Update correction expectations and add formatting regressions. |
| Rules/PlaceOpenBrace.cs | Remove whitespace after { when inserting a newline. |
| Rules/PlaceCloseBrace.cs | Adjust correction ranges and indentation for closing braces and branches. |
Review effort: Balanced
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
The formatter uses continuation-line indentation for multiline conditions, causing correctly aligned } else { blocks to gain an extra-indented else that brace-only formatting does not repair.
In code that hasn't changed since last review
Rules/PlaceCloseBrace.cs:288
For a condition split as if ($a -and followed by an indented $b) {, GetIndentation() returns the continuation line's indentation rather than the closing brace's indentation. An already correctly aligned } else { therefore becomes } followed by an extra-indented else, which brace-only formatting does not repair. Use the closing brace's actual leading whitespace when it already starts its own line, falling back to the helper only for an inline brace.
Review effort: Balanced
Sorry, something went wrong.
There was an error while loading. Please reload this page.
* Initial plan * Preserve close brace indentation for branches Co-authored-by: bergmeister <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
The closing-brace rule replaces tab-based indentation with spaces when splitting a branch, misaligning else; it needs to preserve the original leading whitespace and include a tab-indented regression test.
Review effort: Balanced
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
* Initial plan * Preserve tab indentation when formatting branches Co-authored-by: bergmeister <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
No active review issues were identified for the brace-formatting corrections.
No open issues 🎉
Review effort: Balanced
Sorry, something went wrong.
There was an error while loading. Please reload this page.
At least 1 approving review is required to merge this pull request.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
IgnoreOneLineBlock=$false Correction Issue in PSPlaceOpenBrace and PSPlaceCloseBrace
There was an error while loading. Please reload this page.