Skip to content

Commit

Permalink
Simplify tokenizer's switch statement
Browse files Browse the repository at this point in the history 
The short and long option code can move into separate functions, which
makes the big switch statement much easier to read.

Also remove one TODO comment that's not relevant to the current feature.
  • Loading branch information
@rmunn
rmunn committed Mar 25, 2020
1 parent 3bed6b4 commit ee77b84
Showing 1 changed file with 97 additions and 84 deletions.
181 changes: 97 additions & 84 deletions src/CommandLine/Core/Tokenizer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ public static Result<IEnumerable<Token>, Error> Tokenize(
Action<string> onUnknownOption = ignoreUnknownArguments ? doNothing : unknownOptionError;

int consumeNext = 0;
Action<int> onConsumeNext = (n => consumeNext = consumeNext + n);

var tokens = new List<Token>();
Action<string> addValue = (s => tokens.Add(new Value(s)));
Action<string> addName = (s => tokens.Add(new Name(s)));

var enumerator = arguments.GetEnumerator();
while (enumerator.MoveNext())
Expand All @@ -44,7 +44,7 @@ public static Result<IEnumerable<Token>, Error> Tokenize(
break;

case string arg when consumeNext > 0:
addValue(arg);
tokens.Add(new Value(arg));
consumeNext = consumeNext - 1;
break;

Expand All @@ -53,100 +53,25 @@ public static Result<IEnumerable<Token>, Error> Tokenize(
break;

case "--":
addValue("--");
tokens.Add(new Value("--"));
break;

case "-":
// A single hyphen is always a value (it usually means "read from stdin" or "write to stdout")
addValue("-");
break;

case string arg when arg.StartsWith("--") && arg.Contains("="):
string[] parts = arg.Substring(2).Split(new char[] { '=' }, 2);
if (String.IsNullOrWhiteSpace(parts[0]) || parts[0].Contains(" "))
{
onBadFormatToken(arg);
}
else
{
switch(nameLookup(parts[0]))
{
case NameLookupResult.NoOptionFound:
onUnknownOption(parts[0]);
break;

default:
addName(parts[0]);
addValue(parts[1]);
break;
}
}
tokens.Add(new Value("-"));
break;

case string arg when arg.StartsWith("--"):
var name = arg.Substring(2);
switch (nameLookup(name))
{
case NameLookupResult.OtherOptionFound:
addName(name);
consumeNext = 1;
break;

case NameLookupResult.NoOptionFound:
// When ignoreUnknownArguments is true and AutoHelp is true, calling code is responsible for
// setting up nameLookup so that it will return a known name for --help, so that we don't skip it here
onUnknownOption(name);
break;

default:
addName(name);
break;
}
tokens.AddRange(TokenizeLongName(arg, nameLookup, onBadFormatToken, onUnknownOption, onConsumeNext));
break;

case string arg when arg.StartsWith("-"):
// First option char that requires a value means we swallow the rest of the string as the value
// But if there is no rest of the string, then instead we swallow the next argument
string chars = arg.Substring(1);
int len = chars.Length;
if (len > 0 && Char.IsDigit(chars[0]))
{
// Assume it's a negative number
addValue(arg);
continue;
}
for (int i = 0; i < len; i++)
{
var s = new String(chars[i], 1);
switch(nameLookup(s))
{
case NameLookupResult.OtherOptionFound:
addName(s);
if (i+1 < len)
{
addValue(chars.Substring(i+1));
i = len; // Can't use "break" inside a switch, so this breaks out of the loop
}
else
{
consumeNext = 1;
}
break;

case NameLookupResult.NoOptionFound:
onUnknownOption(s);
break;

default:
addName(s);
break;
}
}
tokens.AddRange(TokenizeShortName(arg, nameLookup, onUnknownOption, onConsumeNext));
break;

case string arg:
// If we get this far, it's a plain value
addValue(arg);
tokens.Add(new Value(arg));
break;
}
}
Expand All @@ -158,7 +83,6 @@ public static Result<IEnumerable<Token>, Error> ExplodeOptionList(
Result<IEnumerable<Token>, Error> tokenizerResult,
Func<string, Maybe<char>> optionSequenceWithSeparatorLookup)
{
// TODO: I don't like how this works. I don't want "-s foo;bar baz" to put three values into -s. Let's instead have a third token type, List, besides Name and Value.
var tokens = tokenizerResult.SucceededWith().Memoize();

var replaces = tokens.Select((t, i) =>
Expand Down Expand Up @@ -192,5 +116,94 @@ public static Func<
return explodedTokens;
};
}

private static IEnumerable<Token> TokenizeShortName(
string arg,
Func<string, NameLookupResult> nameLookup,
Action<string> onUnknownOption,
Action<int> onConsumeNext)
{

// First option char that requires a value means we swallow the rest of the string as the value
// But if there is no rest of the string, then instead we swallow the next argument
string chars = arg.Substring(1);
int len = chars.Length;
if (len > 0 && Char.IsDigit(chars[0]))
{
// Assume it's a negative number
yield return Token.Value(arg);
yield break;
}
for (int i = 0; i < len; i++)
{
var s = new String(chars[i], 1);
switch(nameLookup(s))
{
case NameLookupResult.OtherOptionFound:
yield return Token.Name(s);

if (i+1 < len)
{
// Rest of this is the value (e.g. "-sfoo" where "-s" is a string-consuming arg)
yield return Token.Value(chars.Substring(i+1));
yield break;
}
else
{
// Value is in next param (e.g., "-s foo")
onConsumeNext(1);
}
break;

case NameLookupResult.NoOptionFound:
onUnknownOption(s);
break;

default:
yield return Token.Name(s);
break;
}
}
}

private static IEnumerable<Token> TokenizeLongName(
string arg,
Func<string, NameLookupResult> nameLookup,
Action<string> onBadFormatToken,
Action<string> onUnknownOption,
Action<int> onConsumeNext)
{
string[] parts = arg.Substring(2).Split(new char[] { '=' }, 2);
string name = parts[0];
string value = (parts.Length > 1) ? parts[1] : null;
// A parameter like "--stringvalue=" is acceptable, and makes stringvalue be the empty string
if (String.IsNullOrWhiteSpace(name) || name.Contains(" "))
{
onBadFormatToken(arg);
yield break;
}
switch(nameLookup(name))
{
case NameLookupResult.NoOptionFound:
onUnknownOption(name);
yield break;

case NameLookupResult.OtherOptionFound:
yield return Token.Name(name);
if (value == null) // NOT String.IsNullOrEmpty
{
onConsumeNext(1);
}
else
{
yield return Token.Value(value);
}
break;

default:
yield return Token.Name(name);
break;
}
}
}
}

0 comments on commit ee77b84

Please sign in to comment.