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
Encoding for New-ModuleManifest on all platforms should be UTF-8 NoBOM #5923
Encoding for New-ModuleManifest on all platforms should be UTF-8 NoBOM #5923
Conversation
@@ -941,11 +941,7 @@ protected override void EndProcessing() | |||
PathUtils.MasterStreamOpen( | |||
this, | |||
filePath, | |||
#if UNIX | |||
new UTF8Encoding(false), // UTF-8, no BOM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would prefer using named parameter instead of a comment about no BOM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will update
{ | ||
$ExpectedManifestBytes = @(35,10) | ||
} | ||
$ExpectedManifestBytes = @(35,10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In BeforeEach, New-Item
should pipe to $null. GitHub won't let me comment on that line since it is not near the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will add
false | ||
cmdlet : this, | ||
filePath : filePath, | ||
resolvedEncoding : new UTF8Encoding(false), // UTF-8, no BOM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be new UTF8Encoding(encoderShouldEmitUTF8Identifier: false)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will update, missed that one
PR Summary
When we decided not to take #4119 it meant that the file encoding for
New-ModuleManifest
on Windows stayed as UTF-16 w/ BOM.We had decided to adopt UTF-8 NoBOM as standard in PSCore6.
The fix is to remove Windows specific logic in the code and in the test for UTF-16 w/ BOM.
PR Checklist
Note: Please mark anything not applicable to this PR
NA
.[feature]
if the change is significant or affects feature testsWIP:
to the beginning of the title and remove the prefix when the PR is ready.