-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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
Prioritize validateset completions over Enums for parameters. #15257
Prioritize validateset completions over Enums for parameters. #15257
Conversation
What if implement this as filter over enum values? I mean get enum values and filter out them with values from ValidateSet? @mklement0 Are there such scenarios where we could use ValidateSet as follow filter after another source of values? |
I'm not sure we need more sophistication here, as it probably isn't that common a scenario. No other value sources where filtering based on explicitly enumerated constants - which is the prerequisite for taking effect for tab-completion - come to mind. The typo problem can be avoided if you use the enum type explicitly, but it's obviously more effort to type: # A typo here would be flagged in Visual Studio Code and would cause a *parse*-time error when invoked.
[ValidateSet([ConsoleColor]::DarkBlue, [ConsoleColor]::DarkCyan)] However, if you use strings, you won't see the problem until runtime, situationally, namely only if you happen to pass as value with a typo. Again, given the use case, I personally think it's fine to make this the command author's responsibility. On a related note, the ability to rule out specifying multiple values for a flag-based enumeration via a dedicated attribute would be handy - see @SeeminglyScience's suggestions here. |
What if we use ValidateScript in the scenario? |
ValidateScript doesn't provide any tab completion, so I'm not really sure how it's relevant here. |
My question is about using it as a filter. |
I mean yeah, you can use it as a filter for the enum values as they're passed into the function, but it doesn't have any real advantages over using ValidateSet. You also still can't use it to filter out autocompletion options, which seems to be the point of the PR here. |
What if a parameter gets a list of VMs from Cloud but want only stopped ones? |
@iSazonov, in that case you'd use a |
Then it is a binary attribute and you write a script? :-) |
It's usable in PowerShell code too; e.g.: function foo {
[cmdletbinding()]
param(
[ArgumentCompleter({ param($cmd, $param, $wordToComplete) 'Unix', 'Win32NT' -like "$wordToComplete*" })]
$Platform
)
[System.PlatformID] $Platform
} Incidentally, this highlights that the |
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 10 days of this comment. |
Why has this been marked as "Waiting on Author"? What do I need to do? |
src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Ilya <darpa@yandex.ru>
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
I would like to get @daxian-dbw to review this before merging. |
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
🎉 Handy links: |
PR Summary
Makes tab completion respect validateset values for parameters with an enum type.
PR Context
The completion for variables already work this way and Powershell does allow you to use ValidateSet to limit valid enums so this is just a fix for the parameter completion.
PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
or[ WIP ]
to the beginning of the title (theWIP
bot will keep its status check atPending
while the prefix is present) and remove the prefix when the PR is ready.(which runs in a different PS Host).