-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Allow numeric Ids and name of registered code pages in '-Encoding' parameters #7636
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
Conversation
cebaad2 to
edc51a7
Compare
|
@TravisEz13 Could you please look why PowerShell-CIs failed on Update-Help tests? (Nightly Travis CI failed too). |
|
there were connection issues to download.microsoft.com... The PR should be rebased. |
edc51a7 to
855dd2d
Compare
|
Rebased to fix CIs. |
| { | ||
| return System.Text.Encoding.GetEncoding(stringName); | ||
| } | ||
| case int intName: |
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.
Can you add some tests for the int case?
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.
Added.
TravisEz13
left a comment
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.
see comments
| BeforeAll { | ||
| $testValue = "ф" | ||
| if ($IsWindows) { | ||
| $expectedBytes = 244,13,10 -join "-" |
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.
Add a comment about what the characters are
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.
Added.
TravisEz13
left a comment
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.
One comment about the expected characters
|
@iSazonov Have you filed a doc issue? |
|
@TravisEz13 Doc issue was created. |
PR Summary
Address #6581.
The PR enhance the encoding transformation attribute:
Full list supported code pages we can found in
https://github.com/dotnet/corefx/blob/0b0f63d47951773cc46393312d80b40176d3a413/src/System.Text.Encoding.CodePages/src/System/Text/EncodingTable.Data.cs
We haven't encoding test at all so I tested the change locally.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.[feature]if the change is significant or affects feature tests