DTW-DanWard / PowerShell-Beautifier

A whitespace reformatter and code cleaner for Windows PowerShell and PowerShell Core
MIT License
333 stars 38 forks source link

Latest PowerShell Core: Cannot bind parameter 'Encoding'. Cannot convert the "byte" value of type "System.String" to type "System.Text.Encoding" #33

Closed DTW-DanWard closed 7 years ago

DTW-DanWard commented 7 years ago

PowerShell RC1: "byte" cast no longer working.

Get-DTWFileEncoding : Cannot bind parameter 'Encoding'. Cannot convert the "byte" value of type "System.String" to type "System.Text.Encoding". At C:\code\GitHub\PowerShell-Beautifier\src\DTW.PS.Beautifier.Main.psm1:1282 char:25

DTW-DanWard commented 7 years ago

This is a little more complicated than it looks. The file encoding detection function uses Get-Content "byte" encoding to read the first bytes of the file to detect what the file encoding is if no BOM is present - but "byte" no longer exists in the latest PowerShell Core build. Getting the correct encoding type is critical for the beautifier because knowing the correct type is required for the API that tokenizes the content.

stevenzeck commented 7 years ago

Possibly related to this pull? https://github.com/PowerShell/PowerShell/pull/5080

DTW-DanWard commented 7 years ago

Yep, that's definitely it. Looks like new param -AsByteStream gets the job done for PSCore. Will tweak tonight it tomorrow.

stevenzeck commented 7 years ago

OK, please also push to the PSGallery, that's where the Docker image is installing from.

DTW-DanWard commented 7 years ago

@szeck87 I got the fix in place for the encoding error up on master. No time to update the PSGallery tonight; I should have time to look into automating that as part of my build process this weekend.

Thank you for identifying and diagnosing this issue!

stevenzeck commented 7 years ago

Sounds good. I've tried building a Docker image via cloning the repo and importing, but it doesn't seem possible to do with the way PowerShell sessions work.

For automating the build, try https://github.com/RamblingCookieMonster/PSDeploy.

stevenzeck commented 7 years ago

Please let me know when PSGallery is updated.

stevenzeck commented 6 years ago

@DTW-DanWard any update on psgallery?

DTW-DanWard commented 6 years ago

@szeck87 Sorry for the delay, started a new job recently and it's consumed 100% of my time.

The version of PowerShell-Beautifier that's in PS Gallery was uploaded by someone else, not me. (I did check it when I first saw it to make sure that it wasn't a Trojan horse). I've asked the person that uploaded it, m.szadziul, to transfer ownership to me. Once I have ownership I'll upload the new version.

DTW-DanWard commented 6 years ago

@szeck87 Ownership has been transferred to me but I have a some restructuring to do to make my GitHub project more PS Gallery friendly. I hope to get these changes complete and published by Monday night, Tuesday night latest. Sorry again for the delay.

DTW-DanWard commented 6 years ago

@szeck87 I've made the changes and pushed up a copy to PS Gallery - get the latest version.

By the way, I posted a question on the Atom Beautifier a long time back (June 25th https://github.com/Glavin001/atom-beautify/issues/333). If a user is using the -StandardOutput parameter and a major error occurs while the beautifier is processing (most likely from PS format error) the error information is output to stdout. Please test your changes on a file with syntax errors; I can change the code to put the error text in stderr.

stevenzeck commented 6 years ago

@DTW-DanWard Can you send a link or post sample code where it will error out?

DTW-DanWard commented 6 years ago

@szeck87 No problem. So, quick background: the way the beautifier works is by taking an existing file and using a built-in PowerShell API to tokenize the content. The beautifier then takes the tokens and writes the content back to the file with better formatting. So if a PowerShell syntax error exists in the user's source file, the tokenizer API call with error out. I capture the syntax error and return it to the user.

First, the sample file:

PS C:\temp> cat .\badfile.ps1
# no end quote
Write-Output "asdf
PS C:\temp>

If you are running without the -StandardOutput param, it outputs the error like a normal cmdlet or function:

PS C:\temp> powershell.exe -NoProfile -Command "Import-Module C:\code\GitHub\PowerShell-Beautifier\PowerShell-Beautifier
.psd1; Edit-DTWBeautifyScript c:\temp\badfile.ps1"
Invoke-TokenizeSourceScriptContent : An error occurred; is there invalid PowerShell or some formatting / syntax issue
in the script? See error record below.
At C:\code\GitHub\PowerShell-Beautifier\src\DTW.PS.Beautifier.Main.psm1:1304 char:5
+     Invoke-TokenizeSourceScriptContent -EV Err
+     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [Write-Error], WriteErrorException
    + FullyQualifiedErrorId : Microsoft.PowerShell.Commands.WriteErrorException,Invoke-TokenizeSourceScriptContent

Invoke-TokenizeSourceScriptContent : The string is missing the terminator: ". Content: "asdf
, line: 2, column: 14
At C:\code\GitHub\PowerShell-Beautifier\src\DTW.PS.Beautifier.Main.psm1:1304 char:5
+     Invoke-TokenizeSourceScriptContent -EV Err
+     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [Write-Error], WriteErrorException
    + FullyQualifiedErrorId : Microsoft.PowerShell.Commands.WriteErrorException,Invoke-TokenizeSourceScriptContent

PS C:\temp>

The beautifier also supports the -StandardOutput param - which Glavin had requested to match the way other re-formatters work. If you use that param and the beautifier doesn't encounter any issues, it returns the cleaned up text via stdout instead of reformatting the file.

But if the beautifier encounters an error while -StandardOutput specified:

If you don't like this, I can change it, but let me show you an example first:

PS C:\temp> powershell.exe -NoProfile -Command "Import-Module C:\code\GitHub\PowerShell-Beautifier\PowerShell-Beautifier
.psd1; Edit-DTWBeautifyScript -StandardOutput c:\temp\badfile.ps1" 2>Err.txt 1>Out.txt
PS C:\temp> cat .\Err.txt
Syntax error: The string is missing the terminator: ".  content: "asdf
  line: 2  column: 14;
PS C:\temp> cat .\Out.txt
PS C:\temp>

So because of the error and -StandardOutput being specified, the error text is much shorter and in the error stream and nothing exists in standardoutput. My thinking was: if an error occurred the user really needs to be alerted and so if nothing is returned via stdout, that's a bad sign so whatever wrapper is using this would/should detect this and this in case check stderr and display that error info while keeping the original text in the editor, untouched. On the other hand, that might not be the best for you: perhaps it should just return the original, unmodified text as-is via stdout so you can always replace the contents of the code window with the results but you still monitor stderr and display any text that might appear there.

The other reason I designed it this way was in the event some error occurred in the basic reading of the file, even before the Tokenize API call. In that case it wouldn't be safe to return the original text via stdout as the text might be corrupted - and I wouldn't want the user to lose any work because of an error in the beautifier - so automatically overwriting the text with the contents of stdout is a bad idea.

Thoughts?

stevenzeck commented 6 years ago

What's the rationale for not modifying the file if no error occurs and -StandardOutput is specified? @Glavin001 do other beautifiers work that way? I'm trying to think of the best way to implement this, what I have right now uses a temp file to send over and read after it's beautified. Is it preferred to read the beautified text as a string versus the actual file?

Glavin001 commented 6 years ago

@szeck87 Ideally we can utilize standard input and standard output, and not require a file. However, using temp files are allowed. The API for Atom-Beautify (and will be for Unibeautify) is to take a string and return a string, with no side effects on the existing file. So a temp file is used in cases when third-party beautifiers do not support stdout.

stevenzeck commented 6 years ago

OK thanks. @DTW-DanWard I don't believe you can send a string into PowerShell-Beautifier can you? If not, I'm thinking of using a temp file for the input, but use the string from -StandardOutput to replace the contents of the editor. Then if it errors, don't change the contents of the file and report the error from stderr.

DTW-DanWard commented 6 years ago

No, it can't take a string in in part because it needs to read the original file to determine the file encoding.

So, do you need me to make a code change so that if an error occurs it returns the original file untouched in stdout? Or will you just monitor the stderr for error and ignore stdout if stderr has text?

stevenzeck commented 6 years ago

No I don't think that's necessary. If there is an error it will just throw the error from stderr and not even look at stdout, and the original file will be preserved. I have a "working" version of it integrated with atom-beautify, but I need to wait on a dependency update to properly support PowerShell Core before merging it for a release. It might work straight up on a Windows machine though.