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 — Add UseConsistentCasing by Jaykul · Pull Request #1704 · PowerShell/PSScriptAnalyzer Conversation
|
|
Looks like I missed updating some docs/tests |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
c9fe6ea to
df1e30d
Compare
|
Hold off merging this, because I have realized that it does the wrong thing for a few operators. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the effort, looks ok from a high level.
But we already have a UseCorrectCasing rule, which is used for cmdlet casing at the moment but I think we could just add CheckOperator and CheckKeyword as options to it and a new option for CheckCmdlet so that user can pick and choose what they prefer. Keeping it in one rule will make things easier to integrate the new settings into the VS-Code extension, which already has scaffolding for UseCorrectCasing. Also, if you wanted the rule to be usable by the formatter and the vs-code extension, you'd have to register it here.
WDYT Rob Holt (@rjmholt)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, I meant to get back to this question earlier. I think it's better all in one rule, because:
All the code looks good and looks pretty portable at this stage, so hopefully it's just a case of sticking it inside of some conditional methods in |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I'm quite happy to merge them. This was a safer PR just because if you didn't want it, I could take my code and go home 😉 I did add this one to the formatter (you can see in the tests that I verified that worked). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Any updates on this PR? Would be nice to have this feature in vscode so it autoformats my keywords properly. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I quite forgot about it. I'll try to look at it again this week |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Reminder Joel Bennett (@Jaykul) that it's already next week 😂😂 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Any update on this PR? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Last chance Joel Bennett (@Jaykul) as we are starting to wrap up for next release this month |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
522aa79 to
7525ff1
Compare
|
It's been a long, long time... |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I have seen that this PR has gotten stale a bit again. Is there anything that could be done to help complete it? I came across this again, because I noticed that I lack an auto-formatter for casing in VS Code for my PowerShell scripts. Which is why it would be really cool to have. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Ping? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
If tests still pass, happy to finally merge it :-)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
It looks like the changes include configuration options. We need to document that. I am OK with accepting the PR as is if someone will add a comment to the PR explaining the configurating with an example. I can update the docs based on that.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| It "Does not throw when correcting certain cmdlets (issue 1516)" { | ||
| $scriptDefinition = 'Get-Content;Test-Path;Get-ChildItem;Get-Content;Test-Path;Get-ChildItem' | ||
| $settings = @{ 'Rules' = @{ 'PSUseCorrectCasing' = @{ 'Enable' = $true } } } | ||
| $settings = @{ 'Rules' = @{ 'PSUseCorrectCasing' = @{ 'Enable' = $true; CheckCommands = $true; CheckKeywords = $true; CheckOperators = $true } } } |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
These are the settings for the rule
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
That's an interesting request, Sean Wheeler (@sdwheeler) -- do we have documentation somewhere about the options? The settings are: 'PSUseCorrectCasing' = @{ Enable = $true CheckCommands = $true # Require the case of all commands to match their actual casing. CheckKeywords = $true # Require the case of all keywords to be lowercase. CheckOperators = $true # Require the case of all operators to be lowercase. } |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
What are the default values? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
f949196 to
3ba33ac
Compare
|
The default should be all |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Sorry, I went back and reviewed the code again. I see where you set the default values. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Christoph Bergmeister (@bergmeister) thoughts on merging this and doing a 1.3 release? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
3ba33ac to
3ba45d0
Compare
There was an error while loading. Please reload this page.
Andy Jordan (@andyleejordan) Yes to both. I assume you meant 1.24 release though? Feel free to message me if you have question or want to plan it. I am on Slack and the PWG (PowerShell Working Group) Discord. At release time, we also sync our doc changes into msdocs repos, which is also a good time to check whether there were any changes in msdocs that weren't backported to PSSA (Sean usually does it but as it's manual, it's easy to omit some) For consumers like PSES, we specify the rules and arguments explicitly and should set our own defaults. What I've usually done is add variable layer in PSES repo and in |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.