-
Notifications
You must be signed in to change notification settings - Fork 11
Get-DscProperty
: Add optional switch parameter IgnoreZeroEnumValue
#151
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
@johlju, I think IgnoreZeroEnumValue requires HasValue to also be set. |
Get-DscProperty
: Add optional switch parameter IgnoreZeroEnumValue
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.
Yes agree. I would not expect it to remove the property from the result when -HasValue
is not used. 🤔 Should we throw an ArgumentException if IgnoreZeroEnumValue
is used without HasValue
?
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @dan-hughes)
I added a ParameterSet to solve this. |
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.
Much better idea. But I think you need two parameter sets.
Currently it only gives this syntax
Get-DscProperty -InputObject <PSObject> [-Name <String[]>] [-ExcludeName
<String[]>] [-Attribute <String[]>] -HasValue [-IgnoreZeroEnumValue]
[<CommonParameters>]
Which makes the HasValue mandatory, always:
PS > Get-DscProperty
cmdlet Get-DscProperty at command pipeline position 1
Supply values for the following parameters:
InputObject: 'String'
HasValue:
I think we need
[CmdletBinding(DefaultParameterSetName = 'Default')]
[OutputType([System.Collections.Hashtable])]
param
(
[Parameter(Mandatory = $true, ValueFromPipeline = $true, ParameterSetName = 'Default')]
[Parameter(Mandatory = $true, ValueFromPipeline = $true, ParameterSetName = 'HasValue')]
[PSObject]
$InputObject,
[Parameter(ParameterSetName = 'Default')]
[Parameter(ParameterSetName = 'HasValue')]
[System.String[]]
$Name,
[Parameter(ParameterSetName = 'Default')]
[Parameter(ParameterSetName = 'HasValue')]
[System.String[]]
$ExcludeName,
[Parameter(ParameterSetName = 'Default')]
[Parameter(ParameterSetName = 'HasValue')]
[ValidateSet('Key', 'Mandatory', 'NotConfigurable', 'Optional')]
[Alias('Type')]
[System.String[]]
$Attribute,
[Parameter(ParameterSetName = 'HasValue', Mandatory = $true)]
[System.Management.Automation.SwitchParameter]
$HasValue,
[Parameter(ParameterSetName = 'HasValue')]
[System.Management.Automation.SwitchParameter]
$IgnoreZeroEnumValue
)
which gives:
Get-DscProperty -InputObject <PSObject> [-Name <String[]>] [-ExcludeName <String[]>] [-Attribute <String[]>] [<CommonParameters>]
Get-DscProperty -InputObject <PSObject> [-Name <String[]>] [-ExcludeName <String[]>] [-Attribute <String[]>] -HasValue [-IgnoreZeroEnumValue] [<CommonParameters>]
Then default works:
PS C:\Users\JohanLjunggren> Get-DscProperty
cmdlet Get-DscProperty at command pipeline position 1
Supply values for the following parameters:
InputObject: 'string'
And using the optional param forces HasValue:
PS > Get-DscProperty -IgnoreZeroEnumValue
cmdlet Get-DscProperty at command pipeline position 1
Supply values for the following parameters:
InputObject: d
Reviewable status: 4 of 5 files reviewed, all discussions resolved (waiting on @dan-hughes)
@johlju, I found the same thing. But I don't think I need to add both sets to all parameters. Unless it's a DSCCommunity convention? |
Last commit makes the syntax to
The first row has |
I think you want It's to keep the behavior the same. |
I would like to see the syntax like this
In your current commit, syntax 1 will never be used when HasValue is specified, so no need to have it there. |
You have the latest commit? That's what mine looks like now. |
No, that looks okay. Must have made something wrong. Let med double check. 🙂 |
I get what you mean now. Let me look at it for at sec. |
Double-checked in PS7 and WinPS and both gives
|
I see it now. I think I will need to define two parametersets on the other parameters for it to work. |
Yes in this case I think it is a must. 🙂 |
Should be as expected now. |
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.
Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @dan-hughes)
@dan-hughes great work! |
I will release this later today. |
Pull Request (PR) description
Add new optional parameter
IgnoreZeroEnumValue
toGet-DscProperty
.This Pull Request (PR) fixes the following issues
Get-DscProperty
: Add optional switch parameterIgnoreZeroEnumValue
#150Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
DSC Community Testing Guidelines.
This change is