PowerShell / PSScriptAnalyzer

Download ScriptAnalyzer from PowerShellGallery
https://www.powershellgallery.com/packages/PSScriptAnalyzer/
MIT License
1.87k stars 378 forks source link

Align groups of assignments (=) #1029

Open hanpq opened 6 years ago

hanpq commented 6 years ago

I made the below feature request in the vscode-powershell extension git and was enlightened that this functionality originate from the PSScriptAnalyzer module. https://github.com/PowerShell/vscode-powershell/issues/1385

I would love to see an option to allow alignment of assignmentgroups. Its easiest to show what I mean with an example. (Github wouldn't allow me to format the code manually and trimmed the whitespaces automatically so I had to attach a printscreen).

![bild](https://user-images.githubusercontent.com/33247576/41896787-d51c55f8-7925-11e8-8036-5845b3c5ea0d.png)

So that '=' are aligned in columns within a group of assignments. A group could be interpreted as delimitered with a blank line.

sheldonhull commented 6 years ago

I do this manually worry alignment extension, but when applying the settings from this it eliminates the alignment, so I'd agree this would be nice to have.

bergmeister commented 6 years ago

There is already a rule called PSAlignAssignmentStatement for that, which maps to the VSCode settting powershell.codeFormatting.alignPropertyValuePairs but it only applies to assignment statements in a hashtable or a DSC Configuration. I think your proposal would best fit into the existing rule by adding an optional customisation to it.

ryan-jan commented 6 years ago

I'm interested in people's opinion on this. I personally think adding this many blank spaces is unnecessary and actually often ends up in increasing line length to the point that you have to break onto the line below. I would not agree that this is desirable behavior as the default.

hanpq commented 6 years ago

Personally I quite never experience that issue so it might be related to window width/editor width rather than the length of the line itself. I would say that it boils down to personal preference. Anyways my intent was that it would be an option, not necessarily the default behavior.

ryan-jan commented 6 years ago

I am talking about having to break line because you are sticking to a set line length, for example 115 characters, which is seen as a good practice generally in most programming languages.

hanpq commented 6 years ago

Of course, but I still rarely have issues with exceeding that line length, maybe naming could be the reason why you often exceed that length? Still I would argue that it comes down to personal preference and if the setting does not fit your way of coding you are free to enable/disable it.

bergmeister commented 6 years ago

@ryan-jan @hanpq VS-Code has the editor.rulers setting to visually show you if you exceed the recommended/maximum line length, if you want to enforce this in CI, then a Pester test would be a good solution for the moment as I do not see such a rule as high priority (but we are open to PRs so feel free to create such a rule as long as it does not get used by default, I am happy to help and give pointers).

hanpq commented 6 years ago

@bergmeister How do you mean? I'm afraid the discussion is drifting off topic, how is that connected to alignment of assignment groups?

bergmeister commented 6 years ago

@hanpq This was just a general comment if line length is a concern

hanpq commented 6 years ago

Ah ok :)

kvprasoon commented 5 years ago

I'm in 1.17.1 version and Invoke-Formatter is capable to do this . But Invoke-ScriptAnalyzer doesn't catch this.

RussPitcher commented 5 years ago

I'd like to be able to add more alignments like this. I rather like the BlockAlign extension (https://github.com/crewone/vscode-blockalign), but of course it doesn't play well with overall formatting which is a shame, if entirely understandable.

dseynhae commented 2 years ago

There is already a rule called PSAlignAssignmentStatement for that, which maps to the VSCode settting powershell.codeFormatting.alignPropertyValuePairs but it only applies to assignment statements in a hashtable or a DSC Configuration. I think your proposal would best fit into the existing rule by adding an optional customisation to it.

I actually have trouble interpreting the configuration of the PSAlignAssignmentStatement rule:

Enable CheckHashtable State
$false $false Nothing gets checked, nothing gets fixed
$true $false Nothing gets checked, yet hashtables get fixed
$false $true Nothing gets checked, yet hashtables get fixed
$true $true Hashtables get checked, and hashtables get fixed

The configuration variables don't map to expected behavior:

πŸ’‘ If Enable is false, the rule is should not be deployed. ❓ Yet it "fixes" hashtables for CheckHashtables set to $true... πŸ’‘ If Enable is true, then it should check both variables and hash tables. ❓ Yet it never checks variables, so why would even need the CheckHashtables configuration... ❓ Even if CheckHashtable is set to $false, the "rule" still fixes hashtables...

So what I expect, even after reading the documentation, is this:

Enable CheckHashtable Expected State (NOTHING GETS FIXED)
$false Don't Care Nothing gets checked
$true $false Variables get checked
$true $true Both Variables and Hashtables get checked

πŸ‘‰ Any automatic "fix" should be implemented in the Formatter, not the Analyzer!

claudio-salvio commented 2 months ago

Hi, πŸ‘‹

I know this is an issue that dates back a while. I'm coming back to it because I think vertical alignment is important to improve code readability.

I use VSCode and -so far- I haven't found any alignment extension that work properly with PowerShell .ps1 files. Some of the ones I tried align assignment sections well, positioning us in them or selecting them. Unfortunately they cause some problems by not properly interpreting comments or urls, or by treating the file as a whole and not detecting assignment blocks.

Please, it would be very helpful if someone with knowledge of the project could enable this functionality based on what was mentioned in the different posts on this issue and in some other issues such as the enum's related one. It would be great if VSCode's "format document" would handle this correctly using exclusively the PowerShell extension.

πŸ™Many thanks to everyone who has contributed to this useful project. Kind regards, Claudio Salvio