Skip to content
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

Parameter Naming Inconsistancy: Option vs. Options #10551

Open
tommymaynard opened this issue Sep 16, 2019 · 12 comments

Comments

@tommymaynard
Copy link
Contributor

commented Sep 16, 2019

PS C:\> New-Variable -Name x -Value 'Read-only variable' -Option ReadOnly
PS C:\> New-Item -Path Function:\xf -Value {'Does nothing'} -Options ReadOnly | Out-Null

Expected behavior

The expected behavior is that both parameters would have been named "Option" (singular) in both cmdlets, regardless of the PS Provider, and whether or not a parameter is dynamic. Were this type of discrepancy found in more places in Microsoft delivered code, even as small as this is, it may have had a largely negative impact on PowerShell overall. I do understand that, "The data stores that the provider exposes can be as varied as Active Directory location and Microsoft Exchange Server mailboxes." Even so, this seems like a small, non-breaking change. That said, this change would require edits to About Providers, and Function provider.

Actual behavior

"Option" and "Options" are both used for the same parameter on different cmdlets (different PS Providers). Each parameter includes the identical arguments, or parameter values: AllScope, Constant, None, Private, ReadOnly, Unspecified.

Environment data

Name                           Value
----                           -----
PSVersion                      6.2.3
PSEdition                      Core
GitCommitId                    6.2.3
OS                             Microsoft Windows 10.0.18362 
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0
@iSazonov

This comment has been minimized.

Copy link
Collaborator

commented Sep 17, 2019

Change from long name to short (Options -> Option) is breaking change.

@vexx32

This comment has been minimized.

Copy link
Collaborator

commented Sep 17, 2019

I can't think of why it needs to be. We can leave an alias on the affected parameter so that it will still work with legacy code.

@tommymaynard

This comment has been minimized.

Copy link
Contributor Author

commented Sep 17, 2019

I agree with you, @iSazonov, in that it would be a breaking change -- I didn't think it all the way through. That said, after your comment, but prior to @vexx32's, I also thought that adding a parameter alias Options, to an Option parameter would keep things working for currently written automation that uses the Options parameter.

@iSazonov

This comment has been minimized.

Copy link
Collaborator

commented Sep 17, 2019

Alias doesn't cover all scenarios. :-(

@vexx32

This comment has been minimized.

Copy link
Collaborator

commented Sep 17, 2019

What scenarios wouldn't bind correctly with use of an alias?

@iSazonov

This comment has been minimized.

Copy link
Collaborator

commented Sep 17, 2019

@vexx32 Not binding, I updated my comment. See #3307

@vexx32

This comment has been minimized.

Copy link
Collaborator

commented Sep 17, 2019

@iSazonov I don't think that scenario applies here.

  1. Objects aren't being input to the parameter;
  2. The casting behaviour of the input object(s) is the same (the parameter values are strings / enum);
  3. This is not a pipeline parameter.

Am I missing something?

@iSazonov

This comment has been minimized.

Copy link
Collaborator

commented Sep 18, 2019

@vexx32 Any of this may change in the future.

@vexx32

This comment has been minimized.

Copy link
Collaborator

commented Sep 18, 2019

Changing any of that would be a bigger breaking change. Nobody's calling for this cmdlet's design to change. Just a parameter name... and we'd leave an alias to the old name for backwards compatibility. This was done with the Out-File cmdlet (-Path on that cmdlet is -FilePath on Windows PowerShell) and there have been no issues with that since that I'm aware of.

Am I missing something? I'm not seeing any real point of contention here, I'm not understanding why you think changing this is a net negative.

@iSazonov

This comment has been minimized.

Copy link
Collaborator

commented Sep 18, 2019

@vexx32 For discussion we need to collect all arguments pro and cons. :-)

Also in general for providers (that is plugins) we can not control provider parameter names.

@vexx32

This comment has been minimized.

Copy link
Collaborator

commented Sep 18, 2019

Fair enough!

Provider or no, I think it ought to be fixed either way, really. It just changes where in the code the fix needs to be, no? 🙂

@iSazonov

This comment has been minimized.

Copy link
Collaborator

commented Sep 18, 2019

the code the fix needs to be, no?

I'm not sure yet. I did not find a description in the documentation. This is also a problem. And I did not find this code. Therefore, it is not yet clear what intentions were under the design. Provider code always very sensitive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.