Powershell: The switch parameters for Split-Path should be mandatory

Created on 18 Jun 2020  Â·  14Comments  Â·  Source: PowerShell/PowerShell

This is related to the recent change made in PR #12960

I was reviewing this PR to understand the change needed in the documentation. I also looked at the source code and find that the pattern used by the parameters is confusing. The switch parameters are all optional but are used to define the unique ParameterSet. So, they are mandatory for that parameter set.

PS C:\temp\test> syntax Split-Path | ft -w

Cmdlet     ParameterSetName Parameters
------     ---------------- ----------
Split-Path ParentSet        [-Path] <string[]> [-Parent] [-Resolve] [-Credential <pscredential>] [<CommonParameters>]
Split-Path LeafSet          [-Path] <string[]> [-Leaf] [-Resolve] [-Credential <pscredential>] [<CommonParameters>]
Split-Path LeafBaseSet      [-Path] <string[]> [-LeafBase] [-Resolve] [-Credential <pscredential>] [<CommonParameters>]
Split-Path ExtensionSet     [-Path] <string[]> [-Extension] [-Resolve] [-Credential <pscredential>] [<CommonParameters>]
Split-Path QualifierSet     [-Path] <string[]> [-Qualifier] [-Resolve] [-Credential <pscredential>] [<CommonParameters>]
Split-Path NoQualifierSet   [-Path] <string[]> [-NoQualifier] [-Resolve] [-Credential <pscredential>] [<CommonParameters>]
Split-Path IsAbsoluteSet    [-Path] <string[]> [-Resolve] [-IsAbsolute] [-Credential <pscredential>] [<CommonParameters>]
Split-Path LiteralPathSet   -LiteralPath <string[]> [-Resolve] [-Credential <pscredential>] [<CommonParameters>]

I think the parameters should be changed to Mandatory = true. Are there any risks created by that change?

As a secondary question, what is the use-case scenario for allowing ValueFromPipelineByPropertyName on those switch parameters? Seems like a strange use case and hard to document.

Area-Cmdlets-Management First-Time-Issue Issue-Enhancement Resolution-Fixed Up-for-Grabs

Most helpful comment

cool, so I will create a PR. Will reserve this for a small session with the UG here to show how simple some changes are, coz people are often afraid to contribute thinking its always complex changes we do here :-).

All 14 comments

At least one of the switches will need to be optional as it is the default mode -- the -Parent switch.

The rest are _probably_ fine being made mandatory for that set.

Also, did you mean to reference a different PR? I'm not seeing anything that jumps out to me about Split-Path in that PR.

@vexx32 Thanks for spotting the PR ref. I fixed the copy/paste error.

I do not think there are any risks—but I think that the change you request is inconsistent with the concept. The command operates in several modes and one mode is the default, so the switch to that mode is optional. However, the decision which mode is the default is arbitrary, and while it would be an unwanted breaking change to change it, it is only caused by the inertia of the code base and not by any inherent special property of the default mode that makes it the default. After this change, any attempt to change the default mode (e.g. in a fork) would involve tweaking the mandatory statuses. I can see no value in doing this.

  • What is wrong with writing that all parameters are optional?

I think that the general rule that parameter set leaders should be mandatory is misleading to the users. Since I, as a user, have a free choice of which parameter set to use, I perceive the corresponding leading switch as optional. The fact that it is necessary for the parameter set to be used does not make it mandatory.
Compare the usual privacy statement: you are not compelled to give us your data, but unless you do, you will not be able to use our services. It is the same pattern.

We're not suggesting changing the default.

Marking the parameter mandatory simply makes the syntax diagrams shown in Get-Help much clearer, without actually changing anything.

Not really sure why you're so vehemently against a purely helpful UX change.

The fact that it is necessary for the parameter set to be used does not make it mandatory.

Yeah, actually, it does. "Necessary" and "mandatory" are synonyms. It's the same word. It's fundamentally what it means.

Yeah, actually, it does. "Necessary" and "mandatory" are synonyms. It's the same word. It's fundamentally what it means.

OK, one last try and I give up. Drinking water is necessary because otherwise you die. It is not mandatory because nobody actually tells you to do it if you are a healthy adult. It is mandatory only before an ultrasound.

That's basically pedantry level of difference and not even remotely relevant in this application of the words.

Right. In this application, a parameter that is mandatory suggests that users have to provide its value. Which is not true in this case.

It's mandatory (has to be supplied) in order for that parameter set to be chosen. We can argue semantics into next week, but the fact remains that you can't use the associated parameter sets of Join-Path without specifying the switch mode for it. Ergo, they're mandatory.

Users do not use parameter sets (conceptually). Users use parameters to achieve their goals. Parameter sets are an internal technicality mainly for cmdlet developers. Parameter sets are an important development that makes the interface much easier to use—but from users’ PoV it works as expected by magic.
Also consider how NEW-ITEM -NAME is declared mandatory, which is as wrong as can be.

@vexx32 is it about making all switches mandatory in respective parameters sets except for the ParentSet ?

yeah pretty much.

@yecril71pl's concern is, to me, a non-concern. The syntax diagrams are currently far more confusing than they need to be, it's not clear which set is actually which (they all look _really_ similar). Marking the mode switches as mandatory in their set will just make the syntax diagrams clearer without actually changing the underlying behaviour.

cool, so I will create a PR. Will reserve this for a small session with the UG here to show how simple some changes are, coz people are often afraid to contribute thinking its always complex changes we do here :-).

If y'all run into issues at all just give me a shout, happy to help if needed! 💖

yeah pretty much.

@yecril71pl's concern is, to me, a non-concern. The syntax diagrams are currently far more confusing than they need to be, it's not clear which set is actually which (they all look _really_ similar). Marking the mode switches as mandatory in their set will just make the syntax diagrams clearer without actually changing the underlying behaviour.

I agree that the following meta-syntax is awkward and confusing:

Run-Command [-ChooseParamSet1] [-OtherParams]
Run-Command [-ChooseParamSet2] [-OtherParams]
…

and that the meta-syntax that shows parameter set triggers as mandatory

Run-Command -ChooseParamSet1 [-OtherParams]
Run-Command -ChooseParamSet2 [-OtherParams]
…

would be more correct and appealing.

However, there may be several trigger parameters for a parameter set. For example, in Pester, it would be fully reasonable if Should -ErrorId:… implied -Throw. That would make both parameters Throw and ErrorId optional, so your solution to this problem is not only logically incorrect but also partial in practice. It would be better to explicitly mark parameter set triggers rather than make them all mandatory, which, as I have said, you cannot always do.

IMVHO.

Was this page helpful?
0 / 5 - 0 ratings