- Notifications
You must be signed in to change notification settings - Fork407
Fix PSUseConsistentIndentation.PipelineIndentation.None to not remove code when the previous line ended with a pipe#1746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
andyleejordan left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thank you for working on this@bergmeister. I'm wondering if we need a few more tests?#1580 provided several examples, and also pointed out that this issue occurs with bothNone and strangely a setting of3. What do you think?
I can spend some time to manually test this if you'd like and perhaps come up with more test coverage.
bergmeister commentedNov 22, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Yes, testing is always a challenge. It's a compromise between doing lots of integration tests or complex code and maintaining all the test cases if a change necessitates updating expectations. Definitely happy to add more test cases or something more complex, do you have something specific in mind? Otherwise my suggestions is to dog food the local build with this setting. PSScriptAnalyzer/Rules/UseConsistentIndentation.cs Lines 61 to 95 in58c4423
|
andyleejordan left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
With regards to the repro also working for 3, this is in fact the same setting, it is just the enum representation of it:
Gotcha! In that case, I'll defer to your judgement. Looks good to me!
PR Summary
Fixes#1580 by resetting the newline in the case of the first token after a pipeline and newline lands in the
defaultswitch statement. It seems the existing test case was too simple.PR Checklist
.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.