Skip to content

Allow numeric Ids and name of registered code pages in '-Encoding' parameters - #7636

Merged
Ilya (iSazonov) merged 4 commits into
PowerShell:masterfrom
iSazonov:encoding-enhance
Sep 19, 2018
Merged

Allow numeric Ids and name of registered code pages in '-Encoding' parameters#7636
Ilya (iSazonov) merged 4 commits into
PowerShell:masterfrom
iSazonov:encoding-enhance

Conversation

@iSazonov

@iSazonov Ilya (iSazonov) commented Aug 27, 2018

Copy link
Copy Markdown
Collaborator

PR Summary

Address #6581.

The PR enhance the encoding transformation attribute:

  • to allow numeric Ids of registered code pages (like -Encoding 1251)
  • to allow string names of registered code pages (like -Encoding "windows-1251")

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.

Set-Content -Path .\test1.txt -Value "я" -Encoding 1251
Set-Content -Path .\test2.txt -Value "я" -Encoding "windows-1251"

PR Checklist

@iSazonov

Ilya (iSazonov) commented Sep 10, 2018

Copy link
Copy Markdown
Collaborator Author

Travis Plunk (@TravisEz13) Could you please look why PowerShell-CIs failed on Update-Help tests? (Nightly Travis CI failed too).

@TravisEz13

Copy link
Copy Markdown
Member

there were connection issues to download.microsoft.com... The PR should be rebased.

@iSazonov

Copy link
Copy Markdown
Collaborator Author

Rebased to fix CIs.

{
return System.Text.Encoding.GetEncoding(stringName);
}
case int intName:

Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

@TravisEz13 Travis Plunk (TravisEz13) left a comment

Copy link
Copy Markdown
Member

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 "-"

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

@TravisEz13 Travis Plunk (TravisEz13) left a comment

Copy link
Copy Markdown
Member

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 Ilya (iSazonov) added the Documentation Needed in this repo Documentation is needed in this repo label Sep 18, 2018
@TravisEz13

Copy link
Copy Markdown
Member

Ilya (@iSazonov) Have you filed a doc issue?

@iSazonov

Copy link
Copy Markdown
Collaborator Author

Travis Plunk (@TravisEz13) Doc issue was created.

@iSazonov
Ilya (iSazonov) merged commit cc5f36c into PowerShell:master Sep 19, 2018
@iSazonov
Ilya (iSazonov) deleted the encoding-enhance branch September 19, 2018 13:53
@joeyaiello Joey Aiello (joeyaiello) removed Documentation Needed in this repo Documentation is needed in this repo labels Oct 15, 2018
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
…rameters (PowerShell#7636)

Enhance the encoding transformation attribute:
- to allow numeric Ids of registered code pages (like -Encoding 1251)
- to allow string names of registered code pages (like -Encoding "windows-1251")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants