chris-peterson / pwsh-gitlab

:computer: PowerShell module for GitLab
MIT License
22 stars 10 forks source link

Standardize boolean parameters #59

Open chris-peterson opened 1 year ago

chris-peterson commented 1 year ago

[bool] has some annoying behaviors, e.g. you can't pass bare strings true or false

Using this pattern makes consumption easier

        [Parameter()]
        [ValidateSet($null, 'true', 'false')]
        [object]
gaelcolas commented 1 year ago

I think that should be a nullable bool tbh.

[nullable[bool]]

You shouldn't pass strings to that...

chris-peterson commented 1 year ago

The [ValidateSet] + string approach is certainly not ideal.

But if you use [nullable[bool]], you can't provide a string literal, e.g.

<CmdLet> -BooleanProperty false

results in ⬇️

Cannot convert value "System.String" to type "System.Nullable`1[System.Boolean]".
Boolean parameters accept only Boolean values and numbers, such as $True, $False, 1 or 0.

Seems like an oversight in Powershell; if you're going to say integers are valid, why not (unambiguous) strings?

gaelcolas commented 1 year ago

But that's expected, because we want a [bool] (we just have to add a $). Why using 'false'? Is it purely for "shell" usage?

chris-peterson commented 1 year ago

indeed -- I like supporting string literals as often the cmdlets are invoked from CI jobs. in some cases, the fact that the underlying code is powershell is completely hidden