Command line improvements (#1103)

Closes #187
Closes #203
Closes #1059

src/Spectre.Console.Cli/Internal/CommandExecutor.cs

@@ -45,12 +45,10 @@ internal sealed class CommandExecutor
         }
 
         // Parse and map the model against the arguments.
-        var parser = new CommandTreeParser(model, configuration.Settings);
-        var parsedResult = parser.Parse(args);
-        _registrar.RegisterInstance(typeof(CommandTreeParserResult), parsedResult);
+        var parsedResult = ParseCommandLineArguments(model, configuration.Settings, args);
 
         // Currently the root?
-        if (parsedResult.Tree == null)
+        if (parsedResult?.Tree == null)
         {
             // Display help.
             configuration.Settings.Console.SafeRender(HelpWriter.Write(model, configuration.Settings.ShowOptionDefaultValues));
@@ -75,6 +73,7 @@ internal sealed class CommandExecutor
         }
 
         // Register the arguments with the container.
+        _registrar.RegisterInstance(typeof(CommandTreeParserResult), parsedResult);
         _registrar.RegisterInstance(typeof(IRemainingArguments), parsedResult.Remaining);
 
         // Create the resolver and the context.
@@ -86,6 +85,34 @@ internal sealed class CommandExecutor
             return await Execute(leaf, parsedResult.Tree, context, resolver, configuration).ConfigureAwait(false);
         }
     }
+
+    private CommandTreeParserResult? ParseCommandLineArguments(CommandModel model, CommandAppSettings settings, IEnumerable<string> args)
+    {
+        var parser = new CommandTreeParser(model, settings.CaseSensitivity, settings.ParsingMode, settings.ConvertFlagsToRemainingArguments);
+
+        var parserContext = new CommandTreeParserContext(args, settings.ParsingMode);
+        var tokenizerResult = CommandTreeTokenizer.Tokenize(args);
+        var parsedResult = parser.Parse(parserContext, tokenizerResult);
+
+        var lastParsedLeaf = parsedResult?.Tree?.GetLeafCommand();
+        var lastParsedCommand = lastParsedLeaf?.Command;
+        if (lastParsedLeaf != null && lastParsedCommand != null &&
+            lastParsedCommand.IsBranch && !lastParsedLeaf.ShowHelp &&
+            lastParsedCommand.DefaultCommand != null)
+        {
+            // Insert this branch's default command into the command line
+            // arguments and try again to see if it will parse.
+            var argsWithDefaultCommand = new List<string>(args);
+
+            argsWithDefaultCommand.Insert(tokenizerResult.Tokens.Position, lastParsedCommand.DefaultCommand.Name);
+
+            parserContext = new CommandTreeParserContext(argsWithDefaultCommand, settings.ParsingMode);
+            tokenizerResult = CommandTreeTokenizer.Tokenize(argsWithDefaultCommand);
+            parsedResult = parser.Parse(parserContext, tokenizerResult);
+        }
+
+        return parsedResult;
+    }
 
     private static string ResolveApplicationVersion(IConfiguration configuration)
     {

src/Spectre.Console.Cli/Internal/Configuration/CommandAppSettings.cs

@@ -12,7 +12,8 @@ internal sealed class CommandAppSettings : ICommandAppSettings
     public bool PropagateExceptions { get; set; }
     public bool ValidateExamples { get; set; }
     public bool TrimTrailingPeriod { get; set; } = true;
-    public bool StrictParsing { get; set; }
+    public bool StrictParsing { get; set; }
+    public bool ConvertFlagsToRemainingArguments { get; set; } = false;
 
     public ParsingMode ParsingMode =>
         StrictParsing ? ParsingMode.Strict : ParsingMode.Relaxed;

src/Spectre.Console.Cli/Internal/Configuration/Configurator.cs

@@ -23,7 +23,7 @@ internal sealed class Configurator : IUnsafeConfigurator, IConfigurator, IConfig
     public void AddExample(string[] args)
     {
         Examples.Add(args);
-    }
+    }
 
     public ConfiguredCommand SetDefaultCommand<TDefaultCommand>()
         where TDefaultCommand : class, ICommand
@@ -36,7 +36,7 @@ internal sealed class Configurator : IUnsafeConfigurator, IConfigurator, IConfig
     public ICommandConfigurator AddCommand<TCommand>(string name)
         where TCommand : class, ICommand
     {
-        var command = Commands.AddAndReturn(ConfiguredCommand.FromType<TCommand>(name, false));
+        var command = Commands.AddAndReturn(ConfiguredCommand.FromType<TCommand>(name, isDefaultCommand: false));
         return new CommandConfigurator(command);
     }
 

src/Spectre.Console.Cli/Internal/Configuration/ConfiguratorOfT.cs

@@ -1,92 +1,100 @@
-namespace Spectre.Console.Cli;
-
-internal sealed class Configurator<TSettings> : IUnsafeBranchConfigurator, IConfigurator<TSettings>
-    where TSettings : CommandSettings
-{
-    private readonly ConfiguredCommand _command;
-    private readonly ITypeRegistrar? _registrar;
-
-    public Configurator(ConfiguredCommand command, ITypeRegistrar? registrar)
-    {
-        _command = command;
-        _registrar = registrar;
-    }
-
-    public void SetDescription(string description)
-    {
-        _command.Description = description;
-    }
-
-    public void AddExample(string[] args)
-    {
-        _command.Examples.Add(args);
-    }
-
-    public void HideBranch()
-    {
-        _command.IsHidden = true;
-    }
-
-    public ICommandConfigurator AddCommand<TCommand>(string name)
-        where TCommand : class, ICommandLimiter<TSettings>
-    {
-        var command = ConfiguredCommand.FromType<TCommand>(name);
-        var configurator = new CommandConfigurator(command);
-
-        _command.Children.Add(command);
-        return configurator;
-    }
-
-    public ICommandConfigurator AddDelegate<TDerivedSettings>(string name, Func<CommandContext, TDerivedSettings, int> func)
-        where TDerivedSettings : TSettings
-    {
-        var command = ConfiguredCommand.FromDelegate<TDerivedSettings>(
-            name, (context, settings) => func(context, (TDerivedSettings)settings));
-
-        _command.Children.Add(command);
-        return new CommandConfigurator(command);
-    }
-
-    public IBranchConfigurator AddBranch<TDerivedSettings>(string name, Action<IConfigurator<TDerivedSettings>> action)
-        where TDerivedSettings : TSettings
-    {
-        var command = ConfiguredCommand.FromBranch<TDerivedSettings>(name);
-        action(new Configurator<TDerivedSettings>(command, _registrar));
-        var added = _command.Children.AddAndReturn(command);
-        return new BranchConfigurator(added);
-    }
-
-    ICommandConfigurator IUnsafeConfigurator.AddCommand(string name, Type command)
-    {
-        var method = GetType().GetMethod("AddCommand");
-        if (method == null)
-        {
-            throw new CommandConfigurationException("Could not find AddCommand by reflection.");
-        }
-
-        method = method.MakeGenericMethod(command);
-
-        if (!(method.Invoke(this, new object[] { name }) is ICommandConfigurator result))
-        {
-            throw new CommandConfigurationException("Invoking AddCommand returned null.");
-        }
-
-        return result;
-    }
-
-    IBranchConfigurator IUnsafeConfigurator.AddBranch(string name, Type settings, Action<IUnsafeBranchConfigurator> action)
-    {
-        var command = ConfiguredCommand.FromBranch(settings, name);
-
-        // Create the configurator.
-        var configuratorType = typeof(Configurator<>).MakeGenericType(settings);
-        if (!(Activator.CreateInstance(configuratorType, new object?[] { command, _registrar }) is IUnsafeBranchConfigurator configurator))
-        {
-            throw new CommandConfigurationException("Could not create configurator by reflection.");
-        }
-
-        action(configurator);
-        var added = _command.Children.AddAndReturn(command);
-        return new BranchConfigurator(added);
-    }
+namespace Spectre.Console.Cli;
+
+internal sealed class Configurator<TSettings> : IUnsafeBranchConfigurator, IConfigurator<TSettings>
+    where TSettings : CommandSettings
+{
+    private readonly ConfiguredCommand _command;
+    private readonly ITypeRegistrar? _registrar;
+
+    public Configurator(ConfiguredCommand command, ITypeRegistrar? registrar)
+    {
+        _command = command;
+        _registrar = registrar;
+    }
+
+    public void SetDescription(string description)
+    {
+        _command.Description = description;
+    }
+
+    public void AddExample(string[] args)
+    {
+        _command.Examples.Add(args);
+    }
+
+    public void SetDefaultCommand<TDefaultCommand>()
+        where TDefaultCommand : class, ICommandLimiter<TSettings>
+    {
+        var defaultCommand = ConfiguredCommand.FromType<TDefaultCommand>(
+            CliConstants.DefaultCommandName, isDefaultCommand: true);
+
+        _command.Children.Add(defaultCommand);
+    }
+
+    public void HideBranch()
+    {
+        _command.IsHidden = true;
+    }
+
+    public ICommandConfigurator AddCommand<TCommand>(string name)
+        where TCommand : class, ICommandLimiter<TSettings>
+    {
+        var command = ConfiguredCommand.FromType<TCommand>(name, isDefaultCommand: false);
+        var configurator = new CommandConfigurator(command);
+
+        _command.Children.Add(command);
+        return configurator;
+    }
+
+    public ICommandConfigurator AddDelegate<TDerivedSettings>(string name, Func<CommandContext, TDerivedSettings, int> func)
+        where TDerivedSettings : TSettings
+    {
+        var command = ConfiguredCommand.FromDelegate<TDerivedSettings>(
+            name, (context, settings) => func(context, (TDerivedSettings)settings));
+
+        _command.Children.Add(command);
+        return new CommandConfigurator(command);
+    }
+
+    public IBranchConfigurator AddBranch<TDerivedSettings>(string name, Action<IConfigurator<TDerivedSettings>> action)
+        where TDerivedSettings : TSettings
+    {
+        var command = ConfiguredCommand.FromBranch<TDerivedSettings>(name);
+        action(new Configurator<TDerivedSettings>(command, _registrar));
+        var added = _command.Children.AddAndReturn(command);
+        return new BranchConfigurator(added);
+    }
+
+    ICommandConfigurator IUnsafeConfigurator.AddCommand(string name, Type command)
+    {
+        var method = GetType().GetMethod("AddCommand");
+        if (method == null)
+        {
+            throw new CommandConfigurationException("Could not find AddCommand by reflection.");
+        }
+
+        method = method.MakeGenericMethod(command);
+
+        if (!(method.Invoke(this, new object[] { name }) is ICommandConfigurator result))
+        {
+            throw new CommandConfigurationException("Invoking AddCommand returned null.");
+        }
+
+        return result;
+    }
+
+    IBranchConfigurator IUnsafeConfigurator.AddBranch(string name, Type settings, Action<IUnsafeBranchConfigurator> action)
+    {
+        var command = ConfiguredCommand.FromBranch(settings, name);
+
+        // Create the configurator.
+        var configuratorType = typeof(Configurator<>).MakeGenericType(settings);
+        if (!(Activator.CreateInstance(configuratorType, new object?[] { command, _registrar }) is IUnsafeBranchConfigurator configurator))
+        {
+            throw new CommandConfigurationException("Could not create configurator by reflection.");
+        }
+
+        action(configurator);
+        var added = _command.Children.AddAndReturn(command);
+        return new BranchConfigurator(added);
     }

src/Spectre.Console.Cli/Internal/Configuration/ConfiguredCommand.cs

@@ -27,7 +27,10 @@ internal sealed class ConfiguredCommand
         CommandType = commandType;
         SettingsType = settingsType;
         Delegate = @delegate;
-        IsDefaultCommand = isDefaultCommand;
+        IsDefaultCommand = isDefaultCommand;
+
+        // Default commands are always created as hidden.
+        IsHidden = IsDefaultCommand;
 
         Children = new List<ConfiguredCommand>();
         Examples = new List<string[]>();

src/Spectre.Console.Cli/Internal/Extensions/TypeRegistrarExtensions.cs

@@ -6,10 +6,6 @@ internal static class TypeRegistrarExtensions
     {
         var stack = new Stack<CommandInfo>();
         model.Commands.ForEach(c => stack.Push(c));
-        if (model.DefaultCommand != null)
-        {
-            stack.Push(model.DefaultCommand);
-        }
 
         while (stack.Count > 0)
         {

src/Spectre.Console.Cli/Internal/Modelling/CommandInfo.cs

@@ -1,7 +1,7 @@
 namespace Spectre.Console.Cli;
 
 internal sealed class CommandInfo : ICommandContainer
-{
+{
     public string Name { get; }
     public HashSet<string> Aliases { get; }
     public string? Description { get; }
@@ -10,14 +10,17 @@ internal sealed class CommandInfo : ICommandContainer
     public Type SettingsType { get; }
     public Func<CommandContext, CommandSettings, int>? Delegate { get; }
     public bool IsDefaultCommand { get; }
-    public bool IsHidden { get; }
     public CommandInfo? Parent { get; }
     public IList<CommandInfo> Children { get; }
     public IList<CommandParameter> Parameters { get; }
     public IList<string[]> Examples { get; }
 
     public bool IsBranch => CommandType == null && Delegate == null;
-    IList<CommandInfo> ICommandContainer.Commands => Children;
+    IList<CommandInfo> ICommandContainer.Commands => Children;
+
+    // only branches can have a default command
+    public CommandInfo? DefaultCommand => IsBranch ? Children.FirstOrDefault(c => c.IsDefaultCommand) : null;
+    public bool IsHidden { get; }
 
     public CommandInfo(CommandInfo? parent, ConfiguredCommand prototype)
     {

src/Spectre.Console.Cli/Internal/Modelling/CommandModel.cs

@@ -4,21 +4,20 @@ internal sealed class CommandModel : ICommandContainer
 {
     public string? ApplicationName { get; }
     public ParsingMode ParsingMode { get; }
-    public CommandInfo? DefaultCommand { get; }
     public IList<CommandInfo> Commands { get; }
     public IList<string[]> Examples { get; }
     public bool TrimTrailingPeriod { get; }
+
+    public CommandInfo? DefaultCommand => Commands.FirstOrDefault(c => c.IsDefaultCommand);
 
     public CommandModel(
         CommandAppSettings settings,
-        CommandInfo? defaultCommand,
         IEnumerable<CommandInfo> commands,
         IEnumerable<string[]> examples)
     {
         ApplicationName = settings.ApplicationName;
         ParsingMode = settings.ParsingMode;
         TrimTrailingPeriod = settings.TrimTrailingPeriod;
-        DefaultCommand = defaultCommand;
         Commands = new List<CommandInfo>(commands ?? Array.Empty<CommandInfo>());
         Examples = new List<string[]>(examples ?? Array.Empty<string[]>());
     }

src/Spectre.Console.Cli/Internal/Modelling/CommandModelBuilder.cs

@@ -1,5 +1,5 @@
 namespace Spectre.Console.Cli;
-
+
 internal static class CommandModelBuilder
 {
     // Consider removing this in favor for value tuples at some point.
@@ -25,18 +25,19 @@ internal static class CommandModelBuilder
             result.Add(Build(null, command));
         }
 
-        var defaultCommand = default(CommandInfo);
         if (configuration.DefaultCommand != null)
         {
             // Add the examples from the configuration to the default command.
             configuration.DefaultCommand.Examples.AddRange(configuration.Examples);
 
             // Build the default command.
-            defaultCommand = Build(null, configuration.DefaultCommand);
+            var defaultCommand = Build(null, configuration.DefaultCommand);
+
+            result.Add(defaultCommand);
         }
 
         // Create the command model and validate it.
-        var model = new CommandModel(configuration.Settings, defaultCommand, result, configuration.Examples);
+        var model = new CommandModel(configuration.Settings, result, configuration.Examples);
         CommandModelValidator.Validate(model, configuration.Settings);
 
         return model;
@@ -54,7 +55,7 @@ internal static class CommandModelBuilder
         foreach (var childCommand in command.Children)
         {
             var child = Build(info, childCommand);
-            info.Children.Add(child);
+            info.Children.Add(child);
         }
 
         // Normalize argument positions.

src/Spectre.Console.Cli/Internal/Modelling/CommandModelValidator.cs

@@ -14,7 +14,7 @@ internal static class CommandModelValidator
             throw new ArgumentNullException(nameof(settings));
         }
 
-        if (model.Commands.Count == 0 && model.DefaultCommand == null)
+        if (model.Commands.Count == 0)
         {
             throw CommandConfigurationException.NoCommandConfigured();
         }
@@ -31,7 +31,6 @@ internal static class CommandModelValidator
             }
         }
 
-        Validate(model.DefaultCommand);
         foreach (var command in model.Commands)
         {
             Validate(command);
@@ -147,7 +146,7 @@ internal static class CommandModelValidator
         {
             try
             {
-                var parser = new CommandTreeParser(model, settings, ParsingMode.Strict);
+                var parser = new CommandTreeParser(model, settings.CaseSensitivity, ParsingMode.Strict);
                 parser.Parse(example);
             }
             catch (Exception ex)

src/Spectre.Console.Cli/Internal/Modelling/ICommandContainer.cs

@@ -8,5 +8,13 @@ internal interface ICommandContainer
     /// <summary>
     /// Gets all commands in the container.
     /// </summary>
-    IList<CommandInfo> Commands { get; }
+    IList<CommandInfo> Commands { get; }
+
+    /// <summary>
+    /// Gets the default command for the container.
+    /// </summary>
+    /// <remarks>
+    /// Returns null if a default command has not been set.
+    /// </remarks>
+    CommandInfo? DefaultCommand { get; }
 }

src/Spectre.Console.Cli/Internal/Parsing/CommandTreeParser.cs

@@ -1,10 +1,13 @@
+using static Spectre.Console.Cli.CommandTreeTokenizer;
+
 namespace Spectre.Console.Cli;
 
 internal class CommandTreeParser
 {
     private readonly CommandModel _configuration;
     private readonly ParsingMode _parsingMode;
-    private readonly CommandOptionAttribute _help;
+    private readonly CommandOptionAttribute _help;
+    private readonly bool _convertFlagsToRemainingArguments;
 
     public CaseSensitivity CaseSensitivity { get; }
 
@@ -14,25 +17,26 @@ internal class CommandTreeParser
         Remaining = 1,
     }
 
-    public CommandTreeParser(CommandModel configuration, ICommandAppSettings settings, ParsingMode? parsingMode = null)
+    public CommandTreeParser(CommandModel configuration, CaseSensitivity caseSensitivity, ParsingMode? parsingMode = null, bool? convertFlagsToRemainingArguments = null)
     {
-        if (settings is null)
-        {
-            throw new ArgumentNullException(nameof(settings));
-        }
-
-        _configuration = configuration;
+        _configuration = configuration ?? throw new ArgumentNullException(nameof(configuration));
         _parsingMode = parsingMode ?? _configuration.ParsingMode;
-        _help = new CommandOptionAttribute("-h|--help");
-
-        CaseSensitivity = settings.CaseSensitivity;
-    }
+        _help = new CommandOptionAttribute("-h|--help");
+        _convertFlagsToRemainingArguments = convertFlagsToRemainingArguments ?? false;
 
+        CaseSensitivity = caseSensitivity;
+    }
+
     public CommandTreeParserResult Parse(IEnumerable<string> args)
+    {
+        var parserContext = new CommandTreeParserContext(args, _parsingMode);
+        var tokenizerResult = CommandTreeTokenizer.Tokenize(args);
+
+        return Parse(parserContext, tokenizerResult);
+    }
+
+    public CommandTreeParserResult Parse(CommandTreeParserContext context, CommandTreeTokenizerResult tokenizerResult)
     {
-        var context = new CommandTreeParserContext(args, _parsingMode);
-
-        var tokenizerResult = CommandTreeTokenizer.Tokenize(context.Arguments);
         var tokens = tokenizerResult.Tokens;
         var rawRemaining = tokenizerResult.Remaining;
 
@@ -254,10 +258,8 @@ internal class CommandTreeParser
             // Find the option.
             var option = node.FindOption(token.Value, isLongOption, CaseSensitivity);
             if (option != null)
-            {
-                node.Mapped.Add(new MappedCommandParameter(
-                    option, ParseOptionValue(context, stream, token, node, option)));
-
+            {
+                ParseOptionValue(context, stream, token, node, option);
                 return;
             }
 
@@ -271,7 +273,7 @@ internal class CommandTreeParser
 
         if (context.State == State.Remaining)
         {
-            ParseOptionValue(context, stream, token, node, null);
+            ParseOptionValue(context, stream, token, node);
             return;
         }
 
@@ -281,17 +283,19 @@ internal class CommandTreeParser
         }
         else
         {
-            ParseOptionValue(context, stream, token, node, null);
+            ParseOptionValue(context, stream, token, node);
         }
     }
 
-    private string? ParseOptionValue(
+    private void ParseOptionValue(
         CommandTreeParserContext context,
         CommandTreeTokenStream stream,
         CommandTreeToken token,
-        CommandTree current,
-        CommandParameter? parameter)
-    {
+        CommandTree current,
+        CommandParameter? parameter = null)
+    {
+        bool addToMappedCommandParameters = parameter != null;
+
         var value = default(string);
 
         // Parse the value of the token (if any).
@@ -325,7 +329,21 @@ internal class CommandTreeParser
                                 else
                                 {
                                     // Flags cannot be assigned a value.
-                                    throw CommandParseException.CannotAssignValueToFlag(context.Arguments, token);
+                                    if (_convertFlagsToRemainingArguments)
+                                    {
+                                        value = stream.Consume(CommandTreeToken.Kind.String)?.Value;
+
+                                        context.AddRemainingArgument(token.Value, value);
+
+                                        // Prevent the option and it's non-boolean value from being added to
+                                        // mapped parameters (otherwise an exception will be thrown later
+                                        // when binding the value to the flag in the comand settings)
+                                        addToMappedCommandParameters = false;
+                                    }
+                                    else
+                                    {
+                                        throw CommandParseException.CannotAssignValueToFlag(context.Arguments, token);
+                                    }
                                 }
                             }
                             else
@@ -352,13 +370,14 @@ internal class CommandTreeParser
                 }
             }
             else
-            {
+            {
                 context.AddRemainingArgument(token.Value, parseValue ? valueToken.Value : null);
             }
         }
         else
         {
-            if (context.State == State.Remaining || context.ParsingMode == ParsingMode.Relaxed)
+            if (parameter == null && // Only add tokens which have not been matched to a command parameter
+                (context.State == State.Remaining || context.ParsingMode == ParsingMode.Relaxed))
             {
                 context.AddRemainingArgument(token.Value, null);
             }
@@ -379,10 +398,12 @@ internal class CommandTreeParser
                     {
                         if (parameter.IsFlagValue())
                         {
-                            return null;
+                            value = null;
+                        }
+                        else
+                        {
+                            throw CommandParseException.OptionHasNoValue(context.Arguments, token, option);
                         }
-
-                        throw CommandParseException.OptionHasNoValue(context.Arguments, token, option);
                     }
                     else
                     {
@@ -394,6 +415,9 @@ internal class CommandTreeParser
             }
         }
 
-        return value;
+        if (parameter != null && addToMappedCommandParameters)
+        {
+            current.Mapped.Add(new MappedCommandParameter(parameter, value));
+        }
     }
 }

src/Spectre.Console.Cli/Internal/Parsing/CommandTreeParserContext.cs

@@ -26,19 +26,16 @@ internal class CommandTreeParserContext
     public void IncreaseArgumentPosition()
     {
         CurrentArgumentPosition++;
-    }
-
+    }
+
     public void AddRemainingArgument(string key, string? value)
-    {
-        if (State == CommandTreeParser.State.Remaining || ParsingMode == ParsingMode.Relaxed)
-        {
-            if (!_remaining.ContainsKey(key))
-            {
-                _remaining.Add(key, new List<string?>());
-            }
-
-            _remaining[key].Add(value);
-        }
+    {
+        if (!_remaining.ContainsKey(key))
+        {
+            _remaining.Add(key, new List<string?>());
+        }
+
+        _remaining[key].Add(value);
     }
 
     [SuppressMessage("Style", "IDE0004:Remove Unnecessary Cast", Justification = "Bug in analyzer?")]

src/Spectre.Console.Cli/Internal/Parsing/CommandTreeTokenStream.cs

@@ -5,7 +5,8 @@ internal sealed class CommandTreeTokenStream : IReadOnlyList<CommandTreeToken>
     private readonly List<CommandTreeToken> _tokens;
     private int _position;
 
-    public int Count => _tokens.Count;
+    public int Count => _tokens.Count;
+    public int Position => _position;
 
     public CommandTreeToken this[int index] => _tokens[index];