Skip to content

Commit

Permalink
Revert "Address review comments"
Browse files Browse the repository at this point in the history 
This reverts commit b7102d8.
  • Loading branch information
@moh-hassan
moh-hassan committed Aug 25, 2020
1 parent b7102d8 commit 34ab560
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 83 deletions.
32 changes: 31 additions & 1 deletion src/CommandLine/Core/GetoptTokenizer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,36 @@ public static Result<IEnumerable<Token>, Error> Tokenize(
return Result.Succeed<IEnumerable<Token>, Error>(tokens.AsEnumerable(), errors.AsEnumerable());
}

public static Result<IEnumerable<Token>, Error> ExplodeOptionList(
Result<IEnumerable<Token>, Error> tokenizerResult,
Func<string, Maybe<char>> optionSequenceWithSeparatorLookup)
{
var tokens = tokenizerResult.SucceededWith().Memoize();

var exploded = new List<Token>(tokens is ICollection<Token> coll ? coll.Count : tokens.Count());
var nothing = Maybe.Nothing<char>(); // Re-use same Nothing instance for efficiency
var separator = nothing;
foreach (var token in tokens) {
if (token.IsName()) {
separator = optionSequenceWithSeparatorLookup(token.Text);
exploded.Add(token);
} else {
// Forced values are never considered option values, so they should not be split
if (separator.MatchJust(out char sep) && sep != '\0' && !token.IsValueForced()) {
if (token.Text.Contains(sep)) {
exploded.AddRange(token.Text.Split(sep).Select(Token.ValueFromSeparator));
} else {
exploded.Add(token);
}
} else {
exploded.Add(token);
}
separator = nothing; // Only first value after a separator can possibly be split
}
}
return Result.Succeed(exploded as IEnumerable<Token>, tokenizerResult.SuccessMessages());
}

public static Func<
IEnumerable<string>,
IEnumerable<OptionSpecification>,
Expand All @@ -101,7 +131,7 @@ public static Func<
return (arguments, optionSpecs) =>
{
var tokens = GetoptTokenizer.Tokenize(arguments, name => NameLookup.Contains(name, optionSpecs, nameComparer), ignoreUnknownArguments, enableDashDash, posixlyCorrect);
var explodedTokens = Tokenizer.ExplodeOptionList(tokens, name => NameLookup.HavingSeparator(name, optionSpecs, nameComparer));
var explodedTokens = GetoptTokenizer.ExplodeOptionList(tokens, name => NameLookup.HavingSeparator(name, optionSpecs, nameComparer));
return explodedTokens;
};
}
Expand Down
20 changes: 4 additions & 16 deletions src/CommandLine/Parser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,28 +185,16 @@ private static Result<IEnumerable<Token>, Error> Tokenize(
IEnumerable<OptionSpecification> optionSpecs,
ParserSettings settings)
{
switch (settings.ParserMode)
{
case ParserMode.Legacy:
return Tokenizer.ConfigureTokenizer(
settings.NameComparer,
settings.IgnoreUnknownArguments,
settings.EnableDashDash)(arguments, optionSpecs);

case ParserMode.Getopt:
return GetoptTokenizer.ConfigureTokenizer(
return settings.GetoptMode
? GetoptTokenizer.ConfigureTokenizer(
settings.NameComparer,
settings.IgnoreUnknownArguments,
settings.EnableDashDash,
settings.PosixlyCorrect)(arguments, optionSpecs);

// No need to test ParserMode.Default, as it should always be one of the above modes
default:
return Tokenizer.ConfigureTokenizer(
settings.PosixlyCorrect)(arguments, optionSpecs)
: Tokenizer.ConfigureTokenizer(
settings.NameComparer,
settings.IgnoreUnknownArguments,
settings.EnableDashDash)(arguments, optionSpecs);
}
}

private static ParserResult<T> MakeParserResult<T>(ParserResult<T> parserResult, ParserSettings settings)
Expand Down
45 changes: 10 additions & 35 deletions src/CommandLine/ParserSettings.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,6 @@

namespace CommandLine
{
public enum ParserMode
{
Legacy,
Getopt,

Default = Legacy
}

/// <summary>
/// Provides settings for <see cref="CommandLine.Parser"/>. Once consumed cannot be reused.
/// </summary>
Expand All @@ -35,7 +27,7 @@ public class ParserSettings : IDisposable
private Maybe<bool> enableDashDash;
private int maximumDisplayWidth;
private Maybe<bool> allowMultiInstance;
private ParserMode parserMode;
private bool getoptMode;
private Maybe<bool> posixlyCorrect;

/// <summary>
Expand All @@ -49,7 +41,7 @@ public ParserSettings()
autoVersion = true;
parsingCulture = CultureInfo.InvariantCulture;
maximumDisplayWidth = GetWindowWidth();
parserMode = ParserMode.Default;
getoptMode = false;
enableDashDash = Maybe.Nothing<bool>();
allowMultiInstance = Maybe.Nothing<bool>();
posixlyCorrect = Maybe.Nothing<bool>();
Expand Down Expand Up @@ -174,11 +166,11 @@ public bool AutoVersion
/// <summary>
/// Gets or sets a value indicating whether enable double dash '--' syntax,
/// that forces parsing of all subsequent tokens as values.
/// Normally defaults to false. If ParserMode = ParserMode.Getopt, this defaults to true, but can be turned off by explicitly specifying EnableDashDash = false.
/// If GetoptMode is true, this defaults to true, but can be turned off by explicitly specifying EnableDashDash = false.
/// </summary>
public bool EnableDashDash
{
get => enableDashDash.MatchJust(out bool value) ? value : (parserMode == ParserMode.Getopt);
get => enableDashDash.MatchJust(out bool value) ? value : getoptMode;
set => PopsicleSetter.Set(Consumed, ref enableDashDash, Maybe.Just(value));
}

Expand All @@ -193,38 +185,21 @@ public int MaximumDisplayWidth

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

/// <summary>
/// Set this to change how the parser processes command-line arguments. Currently valid values are:
/// <list>
/// <item>
/// <term>Legacy</term>
/// <description>Uses - for short options and -- for long options.
/// Values of long options can only start with a - character if the = syntax is used.
/// E.g., "--string-option -x" will consider "-x" to be an option, not the value of "--string-option",
/// but "--string-option=-x" will consider "-x" to be the value of "--string-option".</description>
/// </item>
/// <item>
/// <term>Getopt</term>
/// <description>Strict getopt-like processing is applied to option values.
/// Mostly like legacy mode, except that option values with = and with space are more consistent.
/// 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 "-".
/// E.g., both "--string-option=-x" and "--string-option -x" will consider "-x" to be the value of "--string-option".
/// If this mode is chosen, AllowMultiInstance and EnableDashDash will default to true as well, though they can be explicitly turned off if desired.</description>
/// </item>
/// </list>
/// Whether strict getopt-like processing is applied to option values; if true, AllowMultiInstance and EnableDashDash will default to true as well.
/// </summary>
public ParserMode ParserMode
public bool GetoptMode
{
get => parserMode;
set => PopsicleSetter.Set(Consumed, ref parserMode, value);
get => getoptMode;
set => PopsicleSetter.Set(Consumed, ref getoptMode, value);
}

/// <summary>
Expand Down
6 changes: 3 additions & 3 deletions tests/CommandLine.Tests/Unit/Core/GetoptTokenizerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public void Explode_scalar_with_separator_in_odd_args_input_returns_sequence()

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

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

Assert.NotNull(errors);
Assert.NotEmpty(errors);
Assert.Equal(1, errors.Count());
Assert.Equal(ErrorType.BadFormatTokenError, errors.First().Tag);

var tokens = result.SucceededWith();
Expand Down
8 changes: 4 additions & 4 deletions tests/CommandLine.Tests/Unit/GetoptParserTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public void Getopt_parser_without_posixly_correct_allows_mixed_options_and_nonop
{
// Arrange
var sut = new Parser(config => {
config.ParserMode = ParserMode.Getopt;
config.GetoptMode = true;
config.PosixlyCorrect = false;
});

Expand Down Expand Up @@ -215,7 +215,7 @@ public void Getopt_parser_with_posixly_correct_stops_parsing_at_first_nonoption(
{
// Arrange
var sut = new Parser(config => {
config.ParserMode = ParserMode.Getopt;
config.GetoptMode = true;
config.PosixlyCorrect = true;
config.EnableDashDash = true;
});
Expand All @@ -233,7 +233,7 @@ public void Getopt_mode_defaults_to_EnableDashDash_being_true()
{
// Arrange
var sut = new Parser(config => {
config.ParserMode = ParserMode.Getopt;
config.GetoptMode = true;
config.PosixlyCorrect = false;
});
var args = new string [] {"--stringvalue", "foo", "256", "--", "-x", "-sbar" };
Expand All @@ -259,7 +259,7 @@ public void Getopt_mode_can_have_EnableDashDash_expicitly_disabled()
{
// Arrange
var sut = new Parser(config => {
config.ParserMode = ParserMode.Getopt;
config.GetoptMode = true;
config.PosixlyCorrect = false;
config.EnableDashDash = false;
});
Expand Down
48 changes: 24 additions & 24 deletions tests/CommandLine.Tests/Unit/SequenceParsingTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,31 +17,31 @@ public class SequenceParsingTests
{
// Issue #91
[Theory]
[InlineData(ParserMode.Legacy)]
[InlineData(ParserMode.Getopt)]
public static void Enumerable_with_separator_before_values_does_not_try_to_parse_too_much(ParserMode parserMode)
[InlineData(false)]
[InlineData(true)]
public static void Enumerable_with_separator_before_values_does_not_try_to_parse_too_much(bool useGetoptMode)
{
var args = "--exclude=a,b InputFile.txt".Split();
var expected = new Options_For_Issue_91 {
Excluded = new[] { "a", "b" },
Included = Enumerable.Empty<string>(),
InputFileName = "InputFile.txt",
};
var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; });
var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; });
var result = sut.ParseArguments<Options_For_Issue_91>(args);
result.Should().BeOfType<Parsed<Options_For_Issue_91>>();
result.As<Parsed<Options_For_Issue_91>>().Value.Should().BeEquivalentTo(expected);
}

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

// Issue #454
[Theory]
[InlineData(ParserMode.Legacy)]
[InlineData(ParserMode.Getopt)]
[InlineData(false)]
[InlineData(true)]

public static void Enumerable_with_colon_separator_before_values_does_not_try_to_parse_too_much(ParserMode parserMode)
public static void Enumerable_with_colon_separator_before_values_does_not_try_to_parse_too_much(bool useGetoptMode)
{
var args = "-c chanA:chanB file.hdf5".Split();
var expected = new Options_For_Issue_454 {
Channels = new[] { "chanA", "chanB" },
ArchivePath = "file.hdf5",
};
var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; });
var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; });
var result = sut.ParseArguments<Options_For_Issue_454>(args);
result.Should().BeOfType<Parsed<Options_For_Issue_454>>();
result.As<Parsed<Options_For_Issue_454>>().Value.Should().BeEquivalentTo(expected);
}

// Issue #510
[Theory]
[InlineData(ParserMode.Legacy)]
[InlineData(ParserMode.Getopt)]
[InlineData(false)]
[InlineData(true)]

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

// Issue #617
[Theory]
[InlineData(ParserMode.Legacy)]
[InlineData(ParserMode.Getopt)]
[InlineData(false)]
[InlineData(true)]

public static void Enumerable_with_enum_before_values_does_not_try_to_parse_too_much(ParserMode parserMode)
public static void Enumerable_with_enum_before_values_does_not_try_to_parse_too_much(bool useGetoptMode)
{
var args = "--fm D,C a.txt".Split();
var expected = new Options_For_Issue_617 {
Mode = new[] { FMode.D, FMode.C },
Files = new[] { "a.txt" },
};
var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; });
var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; });
var result = sut.ParseArguments<Options_For_Issue_617>(args);
result.Should().BeOfType<Parsed<Options_For_Issue_617>>();
result.As<Parsed<Options_For_Issue_617>>().Value.Should().BeEquivalentTo(expected);
}

// Issue #619
[Theory]
[InlineData(ParserMode.Legacy)]
[InlineData(ParserMode.Getopt)]
[InlineData(false)]
[InlineData(true)]

public static void Separator_just_before_values_does_not_try_to_parse_values(ParserMode parserMode)
public static void Separator_just_before_values_does_not_try_to_parse_values(bool useGetoptMode)
{
var args = "--outdir ./x64/Debug --modules ../utilities/x64/Debug,../auxtool/x64/Debug m_xfunit.f03 m_xfunit_assertion.f03".Split();
var expected = new Options_For_Issue_619 {
Expand All @@ -131,7 +131,7 @@ public static void Separator_just_before_values_does_not_try_to_parse_values(Par
Ignores = Enumerable.Empty<string>(),
Srcs = new[] { "m_xfunit.f03", "m_xfunit_assertion.f03" },
};
var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; });
var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; });
var result = sut.ParseArguments<Options_For_Issue_619>(args);
result.Should().BeOfType<Parsed<Options_For_Issue_619>>();
result.As<Parsed<Options_For_Issue_619>>().Value.Should().BeEquivalentTo(expected);
Expand Down

0 comments on commit 34ab560

Please sign in to comment.