Skip to content

Unify cmdlets with parameter 'Encoding' to be of type System.Text.Encoding #5080

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.

Sign up for GitHub

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

Merged
daxian-dbw merged 8 commits into from
Oct 24, 2017
Merged

Unify cmdlets with parameter 'Encoding' to be of type System.Text.Encoding #5080

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bcc2719
Unify cmdlets with parameter 'Encoding' to be of type System.Text.Enc…
JamesWTruher Oct 2, 2017
431370a
Update to follow coding style guidelines
JamesWTruher Oct 12, 2017
53b31e9
Fix cmdlet casing to follow guidelines
JamesWTruher Oct 12, 2017
6ef8580
Change the encoding to utf8 when the converter gets an empty string
JamesWTruher Oct 13, 2017
e29baac
Change Encoding to remove byte as a possible encoding value
JamesWTruher Oct 16, 2017
b59b0ee
Changed -Byte parameter to -AsByteStream
JamesWTruher Oct 19, 2017
840343d
put ValidateNotNullOrEmpty attribute on Encoding parameters
JamesWTruher Oct 20, 2017
007bae4
Change from IArgumentCompleter attribute to ArgumentCompletions attri…
JamesWTruher Oct 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions src/Microsoft.PowerShell.Commands.Utility/commands/utility/CSVCommands.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,20 @@ public SwitchParameter NoClobber
/// Encoding optional flag
/// </summary>
[Parameter()]
[ValidateSetAttribute(new string[] { "Unicode", "UTF7", "UTF8", "ASCII", "UTF32", "BigEndianUnicode", "Default", "OEM" })]
public string Encoding { get; set; }
[ArgumentToEncodingTransformationAttribute()]
Copy link
Member

@daxian-dbw daxian-dbw Oct 19, 2017

Choose a reason for hiding this comment

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

I think we need to add AttributeNotNullOrEmpty attribute. Before this change, $null value would be rejected because the validate attribute will fail. After this change, user is able to pass in a null value.

[ArgumentCompletions(
EncodingConversion.Ascii,
EncodingConversion.BigEndianUnicode,
EncodingConversion.OEM,
EncodingConversion.Unicode,
EncodingConversion.Utf7,
EncodingConversion.Utf8,
EncodingConversion.Utf8Bom,
EncodingConversion.Utf8NoBom,
EncodingConversion.Utf32
)]
Copy link
Member

@daxian-dbw daxian-dbw Oct 23, 2017

Choose a reason for hiding this comment

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

How about use ArgumentCompletions(EncodingConversion.TabCompletionResults) here? It would be easy for future updates too.

Copy link
Member

@daxian-dbw daxian-dbw Oct 23, 2017

Choose a reason for hiding this comment

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

Same comments for other instances of ArgumentCompletions usage.

Copy link
Collaborator Author

@JamesWTruher JamesWTruher Oct 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

that doesn't work, because arrays aren't constant (I tried that first)

Copy link
Member

@daxian-dbw daxian-dbw Oct 23, 2017

Choose a reason for hiding this comment

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

OK, then we are good here. #Closed

[ValidateNotNullOrEmpty]
public Encoding Encoding { get; set; } = ClrFacade.GetDefaultEncoding();

/// <summary>
/// Property that sets append parameter.
Expand Down Expand Up @@ -373,7 +385,7 @@ private void CreateFileStream()
PathUtils.MasterStreamOpen(
this,
this.Path,
Encoding ?? "ASCII",
Encoding,
false, // defaultEncoding
Append,
Force,
Expand Down Expand Up @@ -577,8 +589,20 @@ public SwitchParameter UseCulture
/// Encoding optional flag
/// </summary>
[Parameter()]
[ValidateSetAttribute(new[] { "Unicode", "UTF7", "UTF8", "ASCII", "UTF32", "BigEndianUnicode", "Default", "OEM" })]
public string Encoding { get; set; }
[ArgumentToEncodingTransformationAttribute()]
[ArgumentCompletions(
EncodingConversion.Ascii,
EncodingConversion.BigEndianUnicode,
EncodingConversion.OEM,
EncodingConversion.Unicode,
EncodingConversion.Utf7,
EncodingConversion.Utf8,
EncodingConversion.Utf8Bom,
EncodingConversion.Utf8NoBom,
EncodingConversion.Utf32
)]
[ValidateNotNullOrEmpty]
public Encoding Encoding { get; set; } = ClrFacade.GetDefaultEncoding();

/// <summary>
/// Avoid writing out duplicate warning messages when there are
Expand Down
21 changes: 13 additions & 8 deletions ...oft.PowerShell.Commands.Utility/commands/utility/FormatAndOutput/format-hex/Format-Hex.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,20 @@ public sealed class FormatHex : PSCmdlet
/// Type of character encoding for InputObject
/// </summary>
[Parameter(ParameterSetName = "ByInputObject")]
[ValidateSetAttribute(new string[] {
EncodingConversion.Unicode,
[ArgumentToEncodingTransformationAttribute()]
[ArgumentCompletions(
EncodingConversion.Ascii,
EncodingConversion.BigEndianUnicode,
EncodingConversion.Utf8,
EncodingConversion.OEM,
EncodingConversion.Unicode,
EncodingConversion.Utf7,
EncodingConversion.Utf32,
EncodingConversion.Ascii})]
public string Encoding { get; set; } = "Ascii";
EncodingConversion.Utf8,
EncodingConversion.Utf8Bom,
EncodingConversion.Utf8NoBom,
EncodingConversion.Utf32
)]
[ValidateNotNullOrEmpty]
public Encoding Encoding { get; set; } = ClrFacade.GetDefaultEncoding();

/// <summary>
/// This parameter is no-op
Expand Down Expand Up @@ -239,8 +245,7 @@ private void ProcessObjectContent(PSObject inputObject)
else if (obj is string)
{
string inputString = obj.ToString();
Encoding resolvedEncoding = EncodingConversion.Convert(this, Encoding);
inputBytes = resolvedEncoding.GetBytes(inputString);
inputBytes = Encoding.GetBytes(inputString);
}

else if (obj is byte)
Expand Down
32 changes: 14 additions & 18 deletions ...crosoft.PowerShell.Commands.Utility/commands/utility/FormatAndOutput/out-file/Out-File.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
--********************************************************************/

using System;
using System.Text;
using System.Management.Automation;
using System.Management.Automation.Internal;
using System.Management.Automation.Host;
Expand Down Expand Up @@ -72,25 +73,20 @@ public string LiteralPath
/// </summary>
///
[Parameter(Position = 1)]
[ValidateNotNullOrEmpty]
Copy link
Member

@daxian-dbw daxian-dbw Oct 19, 2017

Choose a reason for hiding this comment

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

I think we still need this attribute, so that user cannot pass in a null value.

[ValidateSetAttribute(new string[] {
EncodingConversion.Unknown,
EncodingConversion.String,
EncodingConversion.Unicode,
[ArgumentToEncodingTransformationAttribute()]
[ArgumentCompletions(
EncodingConversion.Ascii,
EncodingConversion.BigEndianUnicode,
EncodingConversion.Utf8,
EncodingConversion.OEM,
EncodingConversion.Unicode,
EncodingConversion.Utf7,
EncodingConversion.Utf32,
EncodingConversion.Ascii,
EncodingConversion.Default,
EncodingConversion.OEM })]
public string Encoding
{
get { return _encoding; }
set { _encoding = value; }
}

private string _encoding;
EncodingConversion.Utf8,
EncodingConversion.Utf8Bom,
EncodingConversion.Utf8NoBom,
EncodingConversion.Utf32
)]
[ValidateNotNullOrEmpty]
public Encoding Encoding { get; set; } = ClrFacade.GetDefaultEncoding();

/// <summary>
/// Property that sets append parameter.
Expand Down Expand Up @@ -196,7 +192,7 @@ private LineOutput InstantiateLineOutputInterface()
PathUtils.MasterStreamOpen(
this,
FilePath,
_encoding,
Encoding,
false, // defaultEncoding
Append,
Force,
Expand Down
29 changes: 15 additions & 14 deletions src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,19 +77,20 @@ public SwitchParameter Force
/// Encoding optional flag
/// </summary>
[Parameter]
[ValidateSetAttribute(new string[] { "Unicode", "UTF7", "UTF8", "ASCII", "UTF32", "BigEndianUnicode", "Default", "OEM" })]
public string Encoding
{
get
{
return _encoding.GetType().Name;
}
set
{
_encoding = EncodingConversion.Convert(this, value);
}
}
private Encoding _encoding = System.Text.Encoding.UTF8;
[ArgumentToEncodingTransformationAttribute()]
Copy link
Member

@daxian-dbw daxian-dbw Oct 19, 2017

Choose a reason for hiding this comment

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

Need ValidateNullOrEmpty.

Copy link
Collaborator Author

@JamesWTruher JamesWTruher Oct 23, 2017

Choose a reason for hiding this comment

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

fixed

[ArgumentCompletions(
EncodingConversion.Ascii,
EncodingConversion.BigEndianUnicode,
EncodingConversion.OEM,
EncodingConversion.Unicode,
EncodingConversion.Utf7,
EncodingConversion.Utf8,
EncodingConversion.Utf8Bom,
EncodingConversion.Utf8NoBom,
EncodingConversion.Utf32
)]
[ValidateNotNullOrEmpty]
public Encoding Encoding { get; set; } = ClrFacade.GetDefaultEncoding();

#endregion Parameters

Expand Down Expand Up @@ -144,7 +145,7 @@ protected override void BeginProcessing()
List<string> generatedFiles = GenerateProxyModule(
tempDirectory,
Path.GetFileName(directory.FullName),
_encoding,
Encoding,
_force,
listOfCommandMetadata,
alias2resolvedCommandName,
Expand Down
34 changes: 13 additions & 21 deletions src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
--********************************************************************/

using System;
using System.Text;
using System.Text.RegularExpressions;
using System.IO;
using System.Collections;
Expand Down Expand Up @@ -1201,19 +1202,20 @@ public SwitchParameter AllMatches
/// The text encoding to process each file as.
/// </summary>
[Parameter]
[ValidateNotNullOrEmpty]
[ValidateSetAttribute(new string[] {
[ArgumentToEncodingTransformationAttribute()]
[ArgumentCompletions(
EncodingConversion.Ascii,
EncodingConversion.BigEndianUnicode,
EncodingConversion.OEM,
EncodingConversion.Unicode,
EncodingConversion.Utf7,
EncodingConversion.Utf8,
EncodingConversion.Utf32,
EncodingConversion.Ascii,
EncodingConversion.BigEndianUnicode,
EncodingConversion.Default,
EncodingConversion.OEM })]
public string Encoding { get; set; }

private System.Text.Encoding _textEncoding;
EncodingConversion.Utf8Bom,
EncodingConversion.Utf8NoBom,
EncodingConversion.Utf32
)]
[ValidateNotNullOrEmpty]
public Encoding Encoding { get; set; } = ClrFacade.GetDefaultEncoding();

/// <summary>
/// The number of context lines to collect. If set to a
Expand Down Expand Up @@ -1282,16 +1284,6 @@ public SwitchParameter AllMatches
/// </summary>
protected override void BeginProcessing()
{
// Process encoding switch.
if (Encoding != null)
{
_textEncoding = EncodingConversion.Convert(this, Encoding);
}
else
{
_textEncoding = new System.Text.UTF8Encoding();
}

if (!_simpleMatch)
{
RegexOptions regexOptions = (_caseSensitive) ? RegexOptions.None : RegexOptions.IgnoreCase;
Expand Down Expand Up @@ -1434,7 +1426,7 @@ private bool ProcessFile(string filename)

using (FileStream fs = new FileStream(filename, FileMode.Open, FileAccess.Read, FileShare.ReadWrite))
{
using (StreamReader sr = new StreamReader(fs, _textEncoding))
using (StreamReader sr = new StreamReader(fs, Encoding))
Copy link
Contributor

@dantraMSFT dantraMSFT Oct 19, 2017

Choose a reason for hiding this comment

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

There's an ArgumentNullException path here if Encoding is null.

Copy link
Collaborator Author

@JamesWTruher JamesWTruher Oct 20, 2017

Choose a reason for hiding this comment

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

fixed with [ValidateNotNullOrEmpty] attribute

Copy link
Collaborator Author

@JamesWTruher JamesWTruher Oct 20, 2017

Choose a reason for hiding this comment

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

fixed with attribute

{
String line;
int lineNo = 0;
Expand Down
62 changes: 17 additions & 45 deletions src/Microsoft.PowerShell.Commands.Utility/commands/utility/Send-MailMessage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,20 +89,24 @@ public SwitchParameter BodyAsHtml

/// <summary>
/// Specifies the encoding used for the content of the body and also the subject.
/// This is set to ASCII to ensure there are no problems with any email server
/// </summary>
[Parameter()]
[Alias("BE")]
[ValidateNotNullOrEmpty]
[ArgumentToEncodingNameTransformationAttribute()]
public Encoding Encoding
{
get { return _encoding; }
set
{
_encoding = value;
}
}
private Encoding _encoding = new ASCIIEncoding();
[ArgumentCompletions(
EncodingConversion.Ascii,
EncodingConversion.BigEndianUnicode,
EncodingConversion.OEM,
EncodingConversion.Unicode,
EncodingConversion.Utf7,
EncodingConversion.Utf8,
EncodingConversion.Utf8Bom,
EncodingConversion.Utf8NoBom,
EncodingConversion.Utf32
)]
[ArgumentToEncodingTransformationAttribute()]
public Encoding Encoding { get; set; } = Encoding.ASCII;

/// <summary>
/// Specifies the address collection that contains the
Expand Down Expand Up @@ -369,8 +373,8 @@ protected override
_mMailMessage.Body = _body;

//set the subject and body encoding
_mMailMessage.SubjectEncoding = _encoding;
_mMailMessage.BodyEncoding = _encoding;
_mMailMessage.SubjectEncoding = Encoding;
_mMailMessage.BodyEncoding = Encoding;

// Set the format of the mail message body as HTML
_mMailMessage.IsBodyHtml = _bodyashtml;
Expand Down Expand Up @@ -490,37 +494,5 @@ protected override void EndProcessing()
#endregion
}

/// <summary>
/// To make it easier to specify -Encoding parameter, we add an ArgumentTransformationAttribute here.
/// When the input data is of type string and is valid to be converted to System.Text.Encoding, we do
/// the conversion and return the converted value. Otherwise, we just return the input data.
/// </summary>
internal sealed class ArgumentToEncodingNameTransformationAttribute : ArgumentTransformationAttribute
{
public override object Transform(EngineIntrinsics engineIntrinsics, object inputData)
{
string encodingName;
if (LanguagePrimitives.TryConvertTo<string>(inputData, out encodingName))
{
if (string.Equals(encodingName, EncodingConversion.Unknown, StringComparison.OrdinalIgnoreCase) ||
string.Equals(encodingName, EncodingConversion.String, StringComparison.OrdinalIgnoreCase) ||
string.Equals(encodingName, EncodingConversion.Unicode, StringComparison.OrdinalIgnoreCase) ||
string.Equals(encodingName, EncodingConversion.BigEndianUnicode, StringComparison.OrdinalIgnoreCase) ||
string.Equals(encodingName, EncodingConversion.Utf8, StringComparison.OrdinalIgnoreCase) ||
string.Equals(encodingName, EncodingConversion.Utf7, StringComparison.OrdinalIgnoreCase) ||
string.Equals(encodingName, EncodingConversion.Utf32, StringComparison.OrdinalIgnoreCase) ||
string.Equals(encodingName, EncodingConversion.Ascii, StringComparison.OrdinalIgnoreCase) ||
string.Equals(encodingName, EncodingConversion.Default, StringComparison.OrdinalIgnoreCase) ||
string.Equals(encodingName, EncodingConversion.OEM, StringComparison.OrdinalIgnoreCase))
{
// the encodingName is guaranteed to be valid, so it is safe to pass null to method
// Convert(Cmdlet cmdlet, string encoding) as the value of 'cmdlet'.
return EncodingConversion.Convert(null, encodingName);
}
}
return inputData;
}
}

#endregion
}
}
16 changes: 14 additions & 2 deletions src/Microsoft.PowerShell.Commands.Utility/commands/utility/XmlCommands.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,20 @@ public SwitchParameter NoClobber
/// </summary>
///
[Parameter]
[ValidateSetAttribute(new string[] { "Unicode", "UTF7", "UTF8", "ASCII", "UTF32", "BigEndianUnicode", "Default", "OEM" })]
public string Encoding { get; set; } = "Unicode";
[ArgumentToEncodingTransformationAttribute()]
[ArgumentCompletions(
EncodingConversion.Ascii,
EncodingConversion.BigEndianUnicode,
EncodingConversion.OEM,
EncodingConversion.Unicode,
EncodingConversion.Utf7,
EncodingConversion.Utf8,
EncodingConversion.Utf8Bom,
EncodingConversion.Utf8NoBom,
EncodingConversion.Utf32
)]
[ValidateNotNullOrEmpty]
public Encoding Encoding { get; set; } = ClrFacade.GetDefaultEncoding();

#endregion Command Line Parameters

Expand Down
Loading