Fix PlaceOpenBrace rule correction to take comment at the end of line into account #929
Conversation
| corrections.Add( | ||
| new CorrectionExtent( | ||
| precedingExpression.Extent.StartLineNumber, | ||
| lCurly.Extent.EndLineNumber, | ||
| precedingExpression.Extent.StartColumnNumber, | ||
| lCurly.Extent.EndColumnNumber, | ||
| precedingExpression.Text + " " + lCurly.Text, | ||
| $"{precedingExpression.Text} {lCurly.Text}{optionalComment}", |
There was a problem hiding this comment.
without knowing too much about this project... should there be a space here between the }{:
{lCurly.Text}{optionalComment} for when optionalComment isn't empty string and actually is a comment?
There was a problem hiding this comment.
That is the point of having the separate optionalComment variable, if it is not an empty string, then the variable itself contains the preceding space character. This way I avoid appending a space character when there is no comment.
Tests/Rules/PlaceOpenBrace.tests.ps1
Outdated
| '@ | ||
| Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | Should -Be $expected | ||
| } | ||
|
|
There was a problem hiding this comment.
Is it possible to have the reverse of this?
We have:
foreach ($x in $y) { # PSSA is awesome
Get-Something
}
and want:
foreach ($x in $y) # PSSA is awesome
{
Get-Something
}
would it be worth having a test for that?
There was a problem hiding this comment.
This is a very good point. We should test this case as well, I have not thought of that.
There was a problem hiding this comment.
I added a test case for this now and added implicit integration tests with the code formatting style presets that VSCode uses.
…n the next line to a brace style where the brace is on the same line as suggested in the PR. Added more assertions for an integration test with code formatting styles
…nalyzer into FixPlaceOpenBraceWhenCommentIsPresent
…ext block (they screwed up following tests for some reason).
PR Summary
Fixes #826 which came indirectly via 2 raised VSCode issues.
Fixes #800
Basically when a comment was at the end of the line, then if the rule had settings to move the brace on the same line, it moved it at the end of the line AFTER the comment.
This was because the correction just took the last token. This PR checks if the last token is a comment and if so takes the second last token if possible and appends the comment after the opening brace (including a preceding space character).
Other than that some minor tidy up.
I have locally tested that this also works in PSCore.
PR Checklist
Note: Tick the boxes below that apply to this pull request by putting an
xbetween the square brackets. Please mark anything not applicable to this PRNA.WIP:to the beginning of the title and remove the prefix when the PR is ready