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

Add -Path parameter to Invoke-Formatter #775

Open
kapilmb opened this issue Jun 11, 2017 · 17 comments
Open

Add -Path parameter to Invoke-Formatter #775

kapilmb opened this issue Jun 11, 2017 · 17 comments

Comments

@kapilmb
Copy link

kapilmb commented Jun 11, 2017

Invoke-Formatter takes only a ScriptDefinition parameter. The user should be able to point the cmdlet to a file for formatting. However, the following things need to be preserved when formatting a file:

  • file encoding
  • new line characters
  • indentation type {tab, space}
@Kriegel
Copy link

Kriegel commented Feb 21, 2018

I had even no luck with Get-Content :-(
The following produces only chaos with or without -readcount
Invoke-Formatter -ScriptDefinition (Get-Content -Path 'C:\MyFile.ps1' -ReadCount 0)

@bergmeister
Copy link
Collaborator

@Kriegel You need to use the -Raw option instead because -ScriptDefinition expects a string:

Invoke-Formatter -ScriptDefinition (Get-Content -Path 'C:\MyFile.ps1' -Raw)

@Kriegel
Copy link

Kriegel commented Feb 21, 2018

@bergmeister
Thank you! -raw does it!

This works even fine fo me:

# Replace whole text in current file with formatted text
$psise.CurrentFile.Editor.Text = Invoke-Formatter $psise.CurrentFile.Editor.Text

testing with VSCode .......

@bergmeister
Copy link
Collaborator

@Kriegel You're welcome. I had many cases in the past where -Raw was the better option and recently even when having to write tests that not only work with PowerShell 5 but also older versions like PowerShell 4 and blogged about it here:
https://bracketsandbraces.blogspot.co.uk/2018/02/gotcha-when-developing-powershell-that.html

@bergmeister
Copy link
Collaborator

bergmeister commented Mar 11, 2018

I see that this issue has quite a few upvotes. Can anyone please provide details of a use case? Because VSCode has already integrated Invoke-Formatter, I do not understand why people want to use it this way, especially since using Get-Content -Raw and Set-Content in PowerShell seems to be OK as a workaround. I think stuff like preserving the encoding right, is going to be super hard.

@Kriegel
Copy link

Kriegel commented Mar 12, 2018

I can now live with Get-Content -Raw but it is ugly and was not intuitively to find out.

I work often with code found on the Internet even with Modules which have more then one .ps1 file.
and i have a repository of my own old scripts

Or you handover scripts from one Team to another ...
Or the Team formatting rules are changed ...
with -Path we can batch format and standardize those.

without a -Path Parameter Invoke-Formatter seems to be intended to be used interactive in a Editor only and not made for automating (batch runs).

Invoke-ScriptAnalyzer has a -Path Parameter as the main parameter so lets make the interfaces consistent.

because we can....

@bergmeister
Copy link
Collaborator

bergmeister commented Mar 16, 2018

@Kriegel Thanks for the feedback. This sounds more like we could at least document an automation snippet because gluing stuff together is what PowerShell is for. The reason why implementing this is hard is because the requirements (preservation of encoding, tabs/spaces) are quite difficult to achieve, I already struggled to preserve the encoding in all cases for the -Fix switch on Invoke-ScriptAnalyzer, which re-uses some of the low-level internals of Invoke-Formatter. Applying the formatting is quite quick and easy to do for one file via Ctrl+A and Ctrl+K+F in VSCode, whereas fixing individual warnings was a pain to do manually.
At the end of the day, you still need to validate what the formatter did (never trust software, see e.g. issue 826 for an example of a formatter bug, the formatter is a relatively new feature in PSSA), therefore applying the correction is still much less work compared to validating the correction.
Feel free to give it a go implementing it yourself though, PRs are very welcome.

@Kriegel
Copy link

Kriegel commented Mar 28, 2018

see: Default Encoding for PowerShell 6.0 is UTF-8 without a BOM except for New-ModuleManifest

So i think to Support the UTF-8 without a BOM is enough, if the file is not of this encoding throw an Error and all is done.

without batchfile Support Invoke-Formatter is in my Point of view a poor automating tool ...
So I have the Impression that PSScriptAnalyzer only adresses in Editor (GUI) support.
I suggest to widen it for CLI Support, because there is Need and possibility ....

PRs ? Sorry I am IT-Pro no C-sharpy ...

Offtopic:
i am seeing many PowerShell Modules which are stale and have no or no good Support by the community because C# is a bunker silo for many IT-pros and most C# devlopers do not Need or avoid PowerShell.
The development strategie of PowerShell Modules should be: less C# as possible and much PowerShell Code as possible.
See for example the module Plaster. File structure scaffolding with XML and C# instead use of the Item-xxx cmdlets are mental crazy!
so it is stale ...

the support to create custom rules with PowerShell script code in this module project, is well hidden and bad documented.
I think this Project can get much more contributions with a better support for PowerShell script code rules.

@felixfbecker
Copy link

It's important to have a way to check if all files in a folder are formatted in CI, and fail the build if some are not. Every formatter I've used had this feature.

Something like:

Get-ChildItem -Recurse '*.ps*' |
Get-Content -Raw |
ForEach-Object { Compare-Object $_ (Invoke-Formatter -ScriptDefinition $_) }

@bergmeister
Copy link
Collaborator

@felixfbecker You can, Invoke-ScriptAnalyzer supports that, the most common way is to use Pester integration in CI for PSSA checks. Either specify your custom format in a PSSA setting file and point the -Settings parameter to it or if you tab complete on this parameter you also get the option of choosing one of the built in ones that you know from VSCode.

@joaoe
Copy link

joaoe commented Dec 1, 2019

Hi.

Regarding support for InvokeFormatter -Path ....

  1. The default behavior should be to output the formatted content as a string, to avoid accidentally overwrite the file.
  2. Add an InPlace parameter to modify the file in place, instead of outputting the formatted contents, like sed.
  3. Add a Diff, DryRun or Test parameter that DOES NOT format anything but instead returns $True if any modifications would be done, or $False otherwise. This can then be used in git hooks, to verify the file was formatted before committing, or used in a CI job to check all modified files are properly formatted.

Thank you :)

@metablaster
Copy link

metablaster commented Jun 16, 2020

@bergmeister
I see that this issue has quite a few upvotes. Can anyone please provide details of a use case?

I'm also upvoting on this issue, and here is the reason why current implementation of InvokeFormatter isn't really useful.

  1. I have a bunch of scripts in my project, cca 100 of them,
  2. I want to execute formatter on entry project for all scripts. (ex. with -Recurse parameter) just like you execute Invoke-ScriptAnalyzer .\ -Recurse you also do Invoke-Formatter .\ -Recurse
  3. Expected behavior is that all scripts are recursively formatted according to settings in PSScriptAnalyzerSettings.psd1

I don't understand why anyone would like to want to retain anything as suggested in previous posts here, ie. indentation or encoding, these things should be configurable in PSScriptAnalyzerSettings.psd1 and the formatter runs according to these rules with no exceptions.

Yes there is default formatter in VSCode but but it's not recursive,
This project makes use of PSScriptAnalyzerSettings.psd1 which is far superior to default formatter if you are able to apply it recursively.

This issue is about -Path parameter, but if this is implemented then -Recurse should be implemented as well. both do the same thing anyway.

@bergmeister
Copy link
Collaborator

@metablaster Invoke-Formatter has a -Settings switch already. As mentioned before, you could still script it yourself at the moment with something like

Get-ChildItem -Recurse '*.ps*' |
     ForEach-Object { Set-Content -Path $_.FullName -Value (Invoke-Formatter -ScriptDefinition (Get-Content -Raw -Path $_.FullName) ) }

If you had the option: Would you prefer a Path option in Invoke-Formatter or rather a built in command into the vs-code extension as the latter might be easier to achieve?
Due to the layering in the engine internals, it's actually not that straightforward to add a -Path parameter set to it. Maybe something to be considered for PSSA v2 @rjmholt ?

@metablaster
Copy link

I'll try out the option with Get-ChildItem thanks, it's still better than manually checking formatting for each file.

I believe it's the same (depends on context) whether the recursive formatting is built into extension or into module, what matters is that there is a default way to actually do it when you need it.

Maybe I'm wrong but PSScriptAnalyzerSettings.psd1 seems to have more (or some unique) options than vs-code extension, so it would be natural to prefer Invoke-Formatter.

@bergmeister
Copy link
Collaborator

The vs-code extension basically has individual settings for (most) settings of the formatting rules that you can specify in the psd1, therefore I consider them equivalent. People use the settings file usually for code analysis rules (same for the settingsPath setting in the extension), hence why having a Format all documents in addition to the already existing Format Document vs-code command could potentially be useful to a wider audience.

@metablaster
Copy link

metablaster commented Aug 22, 2020

I just want to input my discovery that might help some people.

In VSCode use search function and search for word that is located inside ALL files, examples such as:
.SYNOPSIS, Copyright or regex Get-\w+

Then simply use "replace all" to replace all occurrences of that word, and repeat it once again to revert of course.

What will happen is that formatter will be invoked on your entry repository formatting all your scripts.
Requirement is that you have your PSScriptAnalzerSettings.psd1 in place and filled with settings.

Of course to minimize errors while doing so, add new comment into every script in your repository such as:

### PSScriptAnalyzerInvokeFormatter

Now use search function and replace all occurrences with:

## PSScriptAnalyzerInvokeFormatter

I find this so cool.

@Jiehong
Copy link

Jiehong commented Aug 18, 2023

@bergmeister

If you had the option: Would you prefer a Path option in Invoke-Formatter or rather a built in command into the vs-code extension as the latter might be easier to achieve? Due to the layering in the engine internals, it's actually not that straightforward to add a -Path parameter set to it. Maybe something to be considered for PSSA v2 @rjmholt ?

Not everyone uses vscode, and I think people are used to having a formatter that can fix the files for you in other languages (eg: go fmt /path/to/dir, cargo fmt, even non programming projects do that such as caddy with caddy fmt --override)

I think there is somewhat easy way to handle that: provide a new cmdlet named Invoke-ScriptFormatter, aligned with Invoke-ScriptAnalyzer.

Invoke-ScriptFormatter could just have its own strict limitations to avoid dealing with them:

  • force utf8;
  • override by default, unless -Dry-Run is provided;
  • delegate to Invoke-Formatter the actual formatting.

Just a QoL wrapper that woudn't require any change to Invoke-Formatter, and people could use if they want.

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