Skip to content

Commit

Permalink
cleanup: make .Values read-only on CommandArgument/Option (#406)
Browse files Browse the repository at this point in the history 
Exposing a raw List<string> of values on CommandArgument and
CommandOption has made it difficult to cleanly implement features like
default values or parsing. This changes the return type of .Values to a
read-only list, and adds TryParse for adding new values, and Reset for
clearing them.
  • Loading branch information
@natemcmaster
natemcmaster committed Jan 10, 2021
1 parent 30e763d commit 7c573c1
Show file tree
Hide file tree
Showing 11 changed files with 60 additions and 35 deletions.
38 changes: 28 additions & 10 deletions src/CommandLineUtils/CommandArgument.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,7 @@ namespace McMaster.Extensions.CommandLineUtils
/// <seealso cref="CommandOption"/>
public class CommandArgument
{
/// <summary>
/// Initializes a new instance of <see cref="CommandArgument"/>.
/// </summary>
public CommandArgument()
{
Values = new List<string?>();
}
private protected List<string?> _values = new List<string?>();

/// <summary>
/// The name of the argument.
Expand All @@ -42,7 +36,7 @@ public CommandArgument()
/// <summary>
/// All values specified, if any.
/// </summary>
public List<string?> Values { get; private set; }
public IReadOnlyList<string?> Values => _values;

/// <summary>
/// Allow multiple values.
Expand All @@ -54,6 +48,11 @@ public CommandArgument()
/// </summary>
public string? Value => Values.FirstOrDefault();

/// <summary>
/// True if this argument has been assigned values.
/// </summary>
public bool HasValue => Values.Any();

/// <summary>
/// A collection of validators that execute before invoking <see cref="CommandLineApplication.OnExecute(Func{int})"/>.
/// When validation fails, <see cref="CommandLineApplication.ValidationErrorHandler"/> is invoked.
Expand All @@ -65,9 +64,28 @@ public CommandArgument()
/// </summary>
internal Type UnderlyingType { get; set; }

internal void Reset()
/// <summary>
/// Try to add a value to this argument.
/// </summary>
/// <param name="value">The argument value to be added.</param>
/// <returns>True if the value was accepted, false if the value cannot be added.</returns>
public bool TryParse(string? value)
{
if (_values.Count == 1 && !MultipleValues)
{
return false;
}

_values.Add(value);
return true;
}

/// <summary>
/// Clear any parsed values from this argument.
/// </summary>
public virtual void Reset()
{
Values.Clear();
_values.Clear();
}
}
}
1 change: 0 additions & 1 deletion src/CommandLineUtils/CommandLineApplication.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.ComponentModel.DataAnnotations;
using System.IO;
using System.Linq;
Expand Down
20 changes: 12 additions & 8 deletions src/CommandLineUtils/CommandOption.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;
using System.Text;
using McMaster.Extensions.CommandLineUtils.Validation;
Expand All @@ -17,6 +16,8 @@ namespace McMaster.Extensions.CommandLineUtils
/// </summary>
public class CommandOption
{
private protected readonly List<string?> _values = new List<string?>();

/// <summary>
/// Initializes a new <see cref="CommandOption"/>.
/// </summary>
Expand Down Expand Up @@ -99,7 +100,7 @@ internal CommandOption(CommandOptionType type)
/// <summary>
/// Any values found during parsing, if any.
/// </summary>
public List<string?> Values { get; } = new List<string?>();
public IReadOnlyList<string?> Values => _values;

/// <summary>
/// Defines the type of the option.
Expand Down Expand Up @@ -138,15 +139,15 @@ public bool TryParse(string? value)
switch (OptionType)
{
case CommandOptionType.MultipleValue:
Values.Add(value);
_values.Add(value);
break;
case CommandOptionType.SingleOrNoValue:
case CommandOptionType.SingleValue:
if (Values.Any())
if (_values.Any())
{
return false;
}
Values.Add(value);
_values.Add(value);
break;
case CommandOptionType.NoValue:
if (value != null)
Expand All @@ -156,7 +157,7 @@ public bool TryParse(string? value)

// Add a value so .HasValue() == true
// Also, the count can be used to determine how many times a flag was specified
Values.Add(null);
_values.Add(null);
break;
default:
throw new NotImplementedException();
Expand Down Expand Up @@ -235,9 +236,12 @@ private bool IsEnglishLetter(char c)
return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z');
}

internal void Reset()
/// <summary>
/// Clear any parsed values from this argument.
/// </summary>
public virtual void Reset()
{
Values.Clear();
_values.Clear();
}
}
}
2 changes: 1 addition & 1 deletion src/CommandLineUtils/Internal/CommandLineProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ private bool ProcessCommandOrParameter(CommandOrParameterArgument arg)

if (_currentCommandArguments.MoveNext())
{
_currentCommandArguments.Current.Values.Add(arg.Raw);
_currentCommandArguments.Current.TryParse(arg.Raw);
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions src/CommandLineUtils/PublicAPI.Shipped.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ McMaster.Extensions.CommandLineUtils.CommandArgument.ShowInHelpText.get -> bool
McMaster.Extensions.CommandLineUtils.CommandArgument.ShowInHelpText.set -> void
McMaster.Extensions.CommandLineUtils.CommandArgument.Validators.get -> System.Collections.Generic.ICollection<McMaster.Extensions.CommandLineUtils.Validation.IArgumentValidator!>!
McMaster.Extensions.CommandLineUtils.CommandArgument.Value.get -> string?
McMaster.Extensions.CommandLineUtils.CommandArgument.Values.get -> System.Collections.Generic.List<string?>!
McMaster.Extensions.CommandLineUtils.CommandArgument.Values.get -> System.Collections.Generic.IReadOnlyList<string?>!
McMaster.Extensions.CommandLineUtils.CommandArgument<T>
McMaster.Extensions.CommandLineUtils.CommandArgument<T>.CommandArgument(McMaster.Extensions.CommandLineUtils.Abstractions.IValueParser<T>! valueParser) -> void
McMaster.Extensions.CommandLineUtils.CommandArgument<T>.ParsedValue.get -> T
Expand Down Expand Up @@ -213,7 +213,7 @@ McMaster.Extensions.CommandLineUtils.CommandOption.Validators.get -> System.Coll
McMaster.Extensions.CommandLineUtils.CommandOption.Value() -> string?
McMaster.Extensions.CommandLineUtils.CommandOption.ValueName.get -> string?
McMaster.Extensions.CommandLineUtils.CommandOption.ValueName.set -> void
McMaster.Extensions.CommandLineUtils.CommandOption.Values.get -> System.Collections.Generic.List<string?>!
McMaster.Extensions.CommandLineUtils.CommandOption.Values.get -> System.Collections.Generic.IReadOnlyList<string?>!
McMaster.Extensions.CommandLineUtils.CommandOption<T>
McMaster.Extensions.CommandLineUtils.CommandOption<T>.CommandOption(McMaster.Extensions.CommandLineUtils.Abstractions.IValueParser<T>! valueParser, string! template, McMaster.Extensions.CommandLineUtils.CommandOptionType optionType) -> void
McMaster.Extensions.CommandLineUtils.CommandOption<T>.ParsedValue.get -> T
Expand Down
4 changes: 4 additions & 0 deletions src/CommandLineUtils/PublicAPI.Unshipped.txt
View file
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
#nullable enable
McMaster.Extensions.CommandLineUtils.CommandArgument.HasValue.get -> bool
McMaster.Extensions.CommandLineUtils.CommandArgument.TryParse(string? value) -> bool
virtual McMaster.Extensions.CommandLineUtils.CommandArgument.Reset() -> void
virtual McMaster.Extensions.CommandLineUtils.CommandOption.Reset() -> void
2 changes: 1 addition & 1 deletion src/CommandLineUtils/Validation/AttributeValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public ValidationResult GetValidationResult(CommandOption option, ValidationCont
public ValidationResult GetValidationResult(CommandArgument argument, ValidationContext context)
=> GetValidationResult(argument.Values, context);

private ValidationResult GetValidationResult(List<string?>? values, ValidationContext context)
private ValidationResult GetValidationResult(IReadOnlyList<string?>? values, ValidationContext context)
{
if (values == null)
{
Expand Down
8 changes: 4 additions & 4 deletions test/CommandLineUtils.Tests/AttributeValidatorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public void ItOnlyInvokesAttributeIfValueExists()

Assert.Equal(ValidationResult.Success, validator.GetValidationResult(arg, context));

arg.Values.Add(null);
arg.TryParse(null);

Assert.Throws<InvalidOperationException>(() => validator.GetValidationResult(arg, context));
}
Expand All @@ -46,12 +46,12 @@ public void ItExecutesValidationAttribute(Type attributeType, string validValue,
var factory = new CommandLineValidationContextFactory(app);
var context = factory.Create(arg);

arg.Values.Add(validValue);
arg.TryParse(validValue);

Assert.Equal(ValidationResult.Success, validator.GetValidationResult(arg, context));

arg.Values.Clear();
arg.Values.Add(invalidValue);
arg.Reset();
arg.TryParse(invalidValue);
var result = validator.GetValidationResult(arg, context);
Assert.NotNull(result);
Assert.NotEmpty(result.ErrorMessage);
Expand Down
4 changes: 2 additions & 2 deletions test/CommandLineUtils.Tests/CommandLineApplicationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,7 @@ public void OptionsWithSameName()
Assert.Equal("top", top.Value());
Assert.Null(nested?.Value());

top.Values.Clear();
top.Reset();

app.Execute("subcmd", "-a", "nested");
Assert.Null(top.Value());
Expand Down Expand Up @@ -765,7 +765,7 @@ public void OptionSeparatorMustNotUseSpace()
app.Parse("--debug:hive", "abc"));
Assert.Equal("Missing value for option 'debug:hive'", ex.Message);

option.Values.Clear();
option.Reset();
app.Parse("--debug:hive=abc");
Assert.Equal("abc", option.Value());
}
Expand Down
10 changes: 5 additions & 5 deletions test/CommandLineUtils.Tests/DefaultHelpTextGeneratorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,10 @@ public class MyApp
}

[Theory]
[InlineData("-h", "-h", " -h Show help information.", " Subcommand ")]
[InlineData("--help", "--help", " --help Show help information.", " Subcommand ")]
[InlineData("-?", "-?", " -? Show help information.", " Subcommand ")]
[InlineData(null, "-?|-h|--help", " -?|-h|--help Show help information.", " Subcommand ")]
[InlineData("-h", "-h", " -h Show help information.", " Subcommand ")]
[InlineData("--help", "--help", " --help Show help information.", " Subcommand ")]
[InlineData("-?", "-?", " -? Show help information.", " Subcommand ")]
[InlineData(null, "-?|-h|--help", " -?|-h|--help Show help information.", " Subcommand ")]
public void ShowHelpWithSubcommands(string helpOption, string expectedHintText, string expectedOptionsText,
string expectedCommandsText)
{
Expand All @@ -261,7 +261,7 @@ public class MyApp
{expectedOptionsText}
Commands:
{expectedCommandsText}
{expectedCommandsText}
Run 'test [command] {expectedHintText}' for more information about a command.
Expand Down
2 changes: 1 addition & 1 deletion test/CommandLineUtils.Tests/ResponseFileTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ private string CreateResponseFile(params string[] lines)
return rsp;
}

private List<string?> ParseResponseFile(ResponseFileHandling options, params string[] responseFileLines)
private IReadOnlyList<string?> ParseResponseFile(ResponseFileHandling options, params string[] responseFileLines)
{
var rsp = CreateResponseFile(responseFileLines);
var app = new CommandLineApplication
Expand Down

0 comments on commit 7c573c1

Please sign in to comment.