-
Notifications
You must be signed in to change notification settings - Fork 416
Remove Argument.AllowedValues, use Validator to implement the logic #1959
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
Changes from 3 commits
2411731
29940bb
d2b0206
36b179c
f517c35
1cb1e9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,8 +39,6 @@ protected Argument(string? name = null, string? description = null) | |
| Description = description; | ||
| } | ||
|
|
||
| internal HashSet<string>? AllowedValues { get; private set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the arity of the argument. | ||
| /// </summary> | ||
|
|
@@ -105,14 +103,15 @@ private protected override string DefaultName | |
| } | ||
| } | ||
|
|
||
| internal List<Action<ArgumentResult>> Validators => _validators ??= new (); | ||
| internal IReadOnlyList<Action<ArgumentResult>> Validators | ||
| => _validators is null ? Array.Empty<Action<ArgumentResult>>() : _validators; | ||
|
|
||
| /// <summary> | ||
| /// Adds a custom validator to the argument. Validators can be used | ||
| /// to provide custom errors based on user input. | ||
| /// </summary> | ||
| /// <param name="validate">The action to validate the parsed argument.</param> | ||
| public void AddValidator(Action<ArgumentResult> validate) => Validators.Add(validate); | ||
| public void AddValidator(Action<ArgumentResult> validate) => (_validators ??= new()).Add(validate); | ||
|
||
|
|
||
| /// <summary> | ||
| /// Gets the default value for the argument. | ||
|
|
@@ -179,16 +178,6 @@ public void SetDefaultValueFactory(Func<ArgumentResult, object?> defaultValueFac | |
| Arity = ArgumentArity.Zero | ||
| }; | ||
|
|
||
| internal void AddAllowedValues(IReadOnlyList<string> values) | ||
| { | ||
| if (AllowedValues is null) | ||
| { | ||
| AllowedValues = new HashSet<string>(); | ||
| } | ||
|
|
||
| AllowedValues.UnionWith(values); | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public override IEnumerable<CompletionItem> GetCompletions(CompletionContext context) | ||
| { | ||
|
|
@@ -244,12 +233,30 @@ public Argument AddCompletions(Func<CompletionContext, IEnumerable<CompletionIte | |
| /// <returns>The configured argument.</returns> | ||
| public Argument AcceptOnlyFromAmong(params string[] values) | ||
| { | ||
| AllowedValues?.Clear(); | ||
| AddAllowedValues(values); | ||
| Completions.Clear(); | ||
| Completions.Add(values); | ||
| if (values is not null && values.Length > 0) | ||
| { | ||
| AddValidator(UnrecognizedArgumentError); | ||
| Completions.Clear(); | ||
| Completions.Add(values); | ||
| } | ||
|
|
||
| return this; | ||
|
|
||
| void UnrecognizedArgumentError(ArgumentResult argumentResult) | ||
| { | ||
| for (var i = 0; i < argumentResult.Tokens.Count; i++) | ||
| { | ||
| var token = argumentResult.Tokens[i]; | ||
|
|
||
| if (token.Symbol is null || token.Symbol == this) | ||
| { | ||
| if (!values.Contains(token.Value)) | ||
| { | ||
| argumentResult.ErrorMessage = argumentResult.LocalizationResources.UnrecognizedArgument(token.Value, values); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -83,7 +83,7 @@ public void OnlyTake(int numberOfTokens) | |||||
|
|
||||||
| if (!string.IsNullOrWhiteSpace(ErrorMessage)) | ||||||
| { | ||||||
| return new ParseError(ErrorMessage!, this); | ||||||
| return new ParseError(ErrorMessage!, Parent is OptionResult option ? option : this); | ||||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change is not obvious. Before this change, the
but this was true only "non-custom errors" (user defined, coming from validators):
Without this change, one test was failing: Since we hide the |
||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are other places that call Validator.Add, which would now add the Action to the Array.Empty<Action>.
command-line-api/src/System.CommandLine/Argument{T}.cs
Line 215 in 350a618
command-line-api/src/System.CommandLine/OptionValidation.cs
Line 21 in 350a618