Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align groups of assignments (=) #1029

Open
hanpq opened this issue Jun 27, 2018 · 14 comments
Open

Align groups of assignments (=) #1029

hanpq opened this issue Jun 27, 2018 · 14 comments

Comments

@hanpq
Copy link

hanpq commented Jun 27, 2018

I made the below feature request in the vscode-powershell extension git and was enlightened that this functionality originate from the PSScriptAnalyzer module. PowerShell/vscode-powershell#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

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

@sheldonhull
Copy link

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
Copy link
Collaborator

bergmeister commented Jul 8, 2018

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
Copy link

ryan-jan commented Aug 19, 2018

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
Copy link
Author

hanpq commented Oct 18, 2018

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
Copy link

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
Copy link
Author

hanpq commented Oct 18, 2018

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
Copy link
Collaborator

bergmeister commented Oct 18, 2018

@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
Copy link
Author

hanpq commented Oct 18, 2018

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

@bergmeister
Copy link
Collaborator

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

@hanpq
Copy link
Author

hanpq commented Oct 18, 2018

Ah ok :)

@kvprasoon
Copy link
Contributor

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

@RussPitcher
Copy link

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
Copy link

dseynhae commented Apr 7, 2022

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:

  • Reading the documentation, I had no idea that the rule doesn't apply to normal assignments:

    #Unaligned assignments not flagged:
    $var1 = "one"
    $variable2 = "two"
    $v3 = "three"
    $tmp = "four"
  • After a while, I figured out why I never got any warnings, even for my hashtables; With the configuration $enable = $true, the rule actually fixes the hashtables, so the problem never gets flagged:

    $hash = @{
      key1    = $var1
      keyfor2 = $variable2
      k3      = $v3
      key     = $tmp
    }

👉 I don't think that a linter/analyzer should automatically fix any violations!
👉 The configuration doesn't make sense:

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
Copy link

claudio-salvio commented Sep 16, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants