Skip to content

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

Merged
merged 8 commits into from
May 2, 2025

Conversation

dan-hughes
Copy link
Contributor

@dan-hughes dan-hughes commented May 1, 2025

Pull Request (PR) description

Add new optional parameter IgnoreZeroEnumValue to Get-DscProperty.

This Pull Request (PR) fixes the following issues

Task list

  • Added an entry to the change log under the Unreleased section of the
    file CHANGELOG.md. Entry should say what was changed and how that
    affects users (if applicable), and reference the issue being resolved
    (if applicable).
  • Documentation added/updated in README.md.
  • Comment-based help added/updated for all new/changed functions.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Community Testing Guidelines.
  • Integration tests added/updated (where possible). See
    DSC Community Testing Guidelines.
  • New/changed code adheres to DSC Community Style Guidelines.

This change is Reviewable

@dan-hughes
Copy link
Contributor Author

@johlju, I think IgnoreZeroEnumValue requires HasValue to also be set.

@dan-hughes dan-hughes changed the title Get-DscProperty: Add optional switch parameter IgnoreZeroEnumValue Get-DscProperty: Add optional switch parameter IgnoreZeroEnumValue May 1, 2025
Copy link
Member

@johlju johlju left a 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: :shipit: complete! all files reviewed, all discussions resolved (waiting on @dan-hughes)

@dan-hughes
Copy link
Contributor Author

I added a ParameterSet to solve this.

Copy link
Member

@johlju johlju left a 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)

@dan-hughes dan-hughes marked this pull request as ready for review May 2, 2025 12:00
@dan-hughes
Copy link
Contributor Author

@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?

@johlju
Copy link
Member

johlju commented May 2, 2025

Last commit makes the syntax to

    Get-DscProperty -InputObject <PSObject> [-Name <String[]>] [-ExcludeName <String[]>] [-Attribute <String[]>] [-HasValue] [<CommonParameters>]
    
    Get-DscProperty -InputObject <PSObject> [-Name <String[]>] [-ExcludeName <String[]>] [-Attribute <String[]>] -HasValue [-IgnoreZeroEnumValue] [<CommonParameters>]

The first row has HasValue too which seems wrong.

@dan-hughes
Copy link
Contributor Author

dan-hughes commented May 2, 2025

Last commit makes the syntax to

    Get-DscProperty -InputObject <PSObject> [-Name <String[]>] [-ExcludeName <String[]>] [-Attribute <String[]>] [-HasValue] [<CommonParameters>]
    
    Get-DscProperty -InputObject <PSObject> [-Name <String[]>] [-ExcludeName <String[]>] [-Attribute <String[]>] -HasValue [-IgnoreZeroEnumValue] [<CommonParameters>]

The first row has HasValue too which seems wrong.

I think you want HasValue (optional) without IgnoreZeroEnum (Set1), but with IgnoreZeroEnum, HasValue is mandatory, otherwise the Enum may not have a value.

It's to keep the behavior the same.

@johlju
Copy link
Member

johlju commented May 2, 2025

I think you want HasValue without IgnoreZeroEnum (Set1), but with IgnoreZeroEnum, HasValue is mandatory, otherwise the Enum may not have a value.

I would like to see the syntax like this

Get-DscProperty -InputObject <PSObject> [-Name <String[]>] [-ExcludeName <String[]>] [-Attribute <String[]>] [<CommonParameters>]
    
Get-DscProperty -InputObject <PSObject> [-Name <String[]>] [-ExcludeName <String[]>] [-Attribute <String[]>] -HasValue [-IgnoreZeroEnumValue] [<CommonParameters>]

IgnoreZeroEnumValue should never be able to be choosen unless HasValue has been specified. So the second syntax will be used is HasValue is specified, that gives the option to add the optional parameter IgnoreZeroEnumValue as well.

In your current commit, syntax 1 will never be used when HasValue is specified, so no need to have it there.

@dan-hughes
Copy link
Contributor Author

I think you want HasValue without IgnoreZeroEnum (Set1), but with IgnoreZeroEnum, HasValue is mandatory, otherwise the Enum may not have a value.

I would like to see the syntax like this

Get-DscProperty -InputObject <PSObject> [-Name <String[]>] [-ExcludeName <String[]>] [-Attribute <String[]>] [<CommonParameters>]
    
Get-DscProperty -InputObject <PSObject> [-Name <String[]>] [-ExcludeName <String[]>] [-Attribute <String[]>] -HasValue [-IgnoreZeroEnumValue] [<CommonParameters>]

IgnoreZeroEnumValue should never be able to be choosen unless HasValue has been specified. So the second syntax will be used is HasValue is specified, that gives the option to add the optional parameter IgnoreZeroEnumValue as well.

You have the latest commit? That's what mine looks like now.

@johlju
Copy link
Member

johlju commented May 2, 2025

No, that looks okay. Must have made something wrong. Let med double check. 🙂

@dan-hughes
Copy link
Contributor Author

dan-hughes commented May 2, 2025

I get what you mean now.

Let me look at it for at sec.

@johlju
Copy link
Member

johlju commented May 2, 2025

Double-checked in PS7 and WinPS and both gives HasValue in syntax 1

Get-DscProperty -InputObject <PSObject> [-Name <String[]>] [-ExcludeName <String[]>] [-Attribute <String[]>] [-HasValue] [<CommonParameters>]

@dan-hughes
Copy link
Contributor Author

Double-checked in PS7 and WinPS and both gives HasValue in syntax 1

Get-DscProperty -InputObject <PSObject> [-Name <String[]>] [-ExcludeName <String[]>] [-Attribute <String[]>] [-HasValue] [<CommonParameters>]

I see it now.

I think I will need to define two parametersets on the other parameters for it to work.

@johlju
Copy link
Member

johlju commented May 2, 2025

Yes in this case I think it is a must. 🙂

@dan-hughes
Copy link
Contributor Author

Should be as expected now.

Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @dan-hughes)

@johlju johlju merged commit 138446c into dsccommunity:main May 2, 2025
9 checks passed
@johlju
Copy link
Member

johlju commented May 2, 2025

@dan-hughes great work!

@johlju
Copy link
Member

johlju commented May 2, 2025

I will release this later today.

@dan-hughes dan-hughes deleted the add-clear-zeroedenums branch May 2, 2025 12:50
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.

Get-DscProperty: Add optional switch parameter IgnoreZeroEnumValue
2 participants