Skip to content

Commit b7102d8

Browse files
rmunnrmunn
committed
Address review comments
* Share common code between Tokenizer and GetoptTokenizer * Use enum for ParserMode as we might add more in the future
1 parent 47618e0 commit b7102d8

File tree

6 files changed

+83
-76
lines changed

6 files changed

+83
-76
lines changed

‎src/CommandLine/Core/GetoptTokenizer.cs

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -88,36 +88,6 @@ public static Result<IEnumerable<Token>, Error> Tokenize(
8888
return Result.Succeed<IEnumerable<Token>, Error>(tokens.AsEnumerable(), errors.AsEnumerable());
8989
}
9090

91-
public static Result<IEnumerable<Token>, Error> ExplodeOptionList(
92-
Result<IEnumerable<Token>, Error> tokenizerResult,
93-
Func<string, Maybe<char>> optionSequenceWithSeparatorLookup)
94-
{
95-
var tokens = tokenizerResult.SucceededWith().Memoize();
96-
97-
var exploded = new List<Token>(tokens is ICollection<Token> coll ? coll.Count : tokens.Count());
98-
var nothing = Maybe.Nothing<char>(); // Re-use same Nothing instance for efficiency
99-
var separator = nothing;
100-
foreach (var token in tokens) {
101-
if (token.IsName()) {
102-
separator = optionSequenceWithSeparatorLookup(token.Text);
103-
exploded.Add(token);
104-
} else {
105-
// Forced values are never considered option values, so they should not be split
106-
if (separator.MatchJust(out char sep) && sep != '\0' && !token.IsValueForced()) {
107-
if (token.Text.Contains(sep)) {
108-
exploded.AddRange(token.Text.Split(sep).Select(Token.ValueFromSeparator));
109-
} else {
110-
exploded.Add(token);
111-
}
112-
} else {
113-
exploded.Add(token);
114-
}
115-
separator = nothing; // Only first value after a separator can possibly be split
116-
}
117-
}
118-
return Result.Succeed(exploded as IEnumerable<Token>, tokenizerResult.SuccessMessages());
119-
}
120-
12191
public static Func<
12292
IEnumerable<string>,
12393
IEnumerable<OptionSpecification>,
@@ -131,7 +101,7 @@ public static Func<
131101
return (arguments, optionSpecs) =>
132102
{
133103
var tokens = GetoptTokenizer.Tokenize(arguments, name => NameLookup.Contains(name, optionSpecs, nameComparer), ignoreUnknownArguments, enableDashDash, posixlyCorrect);
134-
var explodedTokens = GetoptTokenizer.ExplodeOptionList(tokens, name => NameLookup.HavingSeparator(name, optionSpecs, nameComparer));
104+
var explodedTokens = Tokenizer.ExplodeOptionList(tokens, name => NameLookup.HavingSeparator(name, optionSpecs, nameComparer));
135105
return explodedTokens;
136106
};
137107
}

‎src/CommandLine/Parser.cs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -185,16 +185,28 @@ private static Result<IEnumerable<Token>, Error> Tokenize(
185185
IEnumerable<OptionSpecification> optionSpecs,
186186
ParserSettings settings)
187187
{
188-
return settings.GetoptMode
189-
? GetoptTokenizer.ConfigureTokenizer(
188+
switch (settings.ParserMode)
189+
{
190+
case ParserMode.Legacy:
191+
return Tokenizer.ConfigureTokenizer(
192+
settings.NameComparer,
193+
settings.IgnoreUnknownArguments,
194+
settings.EnableDashDash)(arguments, optionSpecs);
195+
196+
case ParserMode.Getopt:
197+
return GetoptTokenizer.ConfigureTokenizer(
190198
settings.NameComparer,
191199
settings.IgnoreUnknownArguments,
192200
settings.EnableDashDash,
193-
settings.PosixlyCorrect)(arguments, optionSpecs)
194-
: Tokenizer.ConfigureTokenizer(
201+
settings.PosixlyCorrect)(arguments, optionSpecs);
202+
203+
// No need to test ParserMode.Default, as it should always be one of the above modes
204+
default:
205+
return Tokenizer.ConfigureTokenizer(
195206
settings.NameComparer,
196207
settings.IgnoreUnknownArguments,
197208
settings.EnableDashDash)(arguments, optionSpecs);
209+
}
198210
}
199211

200212
private static ParserResult<T> MakeParserResult<T>(ParserResult<T> parserResult, ParserSettings settings)

‎src/CommandLine/ParserSettings.cs

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,14 @@
99

1010
namespace CommandLine
1111
{
12+
public enum ParserMode
13+
{
14+
Legacy,
15+
Getopt,
16+
17+
Default = Legacy
18+
}
19+
1220
/// <summary>
1321
/// Provides settings for <see cref="CommandLine.Parser"/>. Once consumed cannot be reused.
1422
/// </summary>
@@ -27,7 +35,7 @@ public class ParserSettings : IDisposable
2735
private Maybe<bool> enableDashDash;
2836
private int maximumDisplayWidth;
2937
private Maybe<bool> allowMultiInstance;
30-
private bool getoptMode;
38+
private ParserMode parserMode;
3139
private Maybe<bool> posixlyCorrect;
3240

3341
/// <summary>
@@ -41,7 +49,7 @@ public ParserSettings()
4149
autoVersion = true;
4250
parsingCulture = CultureInfo.InvariantCulture;
4351
maximumDisplayWidth = GetWindowWidth();
44-
getoptMode = false;
52+
parserMode = ParserMode.Default;
4553
enableDashDash = Maybe.Nothing<bool>();
4654
allowMultiInstance = Maybe.Nothing<bool>();
4755
posixlyCorrect = Maybe.Nothing<bool>();
@@ -166,11 +174,11 @@ public bool AutoVersion
166174
/// <summary>
167175
/// Gets or sets a value indicating whether enable double dash '--' syntax,
168176
/// that forces parsing of all subsequent tokens as values.
169-
/// If GetoptMode is true, this defaults to true, but can be turned off by explicitly specifying EnableDashDash = false.
177+
/// Normally defaults to false. If ParserMode = ParserMode.Getopt, this defaults to true, but can be turned off by explicitly specifying EnableDashDash = false.
170178
/// </summary>
171179
public bool EnableDashDash
172180
{
173-
get => enableDashDash.MatchJust(out bool value) ? value : getoptMode;
181+
get => enableDashDash.MatchJust(out bool value) ? value : (parserMode == ParserMode.Getopt);
174182
set => PopsicleSetter.Set(Consumed, ref enableDashDash, Maybe.Just(value));
175183
}
176184

@@ -185,21 +193,38 @@ public int MaximumDisplayWidth
185193

186194
/// <summary>
187195
/// Gets or sets a value indicating whether options are allowed to be specified multiple times.
188-
/// If GetoptMode is true, this defaults to true, but can be turned off by explicitly specifying AllowMultiInstance = false.
196+
/// If ParserMode = ParserMode.Getopt, this defaults to true, but can be turned off by explicitly specifying AllowMultiInstance = false.
189197
/// </summary>
190198
public bool AllowMultiInstance
191199
{
192-
get => allowMultiInstance.MatchJust(out bool value) ? value : getoptMode;
200+
get => allowMultiInstance.MatchJust(out bool value) ? value : (parserMode == ParserMode.Getopt);
193201
set => PopsicleSetter.Set(Consumed, ref allowMultiInstance, Maybe.Just(value));
194202
}
195203

196204
/// <summary>
197-
/// Whether strict getopt-like processing is applied to option values; if true, AllowMultiInstance and EnableDashDash will default to true as well.
205+
/// Set this to change how the parser processes command-line arguments. Currently valid values are:
206+
/// <list>
207+
/// <item>
208+
/// <term>Legacy</term>
209+
/// <description>Uses - for short options and -- for long options.
210+
/// Values of long options can only start with a - character if the = syntax is used.
211+
/// E.g., "--string-option -x" will consider "-x" to be an option, not the value of "--string-option",
212+
/// but "--string-option=-x" will consider "-x" to be the value of "--string-option".</description>
213+
/// </item>
214+
/// <item>
215+
/// <term>Getopt</term>
216+
/// <description>Strict getopt-like processing is applied to option values.
217+
/// Mostly like legacy mode, except that option values with = and with space are more consistent.
218+
/// After an option that takes a value, and whose value was not specified with "=", the next argument will be considered the value even if it starts with "-".
219+
/// E.g., both "--string-option=-x" and "--string-option -x" will consider "-x" to be the value of "--string-option".
220+
/// If this mode is chosen, AllowMultiInstance and EnableDashDash will default to true as well, though they can be explicitly turned off if desired.</description>
221+
/// </item>
222+
/// </list>
198223
/// </summary>
199-
public bool GetoptMode
224+
public ParserMode ParserMode
200225
{
201-
get => getoptMode;
202-
set => PopsicleSetter.Set(Consumed, ref getoptMode, value);
226+
get => parserMode;
227+
set => PopsicleSetter.Set(Consumed, ref parserMode, value);
203228
}
204229

205230
/// <summary>

‎tests/CommandLine.Tests/Unit/Core/GetoptTokenizerTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public void Explode_scalar_with_separator_in_odd_args_input_returns_sequence()
2424

2525
// Exercize system
2626
var result =
27-
GetoptTokenizer.ExplodeOptionList(
27+
Tokenizer.ExplodeOptionList(
2828
Result.Succeed(
2929
Enumerable.Empty<Token>().Concat(new[] { Token.Name("i"), Token.Value("10"),
3030
Token.Name("string-seq"), Token.Value("aaa,bb,cccc"), Token.Name("switch") }),
@@ -47,7 +47,7 @@ public void Explode_scalar_with_separator_in_even_args_input_returns_sequence()
4747

4848
// Exercize system
4949
var result =
50-
GetoptTokenizer.ExplodeOptionList(
50+
Tokenizer.ExplodeOptionList(
5151
Result.Succeed(
5252
Enumerable.Empty<Token>().Concat(new[] { Token.Name("x"),
5353
Token.Name("string-seq"), Token.Value("aaa,bb,cccc"), Token.Name("switch") }),
@@ -90,7 +90,7 @@ public void Should_return_error_if_option_format_with_equals_is_not_correct()
9090
var errors = result.SuccessMessages();
9191

9292
Assert.NotNull(errors);
93-
Assert.Equal(1, errors.Count());
93+
Assert.NotEmpty(errors);
9494
Assert.Equal(ErrorType.BadFormatTokenError, errors.First().Tag);
9595

9696
var tokens = result.SucceededWith();

‎tests/CommandLine.Tests/Unit/GetoptParserTests.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ public void Getopt_parser_without_posixly_correct_allows_mixed_options_and_nonop
155155
{
156156
// Arrange
157157
var sut = new Parser(config => {
158-
config.GetoptMode = true;
158+
config.ParserMode = ParserMode.Getopt;
159159
config.PosixlyCorrect = false;
160160
});
161161

@@ -215,7 +215,7 @@ public void Getopt_parser_with_posixly_correct_stops_parsing_at_first_nonoption(
215215
{
216216
// Arrange
217217
var sut = new Parser(config => {
218-
config.GetoptMode = true;
218+
config.ParserMode = ParserMode.Getopt;
219219
config.PosixlyCorrect = true;
220220
config.EnableDashDash = true;
221221
});
@@ -233,7 +233,7 @@ public void Getopt_mode_defaults_to_EnableDashDash_being_true()
233233
{
234234
// Arrange
235235
var sut = new Parser(config => {
236-
config.GetoptMode = true;
236+
config.ParserMode = ParserMode.Getopt;
237237
config.PosixlyCorrect = false;
238238
});
239239
var args = new string [] {"--stringvalue", "foo", "256", "--", "-x", "-sbar" };
@@ -259,7 +259,7 @@ public void Getopt_mode_can_have_EnableDashDash_expicitly_disabled()
259259
{
260260
// Arrange
261261
var sut = new Parser(config => {
262-
config.GetoptMode = true;
262+
config.ParserMode = ParserMode.Getopt;
263263
config.PosixlyCorrect = false;
264264
config.EnableDashDash = false;
265265
});

‎tests/CommandLine.Tests/Unit/SequenceParsingTests.cs

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,31 +17,31 @@ public class SequenceParsingTests
1717
{
1818
// Issue #91
1919
[Theory]
20-
[InlineData(false)]
21-
[InlineData(true)]
22-
public static void Enumerable_with_separator_before_values_does_not_try_to_parse_too_much(bool useGetoptMode)
20+
[InlineData(ParserMode.Legacy)]
21+
[InlineData(ParserMode.Getopt)]
22+
public static void Enumerable_with_separator_before_values_does_not_try_to_parse_too_much(ParserMode parserMode)
2323
{
2424
var args = "--exclude=a,b InputFile.txt".Split();
2525
var expected = new Options_For_Issue_91 {
2626
Excluded = new[] { "a", "b" },
2727
Included = Enumerable.Empty<string>(),
2828
InputFileName = "InputFile.txt",
2929
};
30-
var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; });
30+
var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; });
3131
var result = sut.ParseArguments<Options_For_Issue_91>(args);
3232
result.Should().BeOfType<Parsed<Options_For_Issue_91>>();
3333
result.As<Parsed<Options_For_Issue_91>>().Value.Should().BeEquivalentTo(expected);
3434
}
3535

3636
// Issue #396
3737
[Theory]
38-
[InlineData(false)]
39-
[InlineData(true)]
40-
public static void Options_with_similar_names_are_not_ambiguous(bool useGetoptMode)
38+
[InlineData(ParserMode.Legacy)]
39+
[InlineData(ParserMode.Getopt)]
40+
public static void Options_with_similar_names_are_not_ambiguous(ParserMode parserMode)
4141
{
4242
var args = new[] { "--configure-profile", "deploy", "--profile", "local" };
4343
var expected = new Options_With_Similar_Names { ConfigureProfile = "deploy", Profile = "local", Deploys = Enumerable.Empty<string>() };
44-
var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; });
44+
var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; });
4545
var result = sut.ParseArguments<Options_With_Similar_Names>(args);
4646
result.Should().BeOfType<Parsed<Options_With_Similar_Names>>();
4747
result.As<Parsed<Options_With_Similar_Names>>().Value.Should().BeEquivalentTo(expected);
@@ -68,61 +68,61 @@ public static void Values_with_same_name_as_sequence_option_do_not_cause_later_v
6868

6969
// Issue #454
7070
[Theory]
71-
[InlineData(false)]
72-
[InlineData(true)]
71+
[InlineData(ParserMode.Legacy)]
72+
[InlineData(ParserMode.Getopt)]
7373

74-
public static void Enumerable_with_colon_separator_before_values_does_not_try_to_parse_too_much(bool useGetoptMode)
74+
public static void Enumerable_with_colon_separator_before_values_does_not_try_to_parse_too_much(ParserMode parserMode)
7575
{
7676
var args = "-c chanA:chanB file.hdf5".Split();
7777
var expected = new Options_For_Issue_454 {
7878
Channels = new[] { "chanA", "chanB" },
7979
ArchivePath = "file.hdf5",
8080
};
81-
var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; });
81+
var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; });
8282
var result = sut.ParseArguments<Options_For_Issue_454>(args);
8383
result.Should().BeOfType<Parsed<Options_For_Issue_454>>();
8484
result.As<Parsed<Options_For_Issue_454>>().Value.Should().BeEquivalentTo(expected);
8585
}
8686

8787
// Issue #510
8888
[Theory]
89-
[InlineData(false)]
90-
[InlineData(true)]
89+
[InlineData(ParserMode.Legacy)]
90+
[InlineData(ParserMode.Getopt)]
9191

92-
public static void Enumerable_before_values_does_not_try_to_parse_too_much(bool useGetoptMode)
92+
public static void Enumerable_before_values_does_not_try_to_parse_too_much(ParserMode parserMode)
9393
{
9494
var args = new[] { "-a", "1,2", "c" };
9595
var expected = new Options_For_Issue_510 { A = new[] { "1", "2" }, C = "c" };
96-
var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; });
96+
var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; });
9797
var result = sut.ParseArguments<Options_For_Issue_510>(args);
9898
result.Should().BeOfType<Parsed<Options_For_Issue_510>>();
9999
result.As<Parsed<Options_For_Issue_510>>().Value.Should().BeEquivalentTo(expected);
100100
}
101101

102102
// Issue #617
103103
[Theory]
104-
[InlineData(false)]
105-
[InlineData(true)]
104+
[InlineData(ParserMode.Legacy)]
105+
[InlineData(ParserMode.Getopt)]
106106

107-
public static void Enumerable_with_enum_before_values_does_not_try_to_parse_too_much(bool useGetoptMode)
107+
public static void Enumerable_with_enum_before_values_does_not_try_to_parse_too_much(ParserMode parserMode)
108108
{
109109
var args = "--fm D,C a.txt".Split();
110110
var expected = new Options_For_Issue_617 {
111111
Mode = new[] { FMode.D, FMode.C },
112112
Files = new[] { "a.txt" },
113113
};
114-
var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; });
114+
var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; });
115115
var result = sut.ParseArguments<Options_For_Issue_617>(args);
116116
result.Should().BeOfType<Parsed<Options_For_Issue_617>>();
117117
result.As<Parsed<Options_For_Issue_617>>().Value.Should().BeEquivalentTo(expected);
118118
}
119119

120120
// Issue #619
121121
[Theory]
122-
[InlineData(false)]
123-
[InlineData(true)]
122+
[InlineData(ParserMode.Legacy)]
123+
[InlineData(ParserMode.Getopt)]
124124

125-
public static void Separator_just_before_values_does_not_try_to_parse_values(bool useGetoptMode)
125+
public static void Separator_just_before_values_does_not_try_to_parse_values(ParserMode parserMode)
126126
{
127127
var args = "--outdir ./x64/Debug --modules ../utilities/x64/Debug,../auxtool/x64/Debug m_xfunit.f03 m_xfunit_assertion.f03".Split();
128128
var expected = new Options_For_Issue_619 {
@@ -131,7 +131,7 @@ public static void Separator_just_before_values_does_not_try_to_parse_values(boo
131131
Ignores = Enumerable.Empty<string>(),
132132
Srcs = new[] { "m_xfunit.f03", "m_xfunit_assertion.f03" },
133133
};
134-
var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; });
134+
var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; });
135135
var result = sut.ParseArguments<Options_For_Issue_619>(args);
136136
result.Should().BeOfType<Parsed<Options_For_Issue_619>>();
137137
result.As<Parsed<Options_For_Issue_619>>().Value.Should().BeEquivalentTo(expected);

0 commit comments

Comments
 (0)