-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Add -Name, -NoUserOverrides and -ListAvailable parameters to Get-Culture cmdlet #7702
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
Changes from all commits
2273dc5
7da8e8a
db0d588
cf67fd6
8f1a881
2b422cf
7ae1ed9
0e09fb1
dd53428
137e9b1
5b5efb8
f19339c
7987e93
3c35b6b
4cd2b1b
34b404a
173849d
14b8e7b
7bcee51
d00a899
c12d1a1
a54d534
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,124 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Globalization; | ||
| using System.Management.Automation; | ||
|
|
||
| namespace Microsoft.PowerShell.Commands | ||
| { | ||
| /// <summary> | ||
| /// Returns the thread's current culture. | ||
| /// Returns: | ||
| /// - the thread's current culture | ||
| /// - culture by name | ||
| /// - list of all supported cultures. | ||
| /// </summary> | ||
| [Cmdlet(VerbsCommon.Get, "Culture", HelpUri = "https://go.microsoft.com/fwlink/?LinkID=113312")] | ||
| [Cmdlet(VerbsCommon.Get, "Culture", DefaultParameterSetName = CurrentCultureParameterSet, HelpUri = "https://go.microsoft.com/fwlink/?LinkID=113312")] | ||
| [OutputType(typeof(System.Globalization.CultureInfo))] | ||
| public sealed class GetCultureCommand : PSCmdlet | ||
| { | ||
| private const string CurrentCultureParameterSet = "CurrentCulture"; | ||
| private const string NameParameterSet = "Name"; | ||
| private const string ListAvailableParameterSet = "ListAvailable"; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets culture names for which CultureInfo values are returned. | ||
| /// Empty string matches Invariant culture. | ||
| /// </summary> | ||
| [Parameter(ParameterSetName = NameParameterSet, Position = 0, ValueFromPipeline = true)] | ||
| [ValidateSet(typeof(ValidateCultureNamesGenerator))] | ||
| [ValidateNotNull] | ||
| public string[] Name { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets a switch to return current culture with user overrides (by default). | ||
| /// With the switch on, we return current culture without user overrides. | ||
| /// </summary> | ||
| [Parameter(ParameterSetName = CurrentCultureParameterSet)] | ||
| [Parameter(ParameterSetName = NameParameterSet)] | ||
| public SwitchParameter NoUserOverrides { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets a switch to list all available cultures. | ||
| /// </summary> | ||
| [Parameter(ParameterSetName = ListAvailableParameterSet)] | ||
| public SwitchParameter ListAvailable { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Output the current Culture info object. | ||
| /// Output: | ||
| /// - the thread's current culture | ||
| /// - culture by name | ||
| /// - list of all supported cultures. | ||
| /// </summary> | ||
| protected override void BeginProcessing() | ||
| protected override void ProcessRecord() | ||
| { | ||
| CultureInfo ci; | ||
|
|
||
| switch (ParameterSetName) | ||
| { | ||
| case CurrentCultureParameterSet: | ||
| if (NoUserOverrides) | ||
| { | ||
| ci = CultureInfo.GetCultureInfo(Host.CurrentCulture.Name); | ||
| } | ||
| else | ||
| { | ||
| ci = Host.CurrentCulture; | ||
| } | ||
|
|
||
| WriteObject(ci); | ||
|
|
||
| break; | ||
| case NameParameterSet: | ||
| try | ||
| { | ||
| foreach (var cultureName in Name) | ||
| { | ||
| if (!NoUserOverrides && string.Equals(cultureName, Host.CurrentCulture.Name, StringComparison.CurrentCultureIgnoreCase)) | ||
| { | ||
| ci = Host.CurrentCulture; | ||
| } | ||
| else | ||
| { | ||
| ci = CultureInfo.GetCultureInfo(cultureName); | ||
|
||
| } | ||
|
|
||
| WriteObject(ci); | ||
| } | ||
| } | ||
| catch (CultureNotFoundException exc) | ||
| { | ||
| WriteError(new ErrorRecord(exc, "ItemNotFoundException", ErrorCategory.ObjectNotFound, Name)); | ||
| } | ||
|
|
||
| break; | ||
| case ListAvailableParameterSet: | ||
| foreach (var cultureInfo in CultureInfo.GetCultures(CultureTypes.AllCultures)) | ||
| { | ||
| WriteObject(cultureInfo); | ||
| } | ||
|
|
||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Get list of valid culture names for ValidateSet attribute. | ||
| /// </summary> | ||
| public class ValidateCultureNamesGenerator : IValidateSetValuesGenerator | ||
| { | ||
| string[] IValidateSetValuesGenerator.GetValidValues() | ||
| { | ||
| WriteObject(Host.CurrentCulture); | ||
| var cultures = CultureInfo.GetCultures(CultureTypes.AllCultures); | ||
| var result = new List<string>(cultures.Length); | ||
| foreach (var cultureInfo in cultures) | ||
| { | ||
| result.Add(cultureInfo.Name); | ||
| } | ||
|
|
||
| return result.ToArray(); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,72 @@ | ||
| # Copyright (c) Microsoft Corporation. All rights reserved. | ||
| # Licensed under the MIT License. | ||
| Describe "Get-Culture DRT Unit Tests" -Tags "CI" { | ||
| It "Should works proper with get-culture" { | ||
| $results = get-Culture | ||
| $results -is "System.Globalization.CultureInfo" | Should -BeTrue | ||
| $results[0].Name | Should -Be $PSCulture | ||
| } | ||
| } | ||
|
|
||
| Describe "Get-Culture" -Tags "CI" { | ||
|
|
||
| It "Should return a type of CultureInfo for Get-Culture cmdlet" { | ||
|
|
||
| Get-Culture | Should -BeOfType CultureInfo | ||
| $culture = Get-Culture | ||
| $culture | Should -BeOfType [CultureInfo] | ||
| ($culture).EnglishName | Should -BeExactly $host.CurrentCulture.EnglishName | ||
|
|
||
| Get-Culture -NoUserOverrides | Should -BeOfType [CultureInfo] | ||
| } | ||
|
|
||
| It "Should have (Get-Culture).Name variable be equivalent to `$PSCulture" { | ||
|
|
||
| (Get-Culture).Name | Should -BeExactly $PsCulture | ||
| } | ||
|
|
||
| It "Should have $ culture variable be equivalent to (Get-Culture).Name" { | ||
| It "Should return the specified culture with '-Name' parameter" { | ||
|
|
||
| (Get-Culture).Name | Should -Be $PsCulture | ||
| $ci = Get-Culture -Name ru-RU | ||
| $ci | Should -BeOfType [CultureInfo] | ||
| $ci.Name | Should -BeExactly "ru-RU" | ||
|
|
||
| $ci = Get-Culture -Name ru-RU -NoUserOverrides | ||
| $ci | Should -BeOfType [CultureInfo] | ||
| $ci.Name | Should -BeExactly "ru-RU" | ||
| } | ||
|
|
||
| It "Should return specified cultures with '-Name' parameter" { | ||
|
|
||
| $ciArray = Get-Culture "", "ru-RU" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like we should have a specific test case to handle invariant culture making it clear what is being validated
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added the check and comment. |
||
| $ciArray | Should -HaveCount 2 | ||
| $ciArray[0] | Should -BeOfType [CultureInfo] | ||
| $ciArray[0].EnglishName | Should -BeExactly "Invariant Language (Invariant Country)" | ||
|
|
||
| $ciArray[1] | Should -BeOfType [CultureInfo] | ||
| $ciArray[1].Name | Should -BeExactly "ru-RU" | ||
| $ciArray[1].EnglishName | Should -BeExactly "Russian (Russia)" | ||
| } | ||
|
|
||
| It "Should accept values from a pipeline for '-Name' parameter" { | ||
|
|
||
| $ciArray = "", "ru-RU" | Get-Culture | ||
| $ciArray | Should -HaveCount 2 | ||
| $ciArray[0] | Should -BeOfType [CultureInfo] | ||
| $ciArray[0].EnglishName | Should -BeExactly "Invariant Language (Invariant Country)" | ||
| $ciArray[1] | Should -BeOfType [CultureInfo] | ||
| $ciArray[1].Name | Should -BeExactly "ru-RU" | ||
| $ciArray[1].EnglishName | Should -BeExactly "Russian (Russia)" | ||
| } | ||
|
|
||
| It "Should return the culture array with '-ListAvailable' parameter" { | ||
|
|
||
| $ciArray = Get-Culture -ListAvailable | ||
| $ciArray.Count | Should -BeGreaterThan 0 | ||
| $ciArray[0] | Should -BeOfType [CultureInfo] | ||
| } | ||
|
|
||
| It "Should write an error on unsupported culture name" { | ||
|
|
||
| { Get-Culture -Name "abcdefghijkl" -ErrorAction Stop } | Should -PassThru -Throw -ErrorId "ParameterArgumentValidationError,Microsoft.PowerShell.Commands.GetCultureCommand" | ||
| } | ||
| } | ||
|
|
||
| Describe "`$PSCulture" -Tags "CI" { | ||
|
|
||
| It "Check `$PSCulture value" { | ||
| $PSCulture | Should -BeExactly $([System.Globalization.CultureInfo]::CurrentCulture.Name) | ||
| } | ||
| } | ||
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.
Shouldn't this be ValidateNotNullOrEmpty?
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.
For empty string name
InvariantCultureis returned.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.
@iSazonov - I think that behavior's too subtle.
Additionally, the ProcessRecord doesn't match the class comment (Return's the thread's current culture). In short, reviewing the code appears to not support getting the current culture.
NOTE: Line 40 that compares Name to null 'looks' like dead code since name must be non-null for the Name parameter set. Having an explicit or fall through 'default' case statement will make this clearer.
I suggest the following:
1: Retain the original behavior explicitly
Set the DefaultParameterSetName to 'default' and return the thread's current culture.
2: Name parameter set
Either clearly document that an empty string returns InvariantCulture or don't allow empty strings and require explicit names.
3: ListAvailable
As implemented.
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.
I too think that
""is obscure and suggest requiring explicit use ofInvariant-Culture, i.e.:Get-Culture Invariant-CultureThere 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.
1 - Fixed.
2- Let's consider the scenario - script asks user to input a culture, user press Enter to choose "Default". Without user typing the script get empty string as "default". I guess CoreFX consider InvariantCulture as default if culture is unknown or not selected. We use InvariantCulture widely in PowerShell code too. So allowing empty string in Get-Culture looks good.
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.
Re 2: Curiously, the empty string is not just a placeholder that signals the intent to retrieve the invariant culture when you call
.GetCultureInfo(), it is the actual value of the.Nameproperty of the culture-info object returned - so I guess it makes sense to surface that.(Unlike what I thought earlier,"Invariant-Culture"doesn't actually work; on Unix it implicitly creates an ad-hoc culture, on Windows it breaks.)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.
Perhaps add a comment about this in the code
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.