Skip to content
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

Command line argument parsing improvements #1048

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 0 additions & 5 deletions src/Spectre.Console.Cli/CommandParseException.cs
Expand Up @@ -91,11 +91,6 @@ internal static CommandParseException LongOptionNameContainSymbol(TextBuffer rea
return CommandLineParseExceptionFactory.Create(reader.Original, token, "Invalid long option name.", "Invalid character.");
}

internal static CommandParseException UnterminatedQuote(string input, CommandTreeToken token)
{
return CommandLineParseExceptionFactory.Create(input, token, $"Encountered unterminated quoted string '{token.Value}'.", "Did you forget the closing quotation mark?");
}

internal static CommandParseException UnknownCommand(CommandModel model, CommandTree? node, IEnumerable<string> args, CommandTreeToken token)
{
var suggestion = CommandSuggestor.Suggest(model, node?.Command, token.Value);
Expand Down
42 changes: 29 additions & 13 deletions src/Spectre.Console.Cli/Internal/Parsing/CommandTreeParser.cs
Expand Up @@ -308,19 +308,35 @@ public CommandTreeParserResult Parse(IEnumerable<string> args)
{
// Is this a command?
if (current.Command.FindCommand(valueToken.Value, CaseSensitivity) == null)
{
if (parameter != null)
{
if (parameter.ParameterKind == ParameterKind.Flag)
{
if (!CliConstants.AcceptedBooleanValues.Contains(valueToken.Value, StringComparer.OrdinalIgnoreCase))
{
// Flags cannot be assigned a value.
throw CommandParseException.CannotAssignValueToFlag(context.Arguments, token);
}
}

value = stream.Consume(CommandTreeToken.Kind.String)?.Value;
{
if (parameter != null)
{
if (parameter.ParameterKind == ParameterKind.Flag)
{
if (!CliConstants.AcceptedBooleanValues.Contains(valueToken.Value, StringComparer.OrdinalIgnoreCase))
{
if (!valueToken.HadSeparator)
{
// Do nothing
// - assume valueToken is unrelated to the flag parameter (ie. we've parsed it unnecessarily)
// - rely on the "No value?" code below to set the flag to its default value
// - valueToken will be handled on the next pass of the parser
}
else
{
// Flags cannot be assigned a value.
throw CommandParseException.CannotAssignValueToFlag(context.Arguments, token);
}
}
else
{
value = stream.Consume(CommandTreeToken.Kind.String)?.Value;
}
}
else
{
value = stream.Consume(CommandTreeToken.Kind.String)?.Value;
}
}
else
{
Expand Down
7 changes: 6 additions & 1 deletion src/Spectre.Console.Cli/Internal/Parsing/CommandTreeToken.cs
Expand Up @@ -6,7 +6,12 @@ internal sealed class CommandTreeToken
public int Position { get; }
public string Value { get; }
public string Representation { get; }
public bool IsGrouped { get; set; }
public bool IsGrouped { get; set; }

/// <summary>
/// Gets or sets a value indicating whether a separater was encountered immediately before the <see cref="CommandTreeToken.Value"/>.
/// </summary>
public bool HadSeparator { get; set; }

public enum Kind
{
Expand Down
163 changes: 70 additions & 93 deletions src/Spectre.Console.Cli/Internal/Parsing/CommandTreeTokenizer.cs
Expand Up @@ -29,7 +29,14 @@ public static CommandTreeTokenizerResult Tokenize(IEnumerable<string> args)
var context = new CommandTreeTokenizerContext();

foreach (var arg in args)
{
{
if (string.IsNullOrEmpty(arg))
{
// Null strings in the args array are still represented as tokens
tokens.Add(new CommandTreeToken(CommandTreeToken.Kind.String, position, string.Empty, string.Empty));
continue;
}

var start = position;
var reader = new TextBuffer(previousReader, arg);

Expand All @@ -48,39 +55,30 @@ public static CommandTreeTokenizerResult Tokenize(IEnumerable<string> args)
}

private static int ParseToken(CommandTreeTokenizerContext context, TextBuffer reader, int position, int start, List<CommandTreeToken> tokens)
{
while (reader.Peek() != -1)
{
if (reader.ReachedEnd)
{
position += reader.Position - start;
break;
}

var character = reader.Peek();

// Eat whitespace
if (char.IsWhiteSpace(character))
{
reader.Consume();
continue;
}

if (character == '-')
{
// Option
tokens.AddRange(ScanOptions(context, reader));
}
else
{
// Command or argument
tokens.Add(ScanString(context, reader));
}

// Flush remaining tokens
context.FlushRemaining();
}

{
if (!reader.ReachedEnd && reader.Peek() == '-')
{
// Option
tokens.AddRange(ScanOptions(context, reader));
}
else
{
// Command or argument
while (reader.Peek() != -1)
{
if (reader.ReachedEnd)
{
position += reader.Position - start;
break;
}

tokens.Add(ScanString(context, reader));

// Flush remaining tokens
context.FlushRemaining();
}
}

return position;
}

Expand All @@ -89,15 +87,6 @@ private static int ParseToken(CommandTreeTokenizerContext context, TextBuffer re
TextBuffer reader,
char[]? stop = null)
{
if (reader.TryPeek(out var character))
{
// Is this a quoted string?
if (character == '\"')
{
return ScanQuotedString(context, reader);
}
}

var position = reader.Position;
var builder = new StringBuilder();
while (!reader.ReachedEnd)
Expand All @@ -113,48 +102,8 @@ private static int ParseToken(CommandTreeTokenizerContext context, TextBuffer re
builder.Append(current);
}

var value = builder.ToString();
return new CommandTreeToken(CommandTreeToken.Kind.String, position, value.Trim(), value);
}

private static CommandTreeToken ScanQuotedString(CommandTreeTokenizerContext context, TextBuffer reader)
{
var position = reader.Position;

context.FlushRemaining();
reader.Consume('\"');

var builder = new StringBuilder();
var terminated = false;
while (!reader.ReachedEnd)
{
var character = reader.Peek();
if (character == '\"')
{
terminated = true;
reader.Read();
break;
}

builder.Append(reader.Read());
}

if (!terminated)
{
var unterminatedQuote = builder.ToString();
var token = new CommandTreeToken(CommandTreeToken.Kind.String, position, unterminatedQuote, $"\"{unterminatedQuote}");
throw CommandParseException.UnterminatedQuote(reader.Original, token);
}

var quotedString = builder.ToString();

// Add to the context
context.AddRemaining(quotedString);

return new CommandTreeToken(
CommandTreeToken.Kind.String,
position, quotedString,
quotedString);
var value = builder.ToString();
return new CommandTreeToken(CommandTreeToken.Kind.String, position, value, value);
}

private static IEnumerable<CommandTreeToken> ScanOptions(CommandTreeTokenizerContext context, TextBuffer reader)
Expand All @@ -166,7 +115,7 @@ private static IEnumerable<CommandTreeToken> ScanOptions(CommandTreeTokenizerCon
reader.Consume('-');
context.AddRemaining('-');

if (!reader.TryPeek(out var character))
if (!reader.TryPeek(out var character) || character == ' ')
{
var token = new CommandTreeToken(CommandTreeToken.Kind.ShortOption, position, "-", "-");
throw CommandParseException.OptionHasNoName(reader.Original, token);
Expand Down Expand Up @@ -200,8 +149,10 @@ private static IEnumerable<CommandTreeToken> ScanOptions(CommandTreeTokenizerCon
var token = new CommandTreeToken(CommandTreeToken.Kind.String, reader.Position, "=", "=");
throw CommandParseException.OptionValueWasExpected(reader.Original, token);
}

result.Add(ScanString(context, reader));

var tokenValue = ScanString(context, reader);
tokenValue.HadSeparator = true;
result.Add(tokenValue);
}
}

Expand Down Expand Up @@ -235,12 +186,38 @@ private static IEnumerable<CommandTreeToken> ScanShortOptions(CommandTreeTokeniz
? new CommandTreeToken(CommandTreeToken.Kind.ShortOption, position, value, $"-{value}")
: new CommandTreeToken(CommandTreeToken.Kind.ShortOption, position + result.Count, value, value));
}
else
else if (result.Count == 0 && char.IsDigit(current))
{
// We require short options to be named with letters. Short options that start with a number
// ("-1", "-2ab", "-3..7") may actually mean values (either for options or arguments) and will
// be tokenized as strings. This block handles parsing those cases, but we only allow this
// when the digit is the first character in the token (i.e. "-a1" is always an error), hence the
// result.Count == 0 check above.
string value = string.Empty;

while (!reader.ReachedEnd)
{
char c = reader.Peek();

if (char.IsWhiteSpace(c))
{
break;
}

value += c.ToString(CultureInfo.InvariantCulture);
reader.Read();
}

value = "-" + value; // Prefix with the minus sign that we originally thought to mean a short option
result.Add(new CommandTreeToken(CommandTreeToken.Kind.String, position, value, value));
}
else
{
// Create a token representing the short option.
var tokenPosition = position + 1 + result.Count;
var represntation = current.ToString(CultureInfo.InvariantCulture);
var token = new CommandTreeToken(CommandTreeToken.Kind.ShortOption, tokenPosition, represntation, represntation);
var representation = current.ToString(CultureInfo.InvariantCulture);
var tokenPosition = position + 1 + result.Count;
var token = new CommandTreeToken(CommandTreeToken.Kind.ShortOption, tokenPosition, representation, representation);

throw CommandParseException.InvalidShortOptionName(reader.Original, token);
}
}
Expand Down Expand Up @@ -271,7 +248,7 @@ private static CommandTreeToken ScanLongOption(CommandTreeTokenizerContext conte
var name = ScanString(context, reader, new[] { '=', ':' });

// Perform validation of the name.
if (name.Value.Length == 0)
if (name.Value == " ")
{
throw CommandParseException.LongOptionNameIsMissing(reader, position);
}
Expand Down
@@ -1,4 +1,4 @@
Error: Flags cannot be assigned a value.

dog --alive foo
dog --alive=indeterminate foo
^^^^^^^ Can't assign value
@@ -1,4 +1,4 @@
Error: Flags cannot be assigned a value.

dog -a foo
dog -a=indeterminate foo
^^ Can't assign value

This file was deleted.

This file was deleted.