Skip to content

Commit d5a39b1

Browse files
jonsequiturJon Sequeira
andauthored
Fix #1726 - allow FromAmong to be case-insensitive (#2780)
* Fix #1726 * test improvements based on PR feedback --------- Co-authored-by: Jon Sequeira <josequ+microsoft@microsoft.com>
1 parent bf0aeb2 commit d5a39b1

File tree

5 files changed

+116
-3
lines changed

5 files changed

+116
-3
lines changed

src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
public static Argument<T> AcceptLegalFileNamesOnly<T>(this Argument<T> argument)
3636
public static Argument<T> AcceptLegalFilePathsOnly<T>(this Argument<T> argument)
3737
public static Argument<T> AcceptOnlyFromAmong<T>(this Argument<T> argument, System.String[] values)
38+
public static Argument<T> AcceptOnlyFromAmong<T>(this Argument<T> argument, System.StringComparer comparer, System.String[] values)
3839
public class Command : Symbol, System.Collections.IEnumerable
3940
.ctor(System.String name, System.String description = null)
4041
public System.CommandLine.Invocation.CommandLineAction Action { get; set; }
@@ -99,6 +100,7 @@
99100
public Option<T> AcceptLegalFileNamesOnly()
100101
public Option<T> AcceptLegalFilePathsOnly()
101102
public Option<T> AcceptOnlyFromAmong(System.String[] values)
103+
public Option<T> AcceptOnlyFromAmong(System.StringComparer comparer, System.String[] values)
102104
public static class OptionValidation
103105
public static Option<System.IO.FileInfo> AcceptExistingOnly(this Option<System.IO.FileInfo> option)
104106
public static Option<System.IO.DirectoryInfo> AcceptExistingOnly(this Option<System.IO.DirectoryInfo> option)

src/System.CommandLine.Tests/ArgumentTests.cs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,4 +107,61 @@ public void Argument_of_enum_can_limit_enum_members_as_valid_values()
107107
.Should()
108108
.BeEquivalentTo(new[] { $"Argument 'Fuschia' not recognized. Must be one of:\n\t'Red'\n\t'Green'" });
109109
}
110+
111+
[Fact]
112+
public void AcceptOnlyFromAmong_with_comparer_is_case_insensitive()
113+
{
114+
var argument = new Argument<string>("name");
115+
argument.AcceptOnlyFromAmong(StringComparer.OrdinalIgnoreCase, "NAME1", "NAME2");
116+
117+
Command command = new("run")
118+
{
119+
argument
120+
};
121+
122+
var result = command.Parse("run name1");
123+
124+
using var _ = new AssertionScope();
125+
126+
result.Errors.Should().BeEmpty();
127+
result.GetValue(argument).Should().Be("name1");
128+
}
129+
130+
[Fact]
131+
public void AcceptOnlyFromAmong_with_case_insensitive_comparer_rejects_values_outside_the_accepted_set()
132+
{
133+
var argument = new Argument<string>("name");
134+
argument.AcceptOnlyFromAmong(StringComparer.OrdinalIgnoreCase, "NAME1", "NAME2");
135+
136+
Command command = new("run")
137+
{
138+
argument
139+
};
140+
141+
var result = command.Parse("run NAME3");
142+
143+
result.Errors
144+
.Select(e => e.Message)
145+
.Should()
146+
.BeEquivalentTo(["Argument 'NAME3' not recognized. Must be one of:\n\t'NAME1'\n\t'NAME2'"]);
147+
}
148+
149+
[Fact]
150+
public void AcceptOnlyFromAmong_with_case_comparer_rejects_values_outside_the_accepted_case()
151+
{
152+
var argument = new Argument<string>("name");
153+
argument.AcceptOnlyFromAmong(StringComparer.Ordinal, "NAME1", "NAME2");
154+
155+
Command command = new("run")
156+
{
157+
argument
158+
};
159+
160+
var result = command.Parse("run name2");
161+
162+
result.Errors
163+
.Select(e => e.Message)
164+
.Should()
165+
.BeEquivalentTo(["Argument 'name2' not recognized. Must be one of:\n\t'NAME1'\n\t'NAME2'"]);
166+
}
110167
}

src/System.CommandLine.Tests/OptionTests.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,34 @@ public void Option_of_enum_can_limit_enum_members_as_valid_values()
408408
.BeEquivalentTo(new[] { $"Argument 'Fuschia' not recognized. Must be one of:\n\t'Red'\n\t'Green'" });
409409
}
410410

411+
[Fact]
412+
public void AcceptOnlyFromAmong_with_comparer_is_case_insensitive()
413+
{
414+
Option<string> option = new("--name");
415+
option.AcceptOnlyFromAmong(StringComparer.OrdinalIgnoreCase, "NAME1", "NAME2");
416+
417+
var result = new RootCommand { option }.Parse("--name name1");
418+
419+
using var _ = new AssertionScope();
420+
421+
result.Errors.Should().BeEmpty();
422+
result.GetValue(option).Should().Be("name1");
423+
}
424+
425+
[Fact]
426+
public void AcceptOnlyFromAmong_with_comparer_rejects_invalid_values()
427+
{
428+
Option<string> option = new("--name");
429+
option.AcceptOnlyFromAmong(StringComparer.OrdinalIgnoreCase, "NAME1", "NAME2");
430+
431+
var result = new RootCommand { option }.Parse("--name NAME3");
432+
433+
result.Errors
434+
.Select(e => e.Message)
435+
.Should()
436+
.BeEquivalentTo(new[] { $"Argument 'NAME3' not recognized. Must be one of:\n\t'NAME1'\n\t'NAME2'" });
437+
}
438+
411439
[Fact]
412440
public void Option_result_provides_identifier_token_if_name_was_provided()
413441
{

src/System.CommandLine/ArgumentValidation.cs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Collections.Generic;
55
using System.CommandLine.Parsing;
66
using System.IO;
7+
using System.Linq;
78

89
namespace System.CommandLine
910
{
@@ -131,7 +132,21 @@ public static Argument<T> AcceptOnlyFromAmong<T>(
131132
this Argument<T> argument,
132133
params string[] values)
133134
{
134-
if (values is not null && values.Length > 0)
135+
return AcceptOnlyFromAmong(argument, StringComparer.Ordinal, values);
136+
}
137+
138+
/// <summary>
139+
/// Configures the argument to accept only the specified values using the specified comparer, and to suggest them as command line completions.
140+
/// </summary>
141+
/// <param name="argument">The argument to configure.</param>
142+
/// <param name="comparer">The comparer used to match argument values against the allowed values.</param>
143+
/// <param name="values">The values that are allowed for the argument.</param>
144+
public static Argument<T> AcceptOnlyFromAmong<T>(
145+
this Argument<T> argument,
146+
StringComparer comparer,
147+
params string[] values)
148+
{
149+
if (values?.Length > 0)
135150
{
136151
argument.Validators.Clear();
137152
argument.Validators.Add(UnrecognizedArgumentError);
@@ -140,7 +155,7 @@ public static Argument<T> AcceptOnlyFromAmong<T>(
140155
}
141156

142157
return argument;
143-
158+
144159
void UnrecognizedArgumentError(ArgumentResult argumentResult)
145160
{
146161
for (var i = 0; i < argumentResult.Tokens.Count; i++)
@@ -149,7 +164,7 @@ void UnrecognizedArgumentError(ArgumentResult argumentResult)
149164

150165
if (token.Symbol is null || token.Symbol == argument)
151166
{
152-
if (Array.IndexOf(values, token.Value) < 0)
167+
if (!values.Contains(token.Value, comparer))
153168
{
154169
argumentResult.AddError(LocalizationResources.UnrecognizedArgument(token.Value, values));
155170
}

src/System.CommandLine/Option{T}.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,17 @@ public Option<T> AcceptOnlyFromAmong(params string[] values)
5757
return this;
5858
}
5959

60+
/// <summary>
61+
/// Configures the option to accept only the specified values using the specified comparer, and to suggest them as command line completions.
62+
/// </summary>
63+
/// <param name="comparer">The comparer used to match argument values against the allowed values.</param>
64+
/// <param name="values">The values that are allowed for the option.</param>
65+
public Option<T> AcceptOnlyFromAmong(StringComparer comparer, params string[] values)
66+
{
67+
_argument.AcceptOnlyFromAmong(comparer, values);
68+
return this;
69+
}
70+
6071
/// <summary>
6172
/// Configures the option to accept only values representing legal file paths.
6273
/// </summary>

0 commit comments

Comments
 (0)