From 0d76a52a4af320294d791319899534740fb79f7e Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 21 Feb 2023 15:55:27 +0100 Subject: [PATCH 01/13] define Directive symbols --- src/System.CommandLine/Directive.cs | 46 +++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 src/System.CommandLine/Directive.cs diff --git a/src/System.CommandLine/Directive.cs b/src/System.CommandLine/Directive.cs new file mode 100644 index 0000000000..03fea3eaf6 --- /dev/null +++ b/src/System.CommandLine/Directive.cs @@ -0,0 +1,46 @@ +using System.Collections.Generic; +using System.CommandLine.Completions; + +namespace System.CommandLine +{ + /// + /// The purpose of directives is to provide cross-cutting functionality that can apply across command-line apps. + /// Because directives are syntactically distinct from the app's own syntax, they can provide functionality that applies across apps. + /// + /// A directive must conform to the following syntax rules: + /// * It's a token on the command line that comes after the app's name but before any subcommands or options. + /// * It's enclosed in square brackets. + /// * It doesn't contain spaces. + /// + public class Directive : Symbol + { + /// + /// Initializes a new instance of the Directive class. + /// + /// The name of the directive. It can't contain whitespaces. + /// The description of the directive, shown in help. + public Directive(string name, string? description) + { + if (string.IsNullOrWhiteSpace(name)) + { + throw new ArgumentException("Name cannot be null, empty, or consist entirely of whitespace."); + } + + for (var i = 0; i < name.Length; i++) + { + if (char.IsWhiteSpace(name[i])) + { + throw new ArgumentException($"Name cannot contain whitespace: \"{name}\"", nameof(name)); + } + } + + Name = name; + Description = description; + } + + private protected override string DefaultName => throw new NotImplementedException(); + + public override IEnumerable GetCompletions(CompletionContext context) + => Array.Empty(); + } +} From 8ce527191934a614d3974e5e23b34114f48ef9bf Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 21 Feb 2023 16:15:41 +0100 Subject: [PATCH 02/13] tokenization --- .../CommandLineConfiguration.cs | 2 ++ .../Parsing/StringExtensions.cs | 31 ++++++++++++++++--- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/System.CommandLine/CommandLineConfiguration.cs b/src/System.CommandLine/CommandLineConfiguration.cs index 3632e2855e..9f761a9245 100644 --- a/src/System.CommandLine/CommandLineConfiguration.cs +++ b/src/System.CommandLine/CommandLineConfiguration.cs @@ -51,6 +51,8 @@ public class CommandLineConfiguration internal readonly IReadOnlyList Middleware; + internal readonly IReadOnlyList? Directives; + private Func? _helpBuilderFactory; private TryReplaceToken? _tokenReplacer; diff --git a/src/System.CommandLine/Parsing/StringExtensions.cs b/src/System.CommandLine/Parsing/StringExtensions.cs index 78d3e85f95..859af79f52 100644 --- a/src/System.CommandLine/Parsing/StringExtensions.cs +++ b/src/System.CommandLine/Parsing/StringExtensions.cs @@ -86,7 +86,7 @@ internal static void Tokenize( var tokenList = new List(args.Count); - var knownTokens = configuration.RootCommand.ValidTokens(); + var knownTokens = configuration.RootCommand.ValidTokens(configuration); int i = FirstArgumentIsRootCommand(args, configuration.RootCommand, inferRootCommand) ? 0 @@ -121,7 +121,16 @@ internal static void Tokenize( arg[1] != ':' && arg[arg.Length - 1] == ']') { - tokenList.Add(Directive(arg)); + int colonIndex = arg.IndexOf(':', StringComparison.Ordinal); + string directiveName = colonIndex > 0 + ? arg.Substring(1, colonIndex) // [name:value] + : arg.Substring(1, arg.Length - 1); // [name] is a legal directive + + Directive? directive = knownTokens.TryGetValue(directiveName, out var directiveToken) + ? (Directive)directiveToken.Symbol! + : null; + + tokenList.Add(Directive(arg, directive)); continue; } @@ -175,7 +184,8 @@ configuration.TokenReplacer is { } replacer && { if (cmd != configuration.RootCommand) { - knownTokens = cmd.ValidTokens(); + knownTokens = cmd.ValidTokens( + configuration: null); // config contains Directives, they are allowed only for RootCommand } currentCommand = cmd; tokenList.Add(Command(arg, cmd)); @@ -219,7 +229,7 @@ configuration.TokenReplacer is { } replacer && Token DoubleDash() => new("--", TokenType.DoubleDash, default, i); - Token Directive(string value) => new(value, TokenType.Directive, default, i); + Token Directive(string value, Directive? directive) => new(value, TokenType.Directive, directive, i); } tokens = tokenList; @@ -422,10 +432,21 @@ static IEnumerable SplitLine(string line) } } - private static Dictionary ValidTokens(this Command command) + private static Dictionary ValidTokens(this Command command, CommandLineConfiguration? configuration) { Dictionary tokens = new(StringComparer.Ordinal); + if (configuration?.Directives is not null) + { + for (int directiveIndex = 0; directiveIndex < configuration.Directives.Count; directiveIndex++) + { + Directive directive = configuration.Directives[directiveIndex]; + tokens.Add( + directive.Name, + new Token(directive.Name, TokenType.Directive, directive, Token.ImplicitPosition)); + } + } + foreach (string commandAlias in command.Aliases) { tokens.Add( From 3a24a459771f7c4f99387d6f740a86bad8ff7dee Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 21 Feb 2023 16:44:38 +0100 Subject: [PATCH 03/13] parsing --- src/System.CommandLine/Directive.cs | 20 ++++++- .../EnvironmentVariablesDirective.cs | 30 +++++++++++ .../Invocation/SuggestDirectiveResult.cs | 26 ---------- ...seDirectiveResult.cs => ParseDirective.cs} | 14 ++--- src/System.CommandLine/ParseResult.cs | 28 +++++----- .../Parsing/DirectiveResult.cs | 32 ++++++++++++ .../Parsing/ParseOperation.cs | 52 ++++--------------- .../Parsing/StringExtensions.cs | 2 +- .../Parsing/SymbolResult.cs | 7 +++ .../Parsing/SymbolResultTree.cs | 3 ++ src/System.CommandLine/SuggestDirective.cs | 34 ++++++++++++ 11 files changed, 159 insertions(+), 89 deletions(-) create mode 100644 src/System.CommandLine/EnvironmentVariablesDirective.cs delete mode 100644 src/System.CommandLine/Invocation/SuggestDirectiveResult.cs rename src/System.CommandLine/{Invocation/ParseDirectiveResult.cs => ParseDirective.cs} (56%) create mode 100644 src/System.CommandLine/Parsing/DirectiveResult.cs create mode 100644 src/System.CommandLine/SuggestDirective.cs diff --git a/src/System.CommandLine/Directive.cs b/src/System.CommandLine/Directive.cs index 03fea3eaf6..7faf8d016d 100644 --- a/src/System.CommandLine/Directive.cs +++ b/src/System.CommandLine/Directive.cs @@ -1,5 +1,8 @@ using System.Collections.Generic; using System.CommandLine.Completions; +using System.CommandLine.Invocation; +using System.Threading.Tasks; +using System.Threading; namespace System.CommandLine { @@ -19,7 +22,12 @@ public class Directive : Symbol /// /// The name of the directive. It can't contain whitespaces. /// The description of the directive, shown in help. - public Directive(string name, string? description) + /// The synchronous action that is invoked when directive is parsed. + /// The asynchronous action that is invoked when directive is parsed. + public Directive(string name, + string? description = null, + Action? syncHandler = null, + Func? asyncHandler = null) { if (string.IsNullOrWhiteSpace(name)) { @@ -34,10 +42,20 @@ public Directive(string name, string? description) } } + if (syncHandler is null && asyncHandler is null) + { + throw new ArgumentNullException(message: "A single handler needs to be provided", innerException: null); + } + Name = name; Description = description; + Handler = syncHandler is not null + ? new AnonymousCommandHandler(syncHandler) + : new AnonymousCommandHandler(asyncHandler!); } + internal ICommandHandler Handler { get; } + private protected override string DefaultName => throw new NotImplementedException(); public override IEnumerable GetCompletions(CompletionContext context) diff --git a/src/System.CommandLine/EnvironmentVariablesDirective.cs b/src/System.CommandLine/EnvironmentVariablesDirective.cs new file mode 100644 index 0000000000..ef00b1fdd4 --- /dev/null +++ b/src/System.CommandLine/EnvironmentVariablesDirective.cs @@ -0,0 +1,30 @@ +using System.CommandLine.Invocation; + +namespace System.CommandLine +{ + public sealed class EnvironmentVariablesDirective : Directive + { + public EnvironmentVariablesDirective() : base("env", syncHandler: SyncHandler) + { + } + + private static void SyncHandler(InvocationContext context) + { + EnvironmentVariablesDirective symbol = (EnvironmentVariablesDirective)context.ParseResult.Symbol; + string? parsedValues = context.ParseResult.FindResultFor(symbol)!.Value; + + if (parsedValues is not null) + { + string[] components = parsedValues.Split(new[] { '=' }, count: 2); + string variable = components.Length > 0 ? components[0].Trim() : string.Empty; + if (string.IsNullOrEmpty(variable) || components.Length < 2) + { + return; + } + + string value = components[1].Trim(); + Environment.SetEnvironmentVariable(variable, value); + } + } + } +} diff --git a/src/System.CommandLine/Invocation/SuggestDirectiveResult.cs b/src/System.CommandLine/Invocation/SuggestDirectiveResult.cs deleted file mode 100644 index 5e55deb76b..0000000000 --- a/src/System.CommandLine/Invocation/SuggestDirectiveResult.cs +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System.CommandLine.IO; -using System.CommandLine.Parsing; -using System.Linq; - -namespace System.CommandLine.Invocation -{ - internal static class SuggestDirectiveResult - { - internal static void Apply(InvocationContext context, int position) - { - var commandLineToComplete = context.ParseResult.Tokens.LastOrDefault(t => t.Type != TokenType.Directive)?.Value ?? ""; - - var completionParseResult = context.Parser.Parse(commandLineToComplete); - - var completions = completionParseResult.GetCompletions(position); - - context.Console.Out.WriteLine( - string.Join( - Environment.NewLine, - completions)); - } - } -} diff --git a/src/System.CommandLine/Invocation/ParseDirectiveResult.cs b/src/System.CommandLine/ParseDirective.cs similarity index 56% rename from src/System.CommandLine/Invocation/ParseDirectiveResult.cs rename to src/System.CommandLine/ParseDirective.cs index 0f212d6812..75f577554f 100644 --- a/src/System.CommandLine/Invocation/ParseDirectiveResult.cs +++ b/src/System.CommandLine/ParseDirective.cs @@ -1,14 +1,16 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - +using System.CommandLine.Invocation; using System.CommandLine.IO; using System.CommandLine.Parsing; -namespace System.CommandLine.Invocation +namespace System.CommandLine { - internal static class ParseDirectiveResult + public sealed class ParseDirective : Directive { - internal static void Apply(InvocationContext context) + public ParseDirective() : base("parse", syncHandler: SyncHandler) + { + } + + private static void SyncHandler(InvocationContext context) { var parseResult = context.ParseResult; context.Console.Out.WriteLine(parseResult.Diagram()); diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 7c377a6d51..e5da8c18fd 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -17,7 +17,6 @@ public class ParseResult private readonly IReadOnlyList _errors; private readonly CommandResult _rootCommandResult; private readonly IReadOnlyList _unmatchedTokens; - private Dictionary>? _directives; private CompletionContext? _completionContext; private ICommandHandler? _handler; @@ -25,7 +24,6 @@ internal ParseResult( Parser parser, CommandResult rootCommandResult, CommandResult commandResult, - Dictionary>? directives, List tokens, IReadOnlyList? unmatchedTokens, List? errors, @@ -35,7 +33,6 @@ internal ParseResult( Parser = parser; _rootCommandResult = rootCommandResult; CommandResult = commandResult; - _directives = directives; _handler = handler; // skip the root command when populating Tokens property @@ -83,13 +80,18 @@ internal ParseResult( /// Gets the directives found while parsing command line input. /// /// If is set to , then this collection will be empty. - public IReadOnlyDictionary> Directives => _directives ??= new (); + public IReadOnlyDictionary> Directives => null!; /// /// Gets the tokens identified while parsing command line input. /// public IReadOnlyList Tokens { get; } + /// + /// Gets the parsed Symbol which provided Handler. Currently it can be only a Directive or a Command. + /// + internal Symbol Symbol { get; } + /// /// Holds the value of a complete command line input prior to splitting and tokenization, when provided. /// @@ -162,19 +164,20 @@ CommandLineText is null public OptionResult? FindResultFor(Option option) => _rootCommandResult.FindResultFor(option); + /// + /// Gets the result, if any, for the specified directive. + /// + /// The directive for which to find a result. + /// A result for the specified directive, or if it was not provided. + public DirectiveResult? FindResultFor(Directive directive) => _rootCommandResult.FindResultFor(directive); + /// /// Gets the result, if any, for the specified symbol. /// /// The symbol for which to find a result. /// A result for the specified symbol, or if it was not provided and no default was configured. - public SymbolResult? FindResultFor(Symbol symbol) => - symbol switch - { - Argument argument => FindResultFor(argument), - Command command => FindResultFor(command), - Option option => FindResultFor(option), - _ => throw new ArgumentOutOfRangeException(nameof(symbol)) - }; + public SymbolResult? FindResultFor(Symbol symbol) + => _rootCommandResult.SymbolResultTree.TryGetValue(symbol, out SymbolResult? result) ? result : null; /// /// Gets completions based on a given parse result. @@ -190,6 +193,7 @@ public IEnumerable GetCompletions( { ArgumentResult argumentResult => argumentResult.Argument, OptionResult optionResult => optionResult.Option, + DirectiveResult directiveResult => directiveResult.Directive, _ => ((CommandResult)currentSymbolResult).Command }; diff --git a/src/System.CommandLine/Parsing/DirectiveResult.cs b/src/System.CommandLine/Parsing/DirectiveResult.cs new file mode 100644 index 0000000000..bfa1b5ce3a --- /dev/null +++ b/src/System.CommandLine/Parsing/DirectiveResult.cs @@ -0,0 +1,32 @@ +namespace System.CommandLine.Parsing +{ + /// + /// A result produced when parsing an . + /// + public sealed class DirectiveResult : SymbolResult + { + internal DirectiveResult(Directive directive, Token token, string? value, SymbolResultTree symbolResultTree, SymbolResult rootCommand) + : base(symbolResultTree, rootCommand) + { + Directive = directive; + Token = token; + Value = value; + } + + /// + /// Parsed value of an [name:value] directive. + /// + /// Can be null for [name] directives. + public string? Value { get; } + + /// + /// The directive to which the result applies. + /// + public Directive Directive { get; } + + /// + /// The token that was parsed to specify the directive. + /// + public Token Token { get; } + } +} diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 00520d4a7e..a03afad08e 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -16,7 +16,6 @@ internal sealed class ParseOperation private readonly CommandResult _rootCommandResult; private int _index; - private Dictionary>? _directives; private CommandResult _innermostCommandResult; private bool _isHelpRequested, _isParseRequested; private ICommandHandler? _handler; @@ -78,7 +77,6 @@ internal ParseResult Parse(Parser parser) parser, _rootCommandResult, _innermostCommandResult, - _directives, _tokens, _symbolResultTree.UnmatchedTokens, _symbolResultTree.Errors, @@ -298,26 +296,24 @@ void ParseDirective() var token = CurrentToken; ReadOnlySpan withoutBrackets = token.Value.AsSpan(1, token.Value.Length - 2); int indexOfColon = withoutBrackets.IndexOf(':'); - string key = indexOfColon >= 0 - ? withoutBrackets.Slice(0, indexOfColon).ToString() - : withoutBrackets.ToString(); string? value = indexOfColon > 0 ? withoutBrackets.Slice(indexOfColon + 1).ToString() : null; - if (_directives is null || !_directives.TryGetValue(key, out var values)) + if (token.Symbol is not Directive directive) { - values = new List(); - - (_directives ??= new()).Add(key, values); + _symbolResultTree.AddUnmatchedToken(token, commandResult: null); + return; } - if (value is not null) + DirectiveResult result = new (directive, token, value, _symbolResultTree, _rootCommandResult); + _symbolResultTree.Add(directive, result); + _handler = directive.Handler; + + if (directive is ParseDirective) { - ((List)values).Add(value); + _isParseRequested = true; } - - OnDirectiveParsed(key, value); } } @@ -346,35 +342,5 @@ private void Validate() currentResult = currentResult.Parent as CommandResult; } } - - private void OnDirectiveParsed(string directiveKey, string? parsedValues) - { - if (_configuration.EnableEnvironmentVariableDirective && directiveKey == "env") - { - if (parsedValues is not null) - { - var components = parsedValues.Split(new[] { '=' }, count: 2); - var variable = components.Length > 0 ? components[0].Trim() : string.Empty; - if (string.IsNullOrEmpty(variable) || components.Length < 2) - { - return; - } - - var value = components[1].Trim(); - Environment.SetEnvironmentVariable(variable, value); - } - } - else if (_configuration.ParseDirectiveExitCode.HasValue && directiveKey == "parse") - { - _isParseRequested = true; - _handler = new AnonymousCommandHandler(ParseDirectiveResult.Apply); - } - else if (_configuration.EnableSuggestDirective && directiveKey == "suggest") - { - int position = parsedValues is not null ? int.Parse(parsedValues) : _rawInput?.Length ?? 0; - - _handler = new AnonymousCommandHandler(ctx => SuggestDirectiveResult.Apply(ctx, position)); - } - } } } \ No newline at end of file diff --git a/src/System.CommandLine/Parsing/StringExtensions.cs b/src/System.CommandLine/Parsing/StringExtensions.cs index 859af79f52..32acb6a0ad 100644 --- a/src/System.CommandLine/Parsing/StringExtensions.cs +++ b/src/System.CommandLine/Parsing/StringExtensions.cs @@ -121,7 +121,7 @@ internal static void Tokenize( arg[1] != ':' && arg[arg.Length - 1] == ']') { - int colonIndex = arg.IndexOf(':', StringComparison.Ordinal); + int colonIndex = arg.AsSpan().IndexOf(':'); string directiveName = colonIndex > 0 ? arg.Substring(1, colonIndex) // [name:value] : arg.Substring(1, arg.Length - 1); // [name] is a legal directive diff --git a/src/System.CommandLine/Parsing/SymbolResult.cs b/src/System.CommandLine/Parsing/SymbolResult.cs index 73f5b75b93..15f1b0dc47 100644 --- a/src/System.CommandLine/Parsing/SymbolResult.cs +++ b/src/System.CommandLine/Parsing/SymbolResult.cs @@ -65,6 +65,13 @@ private protected SymbolResult(SymbolResultTree symbolResultTree, SymbolResult? /// An option result if the option was matched by the parser or has a default value; otherwise, null. public OptionResult? FindResultFor(Option option) => SymbolResultTree.FindResultFor(option); + /// + /// Finds a result for the specific directive anywhere in the parse tree. + /// + /// The directive for which to find a result. + /// A directive result if the directive was matched by the parser, null otherwise. + public DirectiveResult? FindResultFor(Directive directive) => SymbolResultTree.FindResultFor(directive); + /// public T? GetValue(Argument argument) { diff --git a/src/System.CommandLine/Parsing/SymbolResultTree.cs b/src/System.CommandLine/Parsing/SymbolResultTree.cs index 9d152ec3e6..8642709fd8 100644 --- a/src/System.CommandLine/Parsing/SymbolResultTree.cs +++ b/src/System.CommandLine/Parsing/SymbolResultTree.cs @@ -37,6 +37,9 @@ internal SymbolResultTree(LocalizationResources localizationResources, List TryGetValue(option, out SymbolResult? result) ? (OptionResult)result : default; + internal DirectiveResult? FindResultFor(Directive directive) + => TryGetValue(directive, out SymbolResult? result) ? (DirectiveResult)result : default; + internal IEnumerable GetChildren(SymbolResult parent) { if (parent is not ArgumentResult) diff --git a/src/System.CommandLine/SuggestDirective.cs b/src/System.CommandLine/SuggestDirective.cs new file mode 100644 index 0000000000..6fe7e57f3b --- /dev/null +++ b/src/System.CommandLine/SuggestDirective.cs @@ -0,0 +1,34 @@ +using System.CommandLine.Invocation; +using System.Linq; +using System.CommandLine.IO; +using System.CommandLine.Parsing; + +namespace System.CommandLine +{ + public sealed class SuggestDirective : Directive + { + public SuggestDirective() : base("suggest", syncHandler: SyncHandler) + { + } + + private static void SyncHandler(InvocationContext context) + { + SuggestDirective symbol = (SuggestDirective)context.ParseResult.Symbol; + string? parsedValues = context.ParseResult.FindResultFor(symbol)!.Value; + string? rawInput = context.ParseResult.CommandLineText; + + int position = parsedValues is not null ? int.Parse(parsedValues) : rawInput?.Length ?? 0; + + var commandLineToComplete = context.ParseResult.Tokens.LastOrDefault(t => t.Type != TokenType.Directive)?.Value ?? ""; + + var completionParseResult = context.Parser.Parse(commandLineToComplete); + + var completions = completionParseResult.GetCompletions(position); + + context.Console.Out.WriteLine( + string.Join( + Environment.NewLine, + completions)); + } + } +} From ac851d30a2012df63e5891302ca44287d606a552 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 21 Feb 2023 18:13:51 +0100 Subject: [PATCH 04/13] configuration --- ...ommandLine_api_is_not_changed.approved.txt | 23 +++-- .../DirectiveTests.cs | 3 +- .../Builder/CommandLineBuilder.cs | 19 +---- .../Builder/CommandLineBuilderExtensions.cs | 38 ++------- .../CommandLineConfiguration.cs | 49 ++++------- src/System.CommandLine/DirectiveCollection.cs | 85 ------------------- .../EnvironmentVariablesDirective.cs | 3 + src/System.CommandLine/ParseDirective.cs | 15 +++- .../Parsing/ParseOperation.cs | 2 +- .../Parsing/StringExtensions.cs | 2 +- src/System.CommandLine/Parsing/TokenType.cs | 4 +- src/System.CommandLine/SuggestDirective.cs | 4 + 12 files changed, 66 insertions(+), 181 deletions(-) delete mode 100644 src/System.CommandLine/DirectiveCollection.cs diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt index 9a07c56d77..797c3d3b86 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt +++ b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt @@ -71,7 +71,6 @@ System.CommandLine public static CommandLineBuilder AddMiddleware(this CommandLineBuilder builder, System.CommandLine.Invocation.InvocationMiddleware middleware, System.CommandLine.Invocation.MiddlewareOrder order = Default) public static CommandLineBuilder AddMiddleware(this CommandLineBuilder builder, System.Action onInvoke, System.CommandLine.Invocation.MiddlewareOrder order = Default) public static CommandLineBuilder CancelOnProcessTermination(this CommandLineBuilder builder, System.Nullable timeout = null) - public static CommandLineBuilder EnableDirectives(this CommandLineBuilder builder, System.Boolean value = True) public static CommandLineBuilder EnablePosixBundling(this CommandLineBuilder builder, System.Boolean value = True) public static CommandLineBuilder RegisterWithDotnetSuggest(this CommandLineBuilder builder) public static CommandLineBuilder UseDefaults(this CommandLineBuilder builder) @@ -90,8 +89,8 @@ System.CommandLine public static CommandLineBuilder UseVersionOption(this CommandLineBuilder builder) public static CommandLineBuilder UseVersionOption(this CommandLineBuilder builder, System.String[] aliases) public class CommandLineConfiguration - .ctor(Command command, System.Boolean enablePosixBundling = True, System.Boolean enableDirectives = True, System.Boolean enableTokenReplacement = True, LocalizationResources resources = null, System.Collections.Generic.IReadOnlyList middlewarePipeline = null, System.Func helpBuilderFactory = null, System.CommandLine.Parsing.TryReplaceToken tokenReplacer = null) - public System.Boolean EnableDirectives { get; } + .ctor(Command command, System.Boolean enablePosixBundling = True, System.Boolean enableTokenReplacement = True, LocalizationResources resources = null, System.Collections.Generic.IReadOnlyList middlewarePipeline = null, System.Func helpBuilderFactory = null, System.CommandLine.Parsing.TryReplaceToken tokenReplacer = null) + public System.Collections.Generic.IReadOnlyList Directives { get; } public System.Boolean EnablePosixBundling { get; } public System.Boolean EnableTokenReplacement { get; } public LocalizationResources LocalizationResources { get; } @@ -107,11 +106,11 @@ System.CommandLine public static class ConsoleExtensions public static System.Void Write(this IConsole console, System.String value) public static System.Void WriteLine(this IConsole console, System.String value) - public class DirectiveCollection, System.Collections.Generic.IEnumerable>>, System.Collections.IEnumerable + public class Directive : Symbol + .ctor(System.String name, System.String description = null, System.Action syncHandler = null, System.Func asyncHandler = null) + public System.Collections.Generic.IEnumerable GetCompletions(System.CommandLine.Completions.CompletionContext context) + public class EnvironmentVariablesDirective : Directive .ctor() - public System.Boolean Contains(System.String name) - public System.Collections.Generic.IEnumerator>> GetEnumerator() - public System.Boolean TryGetValues(System.String name, ref System.Collections.Generic.IReadOnlyList values) public static class Handler public static System.Void SetHandler(this Command command, System.Action handle) public static System.Void SetHandler(this Command command, System.Action handle) @@ -210,6 +209,8 @@ System.CommandLine public static Option AcceptExistingOnly(this Option option) public static Option AcceptExistingOnly(this Option option) public static Option AcceptExistingOnly(this Option option) + public class ParseDirective : Directive + .ctor(System.Int32 errorExitCode = 1) public class ParseResult public System.CommandLine.Parsing.CommandResult CommandResult { get; } public System.Collections.Generic.IReadOnlyDictionary> Directives { get; } @@ -221,6 +222,7 @@ System.CommandLine public System.CommandLine.Parsing.ArgumentResult FindResultFor(Argument argument) public System.CommandLine.Parsing.CommandResult FindResultFor(Command command) public System.CommandLine.Parsing.OptionResult FindResultFor(Option option) + public System.CommandLine.Parsing.DirectiveResult FindResultFor(Directive directive) public System.CommandLine.Parsing.SymbolResult FindResultFor(Symbol symbol) public System.CommandLine.Completions.CompletionContext GetCompletionContext() public System.Collections.Generic.IEnumerable GetCompletions(System.Nullable position = null) @@ -231,6 +233,8 @@ System.CommandLine public static System.String ExecutableName { get; } public static System.String ExecutablePath { get; } .ctor(System.String description = ) + public class SuggestDirective : Directive + .ctor() public abstract class Symbol public System.String Description { get; set; } public System.Boolean IsHidden { get; set; } @@ -393,6 +397,10 @@ System.CommandLine.Parsing public System.Collections.Generic.IEnumerable Children { get; } public System.CommandLine.Command Command { get; } public Token Token { get; } + public class DirectiveResult : SymbolResult + public System.CommandLine.Directive Directive { get; } + public Token Token { get; } + public System.String Value { get; } public class OptionResult : SymbolResult public System.Boolean IsImplicit { get; } public System.CommandLine.Option Option { get; } @@ -427,6 +435,7 @@ System.CommandLine.Parsing public ArgumentResult FindResultFor(System.CommandLine.Argument argument) public CommandResult FindResultFor(System.CommandLine.Command command) public OptionResult FindResultFor(System.CommandLine.Option option) + public DirectiveResult FindResultFor(System.CommandLine.Directive directive) public T GetValue(Argument argument) public T GetValue(Option option) public System.String ToString() diff --git a/src/System.CommandLine.Tests/DirectiveTests.cs b/src/System.CommandLine.Tests/DirectiveTests.cs index 0a6b5e8bf5..7492a13173 100644 --- a/src/System.CommandLine.Tests/DirectiveTests.cs +++ b/src/System.CommandLine.Tests/DirectiveTests.cs @@ -146,8 +146,7 @@ public void Directives_can_be_disabled() new RootCommand { new Argument>() - }, - enableDirectives: false)); + })); var result = parser.Parse("[hello]"); diff --git a/src/System.CommandLine/Builder/CommandLineBuilder.cs b/src/System.CommandLine/Builder/CommandLineBuilder.cs index d45654bbc8..5b94f67bc8 100644 --- a/src/System.CommandLine/Builder/CommandLineBuilder.cs +++ b/src/System.CommandLine/Builder/CommandLineBuilder.cs @@ -23,18 +23,6 @@ public class CommandLineBuilder /// internal int? ParseErrorReportingExitCode; - /// - internal bool EnableDirectives = true; - - /// - internal bool EnableEnvironmentVariableDirective; - - /// - internal int? ParseDirectiveExitCode; - - /// - internal bool EnableSuggestDirective; - /// internal int MaxLevenshteinDistance; @@ -98,6 +86,8 @@ internal LocalizationResources LocalizationResources internal TryReplaceToken? TokenReplacer; + internal List? Directives; + /// /// Creates a parser based on the configuration of the command line builder. /// @@ -105,12 +95,9 @@ public Parser Build() => new( new CommandLineConfiguration( Command, + Directives, enablePosixBundling: EnablePosixBundling, - enableDirectives: EnableDirectives, enableTokenReplacement: EnableTokenReplacement, - enableEnvironmentVariableDirective: EnableEnvironmentVariableDirective, - parseDirectiveExitCode: ParseDirectiveExitCode, - enableSuggestDirective: EnableSuggestDirective, parseErrorReportingExitCode: ParseErrorReportingExitCode, maxLevenshteinDistance: MaxLevenshteinDistance, exceptionHandler: ExceptionHandler, diff --git a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs index dbb6d1f69a..0e1e6df0cd 100644 --- a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs +++ b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs @@ -9,8 +9,6 @@ using System.CommandLine.Parsing; using System.IO; using System.Threading; -using System.Threading.Tasks; -using static System.Environment; using Process = System.CommandLine.Invocation.Process; namespace System.CommandLine @@ -40,22 +38,6 @@ public static CommandLineBuilder CancelOnProcessTermination( return builder; } - /// - /// Enables the parser to recognize command line directives. - /// - /// A command line builder. - /// to enable directives. to parse directive-like tokens in the same way as any other token. - /// The same instance of . - /// Command-line directives - /// - public static CommandLineBuilder EnableDirectives( - this CommandLineBuilder builder, - bool value = true) - { - builder.EnableDirectives = value; - return builder; - } - /// /// Enables the parser to recognize and expand POSIX-style bundled options. /// @@ -141,15 +123,13 @@ await feature.EnsureRegistered(async () => return builder; } - /// - /// Enables the use of the [env:key=value] directive, allowing environment variables to be set from the command line during invocation. - /// + /// /// A command line builder. /// The same instance of . public static CommandLineBuilder UseEnvironmentVariableDirective( this CommandLineBuilder builder) { - builder.EnableEnvironmentVariableDirective = true; + (builder.Directives ??= new()).Add(new EnvironmentVariablesDirective()); return builder; } @@ -348,9 +328,8 @@ public static CommandLineBuilder AddMiddleware( return builder; } - /// - /// Enables the use of the [parse] directive, which when specified on the command line will short circuit normal command handling and display a diagram explaining the parse result for the command line input. - /// + + /// /// A command line builder. /// If the parse result contains errors, this exit code will be used when the process exits. /// The same instance of . @@ -358,7 +337,7 @@ public static CommandLineBuilder UseParseDirective( this CommandLineBuilder builder, int errorExitCode = 1) { - builder.ParseDirectiveExitCode = errorExitCode; + (builder.Directives ??= new()).Add(new ParseDirective(errorExitCode)); return builder; } @@ -378,16 +357,13 @@ public static CommandLineBuilder UseParseErrorReporting( return builder; } - /// - /// Enables the use of the [suggest] directive which when specified in command line input short circuits normal command handling and writes a newline-delimited list of suggestions suitable for use by most shells to provide command line completions. - /// - /// The dotnet-suggest tool requires the suggest directive to be enabled for an application to provide completions. + /// /// A command line builder. /// The same instance of . public static CommandLineBuilder UseSuggestDirective( this CommandLineBuilder builder) { - builder.EnableSuggestDirective = true; + (builder.Directives ??= new()).Add(new SuggestDirective()); return builder; } diff --git a/src/System.CommandLine/CommandLineConfiguration.cs b/src/System.CommandLine/CommandLineConfiguration.cs index 9f761a9245..6935c1b87d 100644 --- a/src/System.CommandLine/CommandLineConfiguration.cs +++ b/src/System.CommandLine/CommandLineConfiguration.cs @@ -21,22 +21,6 @@ public class CommandLineConfiguration /// internal readonly Action? ExceptionHandler; - /// - /// Enables the use of the [env:key=value] directive, allowing environment variables to be set from the command line during invocation. - /// - internal readonly bool EnableEnvironmentVariableDirective; - - /// - /// If the parse result contains errors, this exit code will be used when the process exits. - /// - internal readonly int? ParseDirectiveExitCode; - - /// - /// Enables the use of the [suggest] directive which when specified in command line input short circuits normal command handling and writes a newline-delimited list of suggestions suitable for use by most shells to provide command line completions. - /// - /// The dotnet-suggest tool requires the suggest directive to be enabled for an application to provide completions. - internal readonly bool EnableSuggestDirective; - /// /// The exit code to use when parser errors occur. /// @@ -51,8 +35,6 @@ public class CommandLineConfiguration internal readonly IReadOnlyList Middleware; - internal readonly IReadOnlyList? Directives; - private Func? _helpBuilderFactory; private TryReplaceToken? _tokenReplacer; @@ -61,7 +43,6 @@ public class CommandLineConfiguration /// /// The root command for the parser. /// to enable POSIX bundling; otherwise, . - /// to enable directive parsing; otherwise, . /// to enable token replacement; otherwise, . /// Provide custom validation messages. /// Provide a custom middleware pipeline. @@ -70,25 +51,32 @@ public class CommandLineConfiguration public CommandLineConfiguration( Command command, bool enablePosixBundling = true, - bool enableDirectives = true, bool enableTokenReplacement = true, LocalizationResources? resources = null, IReadOnlyList? middlewarePipeline = null, Func? helpBuilderFactory = null, TryReplaceToken? tokenReplacer = null) - : this(command, enablePosixBundling, enableDirectives, enableTokenReplacement, false, null, false, null, 0, null, - resources, middlewarePipeline, helpBuilderFactory, tokenReplacer, null) + : this( + command, + directives: null, + enablePosixBundling: enablePosixBundling, + enableTokenReplacement: enableTokenReplacement, + parseErrorReportingExitCode: null, + maxLevenshteinDistance: 0, + processTerminationTimeout: null, + resources: resources, + middlewarePipeline: middlewarePipeline, + helpBuilderFactory: helpBuilderFactory, + tokenReplacer: tokenReplacer, + exceptionHandler: null) { } internal CommandLineConfiguration( Command command, + List? directives, bool enablePosixBundling, - bool enableDirectives, bool enableTokenReplacement, - bool enableEnvironmentVariableDirective, - int? parseDirectiveExitCode, - bool enableSuggestDirective, int? parseErrorReportingExitCode, int maxLevenshteinDistance, TimeSpan? processTerminationTimeout, @@ -99,12 +87,9 @@ internal CommandLineConfiguration( Action? exceptionHandler) { RootCommand = command ?? throw new ArgumentNullException(nameof(command)); + Directives = directives is not null ? directives : Array.Empty(); EnableTokenReplacement = enableTokenReplacement; EnablePosixBundling = enablePosixBundling; - EnableDirectives = enableDirectives || enableEnvironmentVariableDirective || parseDirectiveExitCode.HasValue || enableSuggestDirective; - EnableEnvironmentVariableDirective = enableEnvironmentVariableDirective; - ParseDirectiveExitCode = parseDirectiveExitCode; - EnableSuggestDirective = enableSuggestDirective; ParseErrorReportingExitCode = parseErrorReportingExitCode; MaxLevenshteinDistance = maxLevenshteinDistance; ProcessTerminationTimeout = processTerminationTimeout; @@ -128,9 +113,9 @@ internal static HelpBuilder DefaultHelpBuilderFactory(BindingContext context, in } /// - /// Gets whether directives are enabled. + /// Gets the enabled directives. /// - public bool EnableDirectives { get; } + public IReadOnlyList Directives { get; } /// /// Gets a value indicating whether POSIX bundling is enabled. diff --git a/src/System.CommandLine/DirectiveCollection.cs b/src/System.CommandLine/DirectiveCollection.cs deleted file mode 100644 index 45a469d360..0000000000 --- a/src/System.CommandLine/DirectiveCollection.cs +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System.Collections; -using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; -using System.Linq; - -namespace System.CommandLine -{ - /// - /// A collection of directives parsed from a command line. - /// - /// A directive is specified on the command line using square brackets, containing no spaces and preceding other tokens unless they are also directives. In the following example, two directives are present, directive-one and directive-two: - /// > myapp [directive-one] [directive-two:value] arg1 arg2 - /// The second has a value specified as well, value. Directive values can be read by calling using . - /// - public class DirectiveCollection : IEnumerable>> - { - private Dictionary>? _directives; - - internal void Add(string name, string? value) - { - _directives ??= new(); - - if (!_directives.TryGetValue(name, out var values)) - { - values = new List(); - - _directives.Add(name, values); - } - - if (value is not null) - { - values.Add(value); - } - } - - /// - /// Gets a value determining whether a directive with the specified name was parsed. - /// - /// The name of the directive. - /// if a directive with the specified name was parsed; otherwise, . - public bool Contains(string name) - { - return _directives is not null && _directives.ContainsKey(name); - } - - /// - /// Gets the values specified for a given directive. A return value indicates whether the specified directive name was present. - /// - /// The name of the directive. - /// The values provided for the specified directive. - /// if a directive with the specified name was parsed; otherwise, . - public bool TryGetValues(string name, [NotNullWhen(true)] out IReadOnlyList? values) - { - if (_directives is not null && - _directives.TryGetValue(name, out var v)) - { - values = v; - return true; - } - else - { - values = null; - return false; - } - } - - /// - public IEnumerator>> GetEnumerator() - { - if (_directives is null) - { - return Enumerable.Empty>>().GetEnumerator(); - } - - return _directives - .Select(pair => new KeyValuePair>(pair.Key, pair.Value)) - .GetEnumerator(); - } - - IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); - } -} diff --git a/src/System.CommandLine/EnvironmentVariablesDirective.cs b/src/System.CommandLine/EnvironmentVariablesDirective.cs index ef00b1fdd4..147ad9b67b 100644 --- a/src/System.CommandLine/EnvironmentVariablesDirective.cs +++ b/src/System.CommandLine/EnvironmentVariablesDirective.cs @@ -2,6 +2,9 @@ namespace System.CommandLine { + /// + /// Enables the use of the [env:key=value] directive, allowing environment variables to be set from the command line during invocation. + /// public sealed class EnvironmentVariablesDirective : Directive { public EnvironmentVariablesDirective() : base("env", syncHandler: SyncHandler) diff --git a/src/System.CommandLine/ParseDirective.cs b/src/System.CommandLine/ParseDirective.cs index 75f577554f..6bdbc99652 100644 --- a/src/System.CommandLine/ParseDirective.cs +++ b/src/System.CommandLine/ParseDirective.cs @@ -4,19 +4,26 @@ namespace System.CommandLine { + /// + /// Enables the use of the [parse] directive, which when specified on the command line will short circuit normal command handling and display a diagram explaining the parse result for the command line input. + /// public sealed class ParseDirective : Directive { - public ParseDirective() : base("parse", syncHandler: SyncHandler) + /// If the parse result contains errors, this exit code will be used when the process exits. + public ParseDirective(int errorExitCode = 1) : base("parse", syncHandler: SyncHandler) { + ErrorExitCode = errorExitCode; } + internal int ErrorExitCode { get; } + private static void SyncHandler(InvocationContext context) { + ParseDirective symbol = (ParseDirective)context.ParseResult.Symbol; + var parseResult = context.ParseResult; context.Console.Out.WriteLine(parseResult.Diagram()); - context.ExitCode = parseResult.Errors.Count == 0 - ? 0 - : context.Parser.Configuration.ParseDirectiveExitCode!.Value; + context.ExitCode = parseResult.Errors.Count == 0 ? 0 : symbol.ErrorExitCode; } } } diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index a03afad08e..be2fbe453a 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -283,7 +283,7 @@ private void ParseDirectives() { while (More(out TokenType currentTokenType) && currentTokenType == TokenType.Directive) { - if (_configuration.EnableDirectives) + if (_configuration.Directives.Count > 0) { ParseDirective(); // kept in separate method to avoid JIT } diff --git a/src/System.CommandLine/Parsing/StringExtensions.cs b/src/System.CommandLine/Parsing/StringExtensions.cs index 32acb6a0ad..664b32f102 100644 --- a/src/System.CommandLine/Parsing/StringExtensions.cs +++ b/src/System.CommandLine/Parsing/StringExtensions.cs @@ -82,7 +82,7 @@ internal static void Tokenize( var currentCommand = configuration.RootCommand; var foundDoubleDash = false; - var foundEndOfDirectives = !configuration.EnableDirectives; + var foundEndOfDirectives = configuration.Directives.Count == 0; var tokenList = new List(args.Count); diff --git a/src/System.CommandLine/Parsing/TokenType.cs b/src/System.CommandLine/Parsing/TokenType.cs index 679ba9b799..0aae333afa 100644 --- a/src/System.CommandLine/Parsing/TokenType.cs +++ b/src/System.CommandLine/Parsing/TokenType.cs @@ -30,11 +30,11 @@ public enum TokenType /// A double dash (--) token, which changes the meaning of subsequent tokens. /// DoubleDash, - + /// /// A directive token. /// - /// + /// Directive } } diff --git a/src/System.CommandLine/SuggestDirective.cs b/src/System.CommandLine/SuggestDirective.cs index 6fe7e57f3b..d302f0851b 100644 --- a/src/System.CommandLine/SuggestDirective.cs +++ b/src/System.CommandLine/SuggestDirective.cs @@ -5,6 +5,10 @@ namespace System.CommandLine { + /// + /// Enables the use of the [suggest] directive which when specified in command line input short circuits normal command handling and writes a newline-delimited list of suggestions suitable for use by most shells to provide command line completions. + /// + /// The dotnet-suggest tool requires the suggest directive to be enabled for an application to provide completions. public sealed class SuggestDirective : Directive { public SuggestDirective() : base("suggest", syncHandler: SyncHandler) From d39fe8f6c1c27d612bfe11d4febba843e183bb84 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 21 Feb 2023 18:41:24 +0100 Subject: [PATCH 05/13] making it actually work and pass most of the tests --- samples/RenderingPlayground/Program.cs | 2 +- ...ne_Hosting_api_is_not_changed.approved.txt | 2 +- ...ommandLine_api_is_not_changed.approved.txt | 4 +- .../CommandLine/Perf_Parser_ParseResult.cs | 4 +- .../DirectiveConfigurationExtensions.cs | 11 +- .../HostingExtensions.cs | 13 +- .../CommandLineBuilderExtensions.cs | 36 +++--- .../DirectiveTests.cs | 114 +++++++++--------- .../Builder/CommandLineBuilder.cs | 7 +- .../Builder/CommandLineBuilderExtensions.cs | 6 +- src/System.CommandLine/Directive.cs | 29 +++-- .../EnvironmentVariablesDirective.cs | 29 +++-- src/System.CommandLine/ParseResult.cs | 8 +- .../Parsing/DirectiveResult.cs | 8 +- .../Parsing/ParseOperation.cs | 19 +-- .../Parsing/ParseResultExtensions.cs | 3 +- .../Parsing/StringExtensions.cs | 4 +- .../Parsing/SymbolResultExtensions.cs | 1 + src/System.CommandLine/SuggestDirective.cs | 2 +- 19 files changed, 160 insertions(+), 142 deletions(-) diff --git a/samples/RenderingPlayground/Program.cs b/samples/RenderingPlayground/Program.cs index a9593a03d0..f957db3084 100644 --- a/samples/RenderingPlayground/Program.cs +++ b/samples/RenderingPlayground/Program.cs @@ -54,7 +54,7 @@ public static void Main( var consoleRenderer = new ConsoleRenderer( console, - mode: invocationContext.BindingContext.OutputMode(), + mode: OutputMode.Auto, resetAfterRender: true); switch (sample) diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_Hosting_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_Hosting_api_is_not_changed.approved.txt index fb206743ff..4772b4bfa0 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_Hosting_api_is_not_changed.approved.txt +++ b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_Hosting_api_is_not_changed.approved.txt @@ -1,6 +1,6 @@ System.CommandLine.Hosting public static class DirectiveConfigurationExtensions - public static Microsoft.Extensions.Configuration.IConfigurationBuilder AddCommandLineDirectives(this Microsoft.Extensions.Configuration.IConfigurationBuilder config, System.CommandLine.ParseResult commandline, System.String name) + public static Microsoft.Extensions.Configuration.IConfigurationBuilder AddCommandLineDirectives(this Microsoft.Extensions.Configuration.IConfigurationBuilder config, System.CommandLine.ParseResult commandline, System.CommandLine.Directive directive) public static class HostingExtensions public static OptionsBuilder BindCommandLine(this OptionsBuilder optionsBuilder) public static Microsoft.Extensions.Hosting.IHost GetHost(this System.CommandLine.Invocation.InvocationContext invocationContext) diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt index 797c3d3b86..f5181d1a52 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt +++ b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt @@ -66,6 +66,7 @@ System.CommandLine public class CommandLineBuilder .ctor(Command rootCommand = null) public Command Command { get; } + public System.Collections.Generic.List Directives { get; } public System.CommandLine.Parsing.Parser Build() public static class CommandLineBuilderExtensions public static CommandLineBuilder AddMiddleware(this CommandLineBuilder builder, System.CommandLine.Invocation.InvocationMiddleware middleware, System.CommandLine.Invocation.MiddlewareOrder order = Default) @@ -109,8 +110,10 @@ System.CommandLine public class Directive : Symbol .ctor(System.String name, System.String description = null, System.Action syncHandler = null, System.Func asyncHandler = null) public System.Collections.Generic.IEnumerable GetCompletions(System.CommandLine.Completions.CompletionContext context) + public System.Void OnParsed(System.CommandLine.Parsing.DirectiveResult directiveResult) public class EnvironmentVariablesDirective : Directive .ctor() + public System.Void OnParsed(System.CommandLine.Parsing.DirectiveResult directiveResult) public static class Handler public static System.Void SetHandler(this Command command, System.Action handle) public static System.Void SetHandler(this Command command, System.Action handle) @@ -213,7 +216,6 @@ System.CommandLine .ctor(System.Int32 errorExitCode = 1) public class ParseResult public System.CommandLine.Parsing.CommandResult CommandResult { get; } - public System.Collections.Generic.IReadOnlyDictionary> Directives { get; } public System.Collections.Generic.IReadOnlyList Errors { get; } public System.CommandLine.Parsing.Parser Parser { get; } public System.CommandLine.Parsing.CommandResult RootCommandResult { get; } diff --git a/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs b/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs index be4be31cbe..bb492ff979 100644 --- a/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs +++ b/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs @@ -43,8 +43,8 @@ public IEnumerable GenerateTestParseResults() [Benchmark] [ArgumentsSource(nameof(GenerateTestInputs))] - public IReadOnlyDictionary> ParseResult_Directives(string input) - => _testParser.Parse(input).Directives; + public ParseResult ParseResult_Directives(string input) + => _testParser.Parse(input); [Benchmark] [ArgumentsSource(nameof(GenerateTestParseResults))] diff --git a/src/System.CommandLine.Hosting/DirectiveConfigurationExtensions.cs b/src/System.CommandLine.Hosting/DirectiveConfigurationExtensions.cs index a09f629f24..ca300e77b6 100644 --- a/src/System.CommandLine.Hosting/DirectiveConfigurationExtensions.cs +++ b/src/System.CommandLine.Hosting/DirectiveConfigurationExtensions.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.CommandLine.Parsing; using System.Linq; using Microsoft.Extensions.Configuration; @@ -9,18 +10,18 @@ public static class DirectiveConfigurationExtensions { public static IConfigurationBuilder AddCommandLineDirectives( this IConfigurationBuilder config, ParseResult commandline, - string name) + Directive directive) { if (commandline is null) throw new ArgumentNullException(nameof(commandline)); - if (name is null) - throw new ArgumentNullException(nameof(name)); + if (directive is null) + throw new ArgumentNullException(nameof(directive)); - if (!commandline.Directives.TryGetValue(name, out var directives)) + if (commandline.FindResultFor(directive) is not DirectiveResult result) return config; var kvpSeparator = new[] { '=' }; - return config.AddInMemoryCollection(directives.Select(s => + return config.AddInMemoryCollection(new string[] { result.Value }.Select(s => { var parts = s.Split(kvpSeparator, count: 2); var key = parts[0]; diff --git a/src/System.CommandLine.Hosting/HostingExtensions.cs b/src/System.CommandLine.Hosting/HostingExtensions.cs index 1a4eb4637a..93573e7332 100644 --- a/src/System.CommandLine.Hosting/HostingExtensions.cs +++ b/src/System.CommandLine.Hosting/HostingExtensions.cs @@ -13,12 +13,14 @@ namespace System.CommandLine.Hosting { public static class HostingExtensions { - private const string ConfigurationDirectiveName = "config"; - public static CommandLineBuilder UseHost(this CommandLineBuilder builder, Func hostBuilderFactory, - Action configureHost = null) => - builder.AddMiddleware(async (invocation, cancellationToken, next) => + Action configureHost = null) + { + Directive configurationDirective = new("config"); + builder.Directives.Add(configurationDirective); + + return builder.AddMiddleware(async (invocation, cancellationToken, next) => { var argsRemaining = invocation.ParseResult.UnmatchedTokens.ToArray(); var hostBuilder = hostBuilderFactory?.Invoke(argsRemaining) @@ -27,7 +29,7 @@ public static CommandLineBuilder UseHost(this CommandLineBuilder builder, hostBuilder.ConfigureHostConfiguration(config => { - config.AddCommandLineDirectives(invocation.ParseResult, ConfigurationDirectiveName); + config.AddCommandLineDirectives(invocation.ParseResult, configurationDirective); }); hostBuilder.ConfigureServices(services => { @@ -50,6 +52,7 @@ public static CommandLineBuilder UseHost(this CommandLineBuilder builder, await host.StopAsync(cancellationToken); }); + } public static CommandLineBuilder UseHost(this CommandLineBuilder builder, Action configureHost = null diff --git a/src/System.CommandLine.Rendering/CommandLineBuilderExtensions.cs b/src/System.CommandLine.Rendering/CommandLineBuilderExtensions.cs index 185d24c9f8..5f49ed006a 100644 --- a/src/System.CommandLine.Rendering/CommandLineBuilderExtensions.cs +++ b/src/System.CommandLine.Rendering/CommandLineBuilderExtensions.cs @@ -1,8 +1,7 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -using System.CommandLine.Binding; -using System.Linq; +using System.CommandLine.Parsing; namespace System.CommandLine.Rendering { @@ -11,13 +10,18 @@ public static class CommandLineBuilderExtensions public static CommandLineBuilder UseAnsiTerminalWhenAvailable( this CommandLineBuilder builder) { + Directive enableVtDirective = new ("enable-vt"); + Directive outputDirective = new ("output"); + builder.Directives.Add(enableVtDirective); + builder.Directives.Add(outputDirective); + builder.AddMiddleware(context => { var console = context.Console; var terminal = console.GetTerminal( - PreferVirtualTerminal(context.BindingContext), - OutputMode(context.BindingContext)); + PreferVirtualTerminal(context.ParseResult, enableVtDirective), + OutputMode(context.ParseResult, outputDirective)); context.Console = terminal ?? console; }); @@ -25,16 +29,13 @@ public static CommandLineBuilder UseAnsiTerminalWhenAvailable( return builder; } - internal static bool PreferVirtualTerminal( - this BindingContext context) + private static bool PreferVirtualTerminal(ParseResult parseResult, Directive enableVtDirective) { - if (context.ParseResult.Directives.TryGetValue( - "enable-vt", - out var trueOrFalse)) + if (parseResult.FindResultFor(enableVtDirective) is DirectiveResult result) { - if (bool.TryParse( - trueOrFalse.FirstOrDefault(), - out var pvt)) + string trueOrFalse = result.Value; + + if (bool.TryParse(trueOrFalse, out var pvt)) { return pvt; } @@ -43,15 +44,10 @@ internal static bool PreferVirtualTerminal( return true; } - public static OutputMode OutputMode(this BindingContext context) + private static OutputMode OutputMode(ParseResult parseResult, Directive outputDirective) { - if (context.ParseResult.Directives.TryGetValue( - "output", - out var modeString) && - Enum.TryParse( - modeString.FirstOrDefault(), - true, - out var mode)) + if (parseResult.FindResultFor(outputDirective) is DirectiveResult result + && Enum.TryParse(result.Value, true, out var mode)) { return mode; } diff --git a/src/System.CommandLine.Tests/DirectiveTests.cs b/src/System.CommandLine.Tests/DirectiveTests.cs index 7492a13173..4264e85dbb 100644 --- a/src/System.CommandLine.Tests/DirectiveTests.cs +++ b/src/System.CommandLine.Tests/DirectiveTests.cs @@ -12,11 +12,13 @@ namespace System.CommandLine.Tests public class DirectiveTests { [Fact] - public void Directives_should_not_be_considered_as_unmatched_tokens() + public void Directives_should_not_be_considered_as_unmatched_tokens_when_they_are_enabled() { - var option = new Option("-y"); + RootCommand root = new () { new Option("-y") }; + CommandLineBuilder builder = new (root); + builder.Directives.Add(new ("some")); - var result = option.Parse($"{RootCommand.ExecutableName} [parse] -y"); + var result = builder.Build().Parse($"{RootCommand.ExecutableName} [parse] -y"); result.UnmatchedTokens.Should().BeEmpty(); } @@ -24,43 +26,38 @@ public void Directives_should_not_be_considered_as_unmatched_tokens() [Fact] public void Raw_tokens_still_hold_directives() { - var option = new Option("-y"); + Directive directive = new ("parse"); - var result = option.Parse("[parse] -y"); + ParseResult result = Parse(new Option("-y"), directive, "[parse] -y"); - result.Directives.ContainsKey("parse").Should().BeTrue(); + result.FindResultFor(directive).Should().NotBeNull(); result.Tokens.Should().Contain(t => t.Value == "[parse]"); } - [Fact] - public void Directives_should_parse_into_the_directives_collection() - { - var option = new Option("-y"); - - var result = option.Parse("[parse] -y"); - - result.Directives.ContainsKey("parse").Should().BeTrue(); - } - [Fact] public void Multiple_directives_are_allowed() { - var option = new Option("-y"); + RootCommand root = new() { new Option("-y") }; + Directive parseDirective = new ("parse"); + Directive suggestDirective = new ("suggest"); + CommandLineBuilder builder = new(root); + builder.Directives.Add(parseDirective); + builder.Directives.Add(suggestDirective); - var result = option.Parse("[parse] [suggest] -y"); + var result = builder.Build().Parse("[parse] [suggest] -y"); - result.Directives.ContainsKey("parse").Should().BeTrue(); - result.Directives.ContainsKey("suggest").Should().BeTrue(); + result.FindResultFor(parseDirective).Should().NotBeNull(); + result.FindResultFor(suggestDirective).Should().NotBeNull(); } [Fact] public void Directives_must_be_the_first_argument() { - var option = new Option("-y"); + Directive directive = new("parse"); - var result = option.Parse("-y [suggest]"); + ParseResult result = Parse(new Option("-y"), directive, "-y [parse]"); - result.Directives.Should().BeEmpty(); + result.FindResultFor(directive).Should().BeNull(); } [Theory] @@ -68,27 +65,25 @@ public void Directives_must_be_the_first_argument() [InlineData("[key:value:more]", "key", "value:more")] [InlineData("[key:]", "key", "")] public void Directives_can_have_a_value_which_is_everything_after_the_first_colon( - string directive, - string expectedKey, + string wholeText, + string key, string expectedValue) { - var option = new Option("-y"); + Directive directive = new(key); - var result = option.Parse($"{directive} -y"); + ParseResult result = Parse(new Option("-y"), directive, $"{wholeText} -y"); - result.Directives.TryGetValue(expectedKey, out var values).Should().BeTrue(); - values.Should().BeEquivalentTo(expectedValue); + result.FindResultFor(directive).Value.Should().Be(expectedValue); } [Fact] public void Directives_without_a_value_specified_have_a_value_of_empty_string() { - var option = new Option("-y"); + Directive directive = new("parse"); - var result = option.Parse("[parse] -y"); + ParseResult result = Parse(new Option("-y"), directive, "[parse] -y"); - result.Directives.TryGetValue("parse", out var values).Should().BeTrue(); - values.Should().BeEmpty(); + result.FindResultFor(directive).Value.Should().BeEmpty(); } [Theory] @@ -100,7 +95,6 @@ public void Directives_must_have_a_non_empty_key(string directive) var result = option.Parse($"{directive} -a"); - result.Directives.Should().BeEmpty(); result.UnmatchedTokens.Should().Contain(directive); } @@ -108,38 +102,36 @@ public void Directives_must_have_a_non_empty_key(string directive) [InlineData("[par se]")] [InlineData("[ parse]")] [InlineData("[parse ]")] - public void Directives_cannot_contain_spaces(object value) + public void Directives_cannot_contain_spaces(string value) { - var option = new Option("-a"); - - var result = option.Parse($"{value} -a"); + Action create = () => new Directive(value); - result.Directives.Should().BeEmpty(); + create.Should().Throw(); } - [Fact] - public void When_a_directive_is_specified_more_than_once_then_its_values_are_aggregated() - { - var option = new Option("-a"); + //[Fact] + //public void When_a_directive_is_specified_more_than_once_then_its_values_are_aggregated() + //{ + // var option = new Option("-a"); - var result = option.Parse("[directive:one] [directive:two] -a"); + // var result = option.Parse("[directive:one] [directive:two] -a"); - result.Directives.TryGetValue("directive", out var values).Should().BeTrue(); - values.Should().BeEquivalentTo("one", "two"); - } + // result.Directives.TryGetValue("directive", out var values).Should().BeTrue(); + // values.Should().BeEquivalentTo("one", "two"); + //} - [Fact] - public void Directive_count_is_based_on_distinct_instances_of_directive_name() - { - var command = new RootCommand(); + //[Fact] + //public void Directive_count_is_based_on_distinct_instances_of_directive_name() + //{ + // var command = new RootCommand(); - var result = command.Parse("[one] [two] [one:a] [one:b]"); + // var result = command.Parse("[one] [two] [one:a] [one:b]"); - result.Directives.Should().HaveCount(2); - } + // result.Directives.Should().HaveCount(2); + //} [Fact] - public void Directives_can_be_disabled() + public void When_directives_are_not_enabled_they_are_treated_as_regular_tokens() { var parser = new Parser( new CommandLineConfiguration( @@ -150,12 +142,20 @@ public void Directives_can_be_disabled() var result = parser.Parse("[hello]"); - result.Directives.Count().Should().Be(0); result.CommandResult .Tokens .Select(t => t.Value) .Should() .BeEquivalentTo("[hello]"); } + + private static ParseResult Parse(Option option, Directive directive, string commandLine) + { + RootCommand root = new() { option }; + CommandLineBuilder builder = new(root); + builder.Directives.Add(directive); + + return builder.Build().Parse(commandLine); + } } -} +} \ No newline at end of file diff --git a/src/System.CommandLine/Builder/CommandLineBuilder.cs b/src/System.CommandLine/Builder/CommandLineBuilder.cs index 5b94f67bc8..907cad10ff 100644 --- a/src/System.CommandLine/Builder/CommandLineBuilder.cs +++ b/src/System.CommandLine/Builder/CommandLineBuilder.cs @@ -86,7 +86,10 @@ internal LocalizationResources LocalizationResources internal TryReplaceToken? TokenReplacer; - internal List? Directives; + public List Directives => _directives ??= new (); + + private List? _directives; + /// /// Creates a parser based on the configuration of the command line builder. @@ -95,7 +98,7 @@ public Parser Build() => new( new CommandLineConfiguration( Command, - Directives, + _directives, enablePosixBundling: EnablePosixBundling, enableTokenReplacement: EnableTokenReplacement, parseErrorReportingExitCode: ParseErrorReportingExitCode, diff --git a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs index 0e1e6df0cd..cd3163eded 100644 --- a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs +++ b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs @@ -129,7 +129,7 @@ await feature.EnsureRegistered(async () => public static CommandLineBuilder UseEnvironmentVariableDirective( this CommandLineBuilder builder) { - (builder.Directives ??= new()).Add(new EnvironmentVariablesDirective()); + builder.Directives.Add(new EnvironmentVariablesDirective()); return builder; } @@ -337,7 +337,7 @@ public static CommandLineBuilder UseParseDirective( this CommandLineBuilder builder, int errorExitCode = 1) { - (builder.Directives ??= new()).Add(new ParseDirective(errorExitCode)); + builder.Directives.Add(new ParseDirective(errorExitCode)); return builder; } @@ -363,7 +363,7 @@ public static CommandLineBuilder UseParseErrorReporting( public static CommandLineBuilder UseSuggestDirective( this CommandLineBuilder builder) { - (builder.Directives ??= new()).Add(new SuggestDirective()); + builder.Directives.Add(new SuggestDirective()); return builder; } diff --git a/src/System.CommandLine/Directive.cs b/src/System.CommandLine/Directive.cs index 7faf8d016d..c50a280ad7 100644 --- a/src/System.CommandLine/Directive.cs +++ b/src/System.CommandLine/Directive.cs @@ -3,6 +3,7 @@ using System.CommandLine.Invocation; using System.Threading.Tasks; using System.Threading; +using System.CommandLine.Parsing; namespace System.CommandLine { @@ -42,19 +43,29 @@ public Directive(string name, } } - if (syncHandler is null && asyncHandler is null) - { - throw new ArgumentNullException(message: "A single handler needs to be provided", innerException: null); - } - Name = name; Description = description; - Handler = syncHandler is not null - ? new AnonymousCommandHandler(syncHandler) - : new AnonymousCommandHandler(asyncHandler!); + + if (syncHandler is not null) + { + Handler = new AnonymousCommandHandler(syncHandler); + } + else if (asyncHandler is not null) + { + Handler = new AnonymousCommandHandler(asyncHandler); + } } - internal ICommandHandler Handler { get; } + internal ICommandHandler? Handler { get; } + + /// + /// Method executed when given Directive is being parsed. + /// Useful for Directives that want to perform an action without setting the Handler for ParseResult. + /// + /// Parsed directive result. + public virtual void OnParsed(DirectiveResult directiveResult) + { + } private protected override string DefaultName => throw new NotImplementedException(); diff --git a/src/System.CommandLine/EnvironmentVariablesDirective.cs b/src/System.CommandLine/EnvironmentVariablesDirective.cs index 147ad9b67b..e570552992 100644 --- a/src/System.CommandLine/EnvironmentVariablesDirective.cs +++ b/src/System.CommandLine/EnvironmentVariablesDirective.cs @@ -1,4 +1,4 @@ -using System.CommandLine.Invocation; +using System.CommandLine.Parsing; namespace System.CommandLine { @@ -7,27 +7,26 @@ namespace System.CommandLine /// public sealed class EnvironmentVariablesDirective : Directive { - public EnvironmentVariablesDirective() : base("env", syncHandler: SyncHandler) + public EnvironmentVariablesDirective() : base("env") { } - private static void SyncHandler(InvocationContext context) + public override void OnParsed(DirectiveResult directiveResult) { - EnvironmentVariablesDirective symbol = (EnvironmentVariablesDirective)context.ParseResult.Symbol; - string? parsedValues = context.ParseResult.FindResultFor(symbol)!.Value; - - if (parsedValues is not null) + if (string.IsNullOrEmpty(directiveResult.Value)) { - string[] components = parsedValues.Split(new[] { '=' }, count: 2); - string variable = components.Length > 0 ? components[0].Trim() : string.Empty; - if (string.IsNullOrEmpty(variable) || components.Length < 2) - { - return; - } + return; + } - string value = components[1].Trim(); - Environment.SetEnvironmentVariable(variable, value); + string[] components = directiveResult.Value.Split(new[] { '=' }, count: 2); + string variable = components.Length > 0 ? components[0].Trim() : string.Empty; + if (string.IsNullOrEmpty(variable) || components.Length < 2) + { + return; } + + string value = components[1].Trim(); + Environment.SetEnvironmentVariable(variable, value); } } } diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index e5da8c18fd..639830445e 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -22,6 +22,7 @@ public class ParseResult internal ParseResult( Parser parser, + Symbol symbol, CommandResult rootCommandResult, CommandResult commandResult, List tokens, @@ -31,6 +32,7 @@ internal ParseResult( ICommandHandler? handler = null) { Parser = parser; + Symbol = symbol; _rootCommandResult = rootCommandResult; CommandResult = commandResult; _handler = handler; @@ -76,12 +78,6 @@ internal ParseResult( /// public IReadOnlyList Errors => _errors; - /// - /// Gets the directives found while parsing command line input. - /// - /// If is set to , then this collection will be empty. - public IReadOnlyDictionary> Directives => null!; - /// /// Gets the tokens identified while parsing command line input. /// diff --git a/src/System.CommandLine/Parsing/DirectiveResult.cs b/src/System.CommandLine/Parsing/DirectiveResult.cs index bfa1b5ce3a..20c1cf6e08 100644 --- a/src/System.CommandLine/Parsing/DirectiveResult.cs +++ b/src/System.CommandLine/Parsing/DirectiveResult.cs @@ -5,8 +5,8 @@ /// public sealed class DirectiveResult : SymbolResult { - internal DirectiveResult(Directive directive, Token token, string? value, SymbolResultTree symbolResultTree, SymbolResult rootCommand) - : base(symbolResultTree, rootCommand) + internal DirectiveResult(Directive directive, Token token, string value, SymbolResultTree symbolResultTree) + : base(symbolResultTree, null) // directives don't belong to any command { Directive = directive; Token = token; @@ -16,8 +16,8 @@ internal DirectiveResult(Directive directive, Token token, string? value, Symbol /// /// Parsed value of an [name:value] directive. /// - /// Can be null for [name] directives. - public string? Value { get; } + /// Can be empty for [name] directives. + public string Value { get; } /// /// The directive to which the result applies. diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index be2fbe453a..5476d48a09 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -19,6 +19,7 @@ internal sealed class ParseOperation private CommandResult _innermostCommandResult; private bool _isHelpRequested, _isParseRequested; private ICommandHandler? _handler; + private Symbol? _symbol; public ParseOperation( List tokens, @@ -75,6 +76,7 @@ internal ParseResult Parse(Parser parser) return new ( parser, + _symbol ?? _innermostCommandResult.Command, _rootCommandResult, _innermostCommandResult, _tokens, @@ -294,21 +296,24 @@ private void ParseDirectives() void ParseDirective() { var token = CurrentToken; - ReadOnlySpan withoutBrackets = token.Value.AsSpan(1, token.Value.Length - 2); - int indexOfColon = withoutBrackets.IndexOf(':'); - string? value = indexOfColon > 0 - ? withoutBrackets.Slice(indexOfColon + 1).ToString() - : null; if (token.Symbol is not Directive directive) { - _symbolResultTree.AddUnmatchedToken(token, commandResult: null); + // Directives_should_not_be_considered_as_unmatched_tokens return; } - DirectiveResult result = new (directive, token, value, _symbolResultTree, _rootCommandResult); + ReadOnlySpan withoutBrackets = token.Value.AsSpan(1, token.Value.Length - 2); + int indexOfColon = withoutBrackets.IndexOf(':'); + string? value = indexOfColon > 0 + ? withoutBrackets.Slice(indexOfColon + 1).ToString() + : string.Empty; // Directives_without_a_value_specified_have_a_value_of_empty_string + + DirectiveResult result = new (directive, token, value, _symbolResultTree); _symbolResultTree.Add(directive, result); + directive.OnParsed(result); _handler = directive.Handler; + _symbol = directive; if (directive is ParseDirective) { diff --git a/src/System.CommandLine/Parsing/ParseResultExtensions.cs b/src/System.CommandLine/Parsing/ParseResultExtensions.cs index 89b7a67c02..396db38ba3 100644 --- a/src/System.CommandLine/Parsing/ParseResultExtensions.cs +++ b/src/System.CommandLine/Parsing/ParseResultExtensions.cs @@ -85,9 +85,10 @@ private static void Diagram( builder.Append("!"); } - switch (symbolResult) { + case DirectiveResult: + break; case ArgumentResult argumentResult: { var includeArgumentName = diff --git a/src/System.CommandLine/Parsing/StringExtensions.cs b/src/System.CommandLine/Parsing/StringExtensions.cs index 664b32f102..de009463c0 100644 --- a/src/System.CommandLine/Parsing/StringExtensions.cs +++ b/src/System.CommandLine/Parsing/StringExtensions.cs @@ -123,8 +123,8 @@ internal static void Tokenize( { int colonIndex = arg.AsSpan().IndexOf(':'); string directiveName = colonIndex > 0 - ? arg.Substring(1, colonIndex) // [name:value] - : arg.Substring(1, arg.Length - 1); // [name] is a legal directive + ? arg.Substring(1, colonIndex - 1) // [name:value] + : arg.Substring(1, arg.Length - 2); // [name] is a legal directive Directive? directive = knownTokens.TryGetValue(directiveName, out var directiveToken) ? (Directive)directiveToken.Symbol! diff --git a/src/System.CommandLine/Parsing/SymbolResultExtensions.cs b/src/System.CommandLine/Parsing/SymbolResultExtensions.cs index 86a573f269..2a72e3f138 100644 --- a/src/System.CommandLine/Parsing/SymbolResultExtensions.cs +++ b/src/System.CommandLine/Parsing/SymbolResultExtensions.cs @@ -25,6 +25,7 @@ internal static Token Token(this SymbolResult symbolResult) { CommandResult commandResult => commandResult.Token, OptionResult optionResult => optionResult.Token ?? CreateImplicitToken(optionResult.Option), + DirectiveResult directiveResult => directiveResult.Token, _ => throw new ArgumentOutOfRangeException(nameof(symbolResult)) }; diff --git a/src/System.CommandLine/SuggestDirective.cs b/src/System.CommandLine/SuggestDirective.cs index d302f0851b..182eebe5fd 100644 --- a/src/System.CommandLine/SuggestDirective.cs +++ b/src/System.CommandLine/SuggestDirective.cs @@ -21,7 +21,7 @@ private static void SyncHandler(InvocationContext context) string? parsedValues = context.ParseResult.FindResultFor(symbol)!.Value; string? rawInput = context.ParseResult.CommandLineText; - int position = parsedValues is not null ? int.Parse(parsedValues) : rawInput?.Length ?? 0; + int position = !string.IsNullOrEmpty(parsedValues) ? int.Parse(parsedValues) : rawInput?.Length ?? 0; var commandLineToComplete = context.ParseResult.Tokens.LastOrDefault(t => t.Type != TokenType.Directive)?.Value ?? ""; From 47f9a89f9d61c4956deb59a82ce98acc1397081b Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 22 Feb 2023 15:05:33 +0100 Subject: [PATCH 06/13] handle directive specified multiple times --- ...ommandLine_api_is_not_changed.approved.txt | 2 +- .../DirectiveConfigurationExtensions.cs | 7 +++-- .../CommandLineBuilderExtensions.cs | 8 +++-- .../DirectiveTests.cs | 31 ++++++------------- .../EnvironmentVariablesDirective.cs | 26 +++++++++------- .../Parsing/DirectiveResult.cs | 15 ++++++--- .../Parsing/ParseOperation.cs | 21 ++++++++++--- src/System.CommandLine/SuggestDirective.cs | 2 +- 8 files changed, 63 insertions(+), 49 deletions(-) diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt index f5181d1a52..6ab084b551 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt +++ b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt @@ -402,7 +402,7 @@ System.CommandLine.Parsing public class DirectiveResult : SymbolResult public System.CommandLine.Directive Directive { get; } public Token Token { get; } - public System.String Value { get; } + public System.Collections.Generic.IReadOnlyList Values { get; } public class OptionResult : SymbolResult public System.Boolean IsImplicit { get; } public System.CommandLine.Option Option { get; } diff --git a/src/System.CommandLine.Hosting/DirectiveConfigurationExtensions.cs b/src/System.CommandLine.Hosting/DirectiveConfigurationExtensions.cs index ca300e77b6..4a9238af71 100644 --- a/src/System.CommandLine.Hosting/DirectiveConfigurationExtensions.cs +++ b/src/System.CommandLine.Hosting/DirectiveConfigurationExtensions.cs @@ -17,11 +17,14 @@ public static IConfigurationBuilder AddCommandLineDirectives( if (directive is null) throw new ArgumentNullException(nameof(directive)); - if (commandline.FindResultFor(directive) is not DirectiveResult result) + if (commandline.FindResultFor(directive) is not DirectiveResult result + || result.Values.Count == 0) + { return config; + } var kvpSeparator = new[] { '=' }; - return config.AddInMemoryCollection(new string[] { result.Value }.Select(s => + return config.AddInMemoryCollection(result.Values.Select(s => { var parts = s.Split(kvpSeparator, count: 2); var key = parts[0]; diff --git a/src/System.CommandLine.Rendering/CommandLineBuilderExtensions.cs b/src/System.CommandLine.Rendering/CommandLineBuilderExtensions.cs index 5f49ed006a..c8e1b95aeb 100644 --- a/src/System.CommandLine.Rendering/CommandLineBuilderExtensions.cs +++ b/src/System.CommandLine.Rendering/CommandLineBuilderExtensions.cs @@ -31,9 +31,10 @@ public static CommandLineBuilder UseAnsiTerminalWhenAvailable( private static bool PreferVirtualTerminal(ParseResult parseResult, Directive enableVtDirective) { - if (parseResult.FindResultFor(enableVtDirective) is DirectiveResult result) + if (parseResult.FindResultFor(enableVtDirective) is DirectiveResult result + && result.Values.Count == 1) { - string trueOrFalse = result.Value; + string trueOrFalse = result.Values[0]; if (bool.TryParse(trueOrFalse, out var pvt)) { @@ -47,7 +48,8 @@ private static bool PreferVirtualTerminal(ParseResult parseResult, Directive ena private static OutputMode OutputMode(ParseResult parseResult, Directive outputDirective) { if (parseResult.FindResultFor(outputDirective) is DirectiveResult result - && Enum.TryParse(result.Value, true, out var mode)) + && result.Values.Count == 1 + && Enum.TryParse(result.Values[0], true, out var mode)) { return mode; } diff --git a/src/System.CommandLine.Tests/DirectiveTests.cs b/src/System.CommandLine.Tests/DirectiveTests.cs index 4264e85dbb..1ed3f9f423 100644 --- a/src/System.CommandLine.Tests/DirectiveTests.cs +++ b/src/System.CommandLine.Tests/DirectiveTests.cs @@ -73,17 +73,17 @@ public void Directives_can_have_a_value_which_is_everything_after_the_first_colo ParseResult result = Parse(new Option("-y"), directive, $"{wholeText} -y"); - result.FindResultFor(directive).Value.Should().Be(expectedValue); + result.FindResultFor(directive).Values.Single().Should().Be(expectedValue); } [Fact] - public void Directives_without_a_value_specified_have_a_value_of_empty_string() + public void Directives_without_a_value_specified_have_no_values() { Directive directive = new("parse"); ParseResult result = Parse(new Option("-y"), directive, "[parse] -y"); - result.FindResultFor(directive).Value.Should().BeEmpty(); + result.FindResultFor(directive).Values.Should().BeEmpty(); } [Theory] @@ -109,26 +109,15 @@ public void Directives_cannot_contain_spaces(string value) create.Should().Throw(); } - //[Fact] - //public void When_a_directive_is_specified_more_than_once_then_its_values_are_aggregated() - //{ - // var option = new Option("-a"); - - // var result = option.Parse("[directive:one] [directive:two] -a"); - - // result.Directives.TryGetValue("directive", out var values).Should().BeTrue(); - // values.Should().BeEquivalentTo("one", "two"); - //} - - //[Fact] - //public void Directive_count_is_based_on_distinct_instances_of_directive_name() - //{ - // var command = new RootCommand(); + [Fact] + public void When_a_directive_is_specified_more_than_once_then_its_values_are_aggregated() + { + Directive directive = new("directive"); - // var result = command.Parse("[one] [two] [one:a] [one:b]"); + ParseResult result = Parse(new Option("-a"), directive, "[directive:one] [directive:two] -a"); - // result.Directives.Should().HaveCount(2); - //} + result.FindResultFor(directive).Values.Should().BeEquivalentTo("one", "two"); + } [Fact] public void When_directives_are_not_enabled_they_are_treated_as_regular_tokens() diff --git a/src/System.CommandLine/EnvironmentVariablesDirective.cs b/src/System.CommandLine/EnvironmentVariablesDirective.cs index e570552992..7ad3b0c9af 100644 --- a/src/System.CommandLine/EnvironmentVariablesDirective.cs +++ b/src/System.CommandLine/EnvironmentVariablesDirective.cs @@ -13,20 +13,24 @@ public EnvironmentVariablesDirective() : base("env") public override void OnParsed(DirectiveResult directiveResult) { - if (string.IsNullOrEmpty(directiveResult.Value)) + for (int i = 0; i < directiveResult.Values.Count; i++) { - return; - } + string parsedValue = directiveResult.Values[i]; - string[] components = directiveResult.Value.Split(new[] { '=' }, count: 2); - string variable = components.Length > 0 ? components[0].Trim() : string.Empty; - if (string.IsNullOrEmpty(variable) || components.Length < 2) - { - return; - } + int indexOfSeparator = parsedValue.AsSpan().IndexOf('='); + + if (indexOfSeparator > 0) + { + ReadOnlySpan variable = parsedValue.AsSpan(0, indexOfSeparator).Trim(); - string value = components[1].Trim(); - Environment.SetEnvironmentVariable(variable, value); + if (!variable.IsEmpty) + { + string value = parsedValue.AsSpan(indexOfSeparator + 1).Trim().ToString(); + + Environment.SetEnvironmentVariable(variable.ToString(), value); + } + } + } } } } diff --git a/src/System.CommandLine/Parsing/DirectiveResult.cs b/src/System.CommandLine/Parsing/DirectiveResult.cs index 20c1cf6e08..78ceeaf0d7 100644 --- a/src/System.CommandLine/Parsing/DirectiveResult.cs +++ b/src/System.CommandLine/Parsing/DirectiveResult.cs @@ -1,23 +1,26 @@ -namespace System.CommandLine.Parsing +using System.Collections.Generic; + +namespace System.CommandLine.Parsing { /// /// A result produced when parsing an . /// public sealed class DirectiveResult : SymbolResult { - internal DirectiveResult(Directive directive, Token token, string value, SymbolResultTree symbolResultTree) + private List? _values; + + internal DirectiveResult(Directive directive, Token token, SymbolResultTree symbolResultTree) : base(symbolResultTree, null) // directives don't belong to any command { Directive = directive; Token = token; - Value = value; } /// - /// Parsed value of an [name:value] directive. + /// Parsed values of [name:value] directive(s). /// /// Can be empty for [name] directives. - public string Value { get; } + public IReadOnlyList Values => _values is null ? Array.Empty() : _values; /// /// The directive to which the result applies. @@ -28,5 +31,7 @@ internal DirectiveResult(Directive directive, Token token, string value, SymbolR /// The token that was parsed to specify the directive. /// public Token Token { get; } + + internal void AddValue(string value) => (_values ??= new()).Add(value); } } diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 5476d48a09..96cfa6a291 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -303,14 +303,25 @@ void ParseDirective() return; } + DirectiveResult result; + if (_symbolResultTree.TryGetValue(directive, out var directiveResult)) + { + result = (DirectiveResult)directiveResult; + result.AddToken(token); + } + else + { + result = new (directive, token, _symbolResultTree); + _symbolResultTree.Add(directive, result); + } + ReadOnlySpan withoutBrackets = token.Value.AsSpan(1, token.Value.Length - 2); int indexOfColon = withoutBrackets.IndexOf(':'); - string? value = indexOfColon > 0 - ? withoutBrackets.Slice(indexOfColon + 1).ToString() - : string.Empty; // Directives_without_a_value_specified_have_a_value_of_empty_string + if (indexOfColon > 0) + { + result.AddValue(withoutBrackets.Slice(indexOfColon + 1).ToString()); + } - DirectiveResult result = new (directive, token, value, _symbolResultTree); - _symbolResultTree.Add(directive, result); directive.OnParsed(result); _handler = directive.Handler; _symbol = directive; diff --git a/src/System.CommandLine/SuggestDirective.cs b/src/System.CommandLine/SuggestDirective.cs index 182eebe5fd..e9f71eda65 100644 --- a/src/System.CommandLine/SuggestDirective.cs +++ b/src/System.CommandLine/SuggestDirective.cs @@ -18,7 +18,7 @@ public SuggestDirective() : base("suggest", syncHandler: SyncHandler) private static void SyncHandler(InvocationContext context) { SuggestDirective symbol = (SuggestDirective)context.ParseResult.Symbol; - string? parsedValues = context.ParseResult.FindResultFor(symbol)!.Value; + string? parsedValues = context.ParseResult.FindResultFor(symbol)!.Values.SingleOrDefault(); string? rawInput = context.ParseResult.CommandLineText; int position = !string.IsNullOrEmpty(parsedValues) ? int.Parse(parsedValues) : rawInput?.Length ?? 0; From 7127bc2deded68611a734834c9bdd169543c2b79 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 22 Feb 2023 18:31:55 +0100 Subject: [PATCH 07/13] remove OnParsed: call the parsed command handler from directive handler (a dirty solution) --- ...stem_CommandLine_api_is_not_changed.approved.txt | 2 -- src/System.CommandLine/Directive.cs | 9 --------- .../EnvironmentVariablesDirective.cs | 13 ++++++++++--- src/System.CommandLine/Parsing/ParseOperation.cs | 1 - 4 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt index 6ab084b551..82c459ca0d 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt +++ b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt @@ -110,10 +110,8 @@ System.CommandLine public class Directive : Symbol .ctor(System.String name, System.String description = null, System.Action syncHandler = null, System.Func asyncHandler = null) public System.Collections.Generic.IEnumerable GetCompletions(System.CommandLine.Completions.CompletionContext context) - public System.Void OnParsed(System.CommandLine.Parsing.DirectiveResult directiveResult) public class EnvironmentVariablesDirective : Directive .ctor() - public System.Void OnParsed(System.CommandLine.Parsing.DirectiveResult directiveResult) public static class Handler public static System.Void SetHandler(this Command command, System.Action handle) public static System.Void SetHandler(this Command command, System.Action handle) diff --git a/src/System.CommandLine/Directive.cs b/src/System.CommandLine/Directive.cs index c50a280ad7..9a0fde2d83 100644 --- a/src/System.CommandLine/Directive.cs +++ b/src/System.CommandLine/Directive.cs @@ -58,15 +58,6 @@ public Directive(string name, internal ICommandHandler? Handler { get; } - /// - /// Method executed when given Directive is being parsed. - /// Useful for Directives that want to perform an action without setting the Handler for ParseResult. - /// - /// Parsed directive result. - public virtual void OnParsed(DirectiveResult directiveResult) - { - } - private protected override string DefaultName => throw new NotImplementedException(); public override IEnumerable GetCompletions(CompletionContext context) diff --git a/src/System.CommandLine/EnvironmentVariablesDirective.cs b/src/System.CommandLine/EnvironmentVariablesDirective.cs index 7ad3b0c9af..0dcffd7d3e 100644 --- a/src/System.CommandLine/EnvironmentVariablesDirective.cs +++ b/src/System.CommandLine/EnvironmentVariablesDirective.cs @@ -1,4 +1,5 @@ -using System.CommandLine.Parsing; +using System.CommandLine.Invocation; +using System.CommandLine.Parsing; namespace System.CommandLine { @@ -7,12 +8,15 @@ namespace System.CommandLine /// public sealed class EnvironmentVariablesDirective : Directive { - public EnvironmentVariablesDirective() : base("env") + public EnvironmentVariablesDirective() : base("env", syncHandler: SyncHandler) { } - public override void OnParsed(DirectiveResult directiveResult) + private static void SyncHandler(InvocationContext context) { + EnvironmentVariablesDirective symbol = (EnvironmentVariablesDirective)context.ParseResult.Symbol; + DirectiveResult directiveResult = context.ParseResult.FindResultFor(symbol)!; + for (int i = 0; i < directiveResult.Values.Count; i++) { string parsedValue = directiveResult.Values[i]; @@ -31,6 +35,9 @@ public override void OnParsed(DirectiveResult directiveResult) } } } + + // we need a cleaner, more flexible and intuitive way of continuing the execution + context.ParseResult.CommandResult.Command.Handler?.Invoke(context); } } } diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 96cfa6a291..93de3e2528 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -322,7 +322,6 @@ void ParseDirective() result.AddValue(withoutBrackets.Slice(indexOfColon + 1).ToString()); } - directive.OnParsed(result); _handler = directive.Handler; _symbol = directive; From 96876bc8e357474c60064aa1a6ef604611add4ef Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 22 Feb 2023 23:29:20 +0100 Subject: [PATCH 08/13] exclude private protected properties, as Relative_order_of_arguments_and_options_within_a_command_does_not_matter was calling Directive.GetDefaultName and throwing --- src/System.CommandLine.Tests/ParserTests.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/System.CommandLine.Tests/ParserTests.cs b/src/System.CommandLine.Tests/ParserTests.cs index 16644eeef1..8926cde89a 100644 --- a/src/System.CommandLine.Tests/ParserTests.cs +++ b/src/System.CommandLine.Tests/ParserTests.cs @@ -453,12 +453,14 @@ public void Relative_order_of_arguments_and_options_within_a_command_does_not_ma .BeEquivalentTo( result2, x => x.IgnoringCyclicReferences() - .Excluding(y => y.WhichGetterHas(CSharpAccessModifier.Internal))); + .Excluding(y => y.WhichGetterHas(CSharpAccessModifier.Internal)) + .Excluding(y => y.WhichGetterHas(CSharpAccessModifier.PrivateProtected))); result1.Should() .BeEquivalentTo( result3, x => x.IgnoringCyclicReferences() - .Excluding(y => y.WhichGetterHas(CSharpAccessModifier.Internal))); + .Excluding(y => y.WhichGetterHas(CSharpAccessModifier.Internal)) + .Excluding(y => y.WhichGetterHas(CSharpAccessModifier.PrivateProtected))); } [Theory] From f200208332b96a6fc42a98cb015de1f3992ae3e5 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 22 Feb 2023 23:39:42 +0100 Subject: [PATCH 09/13] ParseDirective can print only other directives --- src/System.CommandLine/Parsing/ParseResultExtensions.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/System.CommandLine/Parsing/ParseResultExtensions.cs b/src/System.CommandLine/Parsing/ParseResultExtensions.cs index 2cfce4fee6..c8a61df0f3 100644 --- a/src/System.CommandLine/Parsing/ParseResultExtensions.cs +++ b/src/System.CommandLine/Parsing/ParseResultExtensions.cs @@ -3,11 +3,8 @@ using System.Collections; using System.CommandLine.Binding; -using System.CommandLine.Invocation; using System.Linq; using System.Text; -using System.Threading; -using System.Threading.Tasks; namespace System.CommandLine.Parsing { @@ -62,7 +59,7 @@ private static void Diagram( switch (symbolResult) { - case DirectiveResult: + case DirectiveResult directiveResult when directiveResult.Directive is not ParseDirective: break; case ArgumentResult argumentResult: { From 484bc57251cace41b902d3f858fbb6439466d6e6 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 22 Feb 2023 23:49:09 +0100 Subject: [PATCH 10/13] the reference to "this" can't be used when calling base class ctor introduce public APIs for setting the handler to make it possible to use reference to "this" in Directive handlers --- ...ommandLine_api_is_not_changed.approved.txt | 2 ++ src/System.CommandLine/Directive.cs | 26 ++++++++++++++++--- .../EnvironmentVariablesDirective.cs | 8 +++--- src/System.CommandLine/ParseDirective.cs | 9 +++---- src/System.CommandLine/ParseResult.cs | 7 ----- .../Parsing/ParseOperation.cs | 3 --- src/System.CommandLine/SuggestDirective.cs | 8 +++--- 7 files changed, 37 insertions(+), 26 deletions(-) diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt index 37233cffa4..110c0503c2 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt +++ b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt @@ -110,6 +110,8 @@ System.CommandLine public class Directive : Symbol .ctor(System.String name, System.String description = null, System.Action syncHandler = null, System.Func asyncHandler = null) public System.Collections.Generic.IEnumerable GetCompletions(System.CommandLine.Completions.CompletionContext context) + public System.Void SetAsynchronousHandler(System.Func handler) + public System.Void SetSynchronousHandler(System.Action handler) public class EnvironmentVariablesDirective : Directive .ctor() public static class Handler diff --git a/src/System.CommandLine/Directive.cs b/src/System.CommandLine/Directive.cs index 9a0fde2d83..822b996d2b 100644 --- a/src/System.CommandLine/Directive.cs +++ b/src/System.CommandLine/Directive.cs @@ -48,15 +48,35 @@ public Directive(string name, if (syncHandler is not null) { - Handler = new AnonymousCommandHandler(syncHandler); + SetSynchronousHandler(syncHandler); } else if (asyncHandler is not null) { - Handler = new AnonymousCommandHandler(asyncHandler); + SetAsynchronousHandler(asyncHandler); } } - internal ICommandHandler? Handler { get; } + public void SetAsynchronousHandler(Func handler) + { + if (handler is null) + { + throw new ArgumentNullException(nameof(handler)); + } + + Handler = new AnonymousCommandHandler(handler); + } + + public void SetSynchronousHandler(Action handler) + { + if (handler is null) + { + throw new ArgumentNullException(nameof(handler)); + } + + Handler = new AnonymousCommandHandler(handler); + } + + internal ICommandHandler? Handler { get; private set; } private protected override string DefaultName => throw new NotImplementedException(); diff --git a/src/System.CommandLine/EnvironmentVariablesDirective.cs b/src/System.CommandLine/EnvironmentVariablesDirective.cs index 0dcffd7d3e..6f65d52c87 100644 --- a/src/System.CommandLine/EnvironmentVariablesDirective.cs +++ b/src/System.CommandLine/EnvironmentVariablesDirective.cs @@ -8,14 +8,14 @@ namespace System.CommandLine /// public sealed class EnvironmentVariablesDirective : Directive { - public EnvironmentVariablesDirective() : base("env", syncHandler: SyncHandler) + public EnvironmentVariablesDirective() : base("env") { + SetSynchronousHandler(SyncHandler); } - private static void SyncHandler(InvocationContext context) + private void SyncHandler(InvocationContext context) { - EnvironmentVariablesDirective symbol = (EnvironmentVariablesDirective)context.ParseResult.Symbol; - DirectiveResult directiveResult = context.ParseResult.FindResultFor(symbol)!; + DirectiveResult directiveResult = context.ParseResult.FindResultFor(this)!; for (int i = 0; i < directiveResult.Values.Count; i++) { diff --git a/src/System.CommandLine/ParseDirective.cs b/src/System.CommandLine/ParseDirective.cs index 6bdbc99652..e6235caae4 100644 --- a/src/System.CommandLine/ParseDirective.cs +++ b/src/System.CommandLine/ParseDirective.cs @@ -10,20 +10,19 @@ namespace System.CommandLine public sealed class ParseDirective : Directive { /// If the parse result contains errors, this exit code will be used when the process exits. - public ParseDirective(int errorExitCode = 1) : base("parse", syncHandler: SyncHandler) + public ParseDirective(int errorExitCode = 1) : base("parse") { + SetSynchronousHandler(SyncHandler); ErrorExitCode = errorExitCode; } internal int ErrorExitCode { get; } - private static void SyncHandler(InvocationContext context) + private void SyncHandler(InvocationContext context) { - ParseDirective symbol = (ParseDirective)context.ParseResult.Symbol; - var parseResult = context.ParseResult; context.Console.Out.WriteLine(parseResult.Diagram()); - context.ExitCode = parseResult.Errors.Count == 0 ? 0 : symbol.ErrorExitCode; + context.ExitCode = parseResult.Errors.Count == 0 ? 0 : ErrorExitCode; } } } diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index a72659de05..afc9c0703a 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -25,7 +25,6 @@ public class ParseResult internal ParseResult( CommandLineConfiguration configuration, - Symbol symbol, CommandResult rootCommandResult, CommandResult commandResult, List tokens, @@ -35,7 +34,6 @@ internal ParseResult( ICommandHandler? handler = null) { Configuration = configuration; - Symbol = symbol; _rootCommandResult = rootCommandResult; CommandResult = commandResult; _handler = handler; @@ -86,11 +84,6 @@ internal ParseResult( /// public IReadOnlyList Tokens { get; } - /// - /// Gets the parsed Symbol which provided Handler. Currently it can be only a Directive or a Command. - /// - internal Symbol Symbol { get; } - /// /// Holds the value of a complete command line input prior to splitting and tokenization, when provided. /// diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 66916a30a1..16242174b7 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -19,7 +19,6 @@ internal sealed class ParseOperation private CommandResult _innermostCommandResult; private bool _isHelpRequested, _isParseRequested; private ICommandHandler? _handler; - private Symbol? _symbol; public ParseOperation( List tokens, @@ -76,7 +75,6 @@ internal ParseResult Parse() return new ( _configuration, - _symbol ?? _innermostCommandResult.Command, _rootCommandResult, _innermostCommandResult, _tokens, @@ -323,7 +321,6 @@ void ParseDirective() } _handler = directive.Handler; - _symbol = directive; if (directive is ParseDirective) { diff --git a/src/System.CommandLine/SuggestDirective.cs b/src/System.CommandLine/SuggestDirective.cs index ca9a85f407..c0c445eefb 100644 --- a/src/System.CommandLine/SuggestDirective.cs +++ b/src/System.CommandLine/SuggestDirective.cs @@ -11,15 +11,15 @@ namespace System.CommandLine /// The dotnet-suggest tool requires the suggest directive to be enabled for an application to provide completions. public sealed class SuggestDirective : Directive { - public SuggestDirective() : base("suggest", syncHandler: SyncHandler) + public SuggestDirective() : base("suggest") { + SetSynchronousHandler(SyncHandler); } - private static void SyncHandler(InvocationContext context) + private void SyncHandler(InvocationContext context) { ParseResult parseResult = context.ParseResult; - SuggestDirective symbol = (SuggestDirective)parseResult.Symbol; - string? parsedValues = parseResult.FindResultFor(symbol)!.Values.SingleOrDefault(); + string? parsedValues = parseResult.FindResultFor(this)!.Values.SingleOrDefault(); string? rawInput = parseResult.CommandLineText; int position = !string.IsNullOrEmpty(parsedValues) ? int.Parse(parsedValues) : rawInput?.Length ?? 0; From fc987d4802440d9b407c8bc610919f1163a15ff2 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 23 Feb 2023 15:54:57 +0100 Subject: [PATCH 11/13] provide more flexible mechanism for directive handlers: possibility to pass next handler and invoke it or not --- ...ommandLine_api_is_not_changed.approved.txt | 6 +- .../DirectiveTests.cs | 88 +++++++++++++++++++ src/System.CommandLine/Directive.cs | 44 +++------- .../EnvironmentVariablesDirective.cs | 29 ++++-- .../Invocation/CombinedCommandHandler.cs | 81 +++++++++++++++++ src/System.CommandLine/ParseDirective.cs | 20 ++++- src/System.CommandLine/ParseResult.cs | 22 ++--- .../Parsing/ParseOperation.cs | 44 ++++------ src/System.CommandLine/SuggestDirective.cs | 16 +++- 9 files changed, 269 insertions(+), 81 deletions(-) create mode 100644 src/System.CommandLine/Invocation/CombinedCommandHandler.cs diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt index 110c0503c2..e7e6d35eaf 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt +++ b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt @@ -108,10 +108,10 @@ System.CommandLine public static System.Void Write(this IConsole console, System.String value) public static System.Void WriteLine(this IConsole console, System.String value) public class Directive : Symbol - .ctor(System.String name, System.String description = null, System.Action syncHandler = null, System.Func asyncHandler = null) + .ctor(System.String name, System.String description = null, System.Action syncHandler = null, System.Func asyncHandler = null) public System.Collections.Generic.IEnumerable GetCompletions(System.CommandLine.Completions.CompletionContext context) - public System.Void SetAsynchronousHandler(System.Func handler) - public System.Void SetSynchronousHandler(System.Action handler) + public System.Void SetAsynchronousHandler(System.Func handler) + public System.Void SetSynchronousHandler(System.Action handler) public class EnvironmentVariablesDirective : Directive .ctor() public static class Handler diff --git a/src/System.CommandLine.Tests/DirectiveTests.cs b/src/System.CommandLine.Tests/DirectiveTests.cs index 951980988f..a4e09f321d 100644 --- a/src/System.CommandLine.Tests/DirectiveTests.cs +++ b/src/System.CommandLine.Tests/DirectiveTests.cs @@ -2,7 +2,9 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; +using System.Globalization; using System.Linq; +using System.Threading.Tasks; using FluentAssertions; using Xunit; @@ -137,6 +139,50 @@ public void When_directives_are_not_enabled_they_are_treated_as_regular_tokens() .BeEquivalentTo("[hello]"); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Directive_can_restore_the_state_after_running_continuation(bool async) + { + const string plCulture = "pl-PL", enUsCulture = "en-US"; + const string envVarName = "uniqueName", envVarValue = "just"; + + var before = CultureInfo.CurrentUICulture; + + try + { + CultureInfo.CurrentUICulture = new(enUsCulture); + + bool invoked = false; + Option option = new("-a"); + RootCommand root = new() { option }; + CommandLineBuilder builder = new(root); + builder.Directives.Add(new EnvironmentVariablesDirective()); + builder.Directives.Add(new CultureDirective()); + root.SetHandler(ctx => + { + invoked = true; + CultureInfo.CurrentUICulture.Name.Should().Be(plCulture); + Environment.GetEnvironmentVariable(envVarName).Should().Be(envVarValue); + }); + + if (async) + { + await builder.Build().InvokeAsync($"[culture:{plCulture}] [env:{envVarName}={envVarValue}]"); + } + else + { + builder.Build().Invoke($"[culture:{plCulture}] [env:{envVarName}={envVarValue}]"); + } + + invoked.Should().BeTrue(); + } + finally + { + CultureInfo.CurrentUICulture = before; + } + } + private static ParseResult Parse(Option option, Directive directive, string commandLine) { RootCommand root = new() { option }; @@ -145,5 +191,47 @@ private static ParseResult Parse(Option option, Directive directive, string comm return root.Parse(commandLine, builder.Build()); } + + private sealed class CultureDirective : Directive + { + public CultureDirective() : base("culture") + { + SetSynchronousHandler((ctx, next) => + { + CultureInfo cultureBefore = CultureInfo.CurrentUICulture; + + try + { + string cultureName = ctx.ParseResult.FindResultFor(this).Values.Single(); + + CultureInfo.CurrentUICulture = new CultureInfo(cultureName); + + next?.Invoke(ctx); + } + finally + { + CultureInfo.CurrentUICulture = cultureBefore; + } + }); + SetAsynchronousHandler(async (ctx, next, ct) => + { + CultureInfo cultureBefore = CultureInfo.CurrentUICulture; + + try + { + string cultureName = ctx.ParseResult.FindResultFor(this).Values.Single(); + + CultureInfo.CurrentUICulture = new CultureInfo(cultureName); + + await next?.InvokeAsync(ctx, ct); + } + finally + { + CultureInfo.CurrentUICulture = cultureBefore; + } + }); + } + } + } } \ No newline at end of file diff --git a/src/System.CommandLine/Directive.cs b/src/System.CommandLine/Directive.cs index 822b996d2b..8362a37755 100644 --- a/src/System.CommandLine/Directive.cs +++ b/src/System.CommandLine/Directive.cs @@ -3,7 +3,6 @@ using System.CommandLine.Invocation; using System.Threading.Tasks; using System.Threading; -using System.CommandLine.Parsing; namespace System.CommandLine { @@ -18,6 +17,9 @@ namespace System.CommandLine /// public class Directive : Symbol { + internal Action? SyncHandler; + internal Func? AsyncHandler; + /// /// Initializes a new instance of the Directive class. /// @@ -25,10 +27,12 @@ public class Directive : Symbol /// The description of the directive, shown in help. /// The synchronous action that is invoked when directive is parsed. /// The asynchronous action that is invoked when directive is parsed. + /// The second argument of both handlers is next handler than can be invoked. + /// Example: a custom directive might just change current culture and run actual command afterwards. public Directive(string name, string? description = null, - Action? syncHandler = null, - Func? asyncHandler = null) + Action? syncHandler = null, + Func? asyncHandler = null) { if (string.IsNullOrWhiteSpace(name)) { @@ -46,37 +50,17 @@ public Directive(string name, Name = name; Description = description; - if (syncHandler is not null) - { - SetSynchronousHandler(syncHandler); - } - else if (asyncHandler is not null) - { - SetAsynchronousHandler(asyncHandler); - } + SyncHandler = syncHandler; + AsyncHandler = asyncHandler; } - public void SetAsynchronousHandler(Func handler) - { - if (handler is null) - { - throw new ArgumentNullException(nameof(handler)); - } + internal bool HasHandler => SyncHandler != null || AsyncHandler != null; - Handler = new AnonymousCommandHandler(handler); - } - - public void SetSynchronousHandler(Action handler) - { - if (handler is null) - { - throw new ArgumentNullException(nameof(handler)); - } - - Handler = new AnonymousCommandHandler(handler); - } + public void SetAsynchronousHandler(Func handler) + => AsyncHandler = handler ?? throw new ArgumentNullException(nameof(handler)); - internal ICommandHandler? Handler { get; private set; } + public void SetSynchronousHandler(Action handler) + => SyncHandler = handler ?? throw new ArgumentNullException(nameof(handler)); private protected override string DefaultName => throw new NotImplementedException(); diff --git a/src/System.CommandLine/EnvironmentVariablesDirective.cs b/src/System.CommandLine/EnvironmentVariablesDirective.cs index 6f65d52c87..ff91405bda 100644 --- a/src/System.CommandLine/EnvironmentVariablesDirective.cs +++ b/src/System.CommandLine/EnvironmentVariablesDirective.cs @@ -1,5 +1,5 @@ -using System.CommandLine.Invocation; -using System.CommandLine.Parsing; +using System.CommandLine.Parsing; +using System.Threading.Tasks; namespace System.CommandLine { @@ -10,12 +10,28 @@ public sealed class EnvironmentVariablesDirective : Directive { public EnvironmentVariablesDirective() : base("env") { - SetSynchronousHandler(SyncHandler); + SetSynchronousHandler((context, next) => + { + SetEnvVars(context.ParseResult); + + next?.Invoke(context); + }); + SetAsynchronousHandler((context, next, cancellationToken) => + { + if (cancellationToken.IsCancellationRequested) + { + return Task.FromCanceled(cancellationToken); + } + + SetEnvVars(context.ParseResult); + + return next?.InvokeAsync(context, cancellationToken) ?? Task.CompletedTask; + }); } - private void SyncHandler(InvocationContext context) + private void SetEnvVars(ParseResult parseResult) { - DirectiveResult directiveResult = context.ParseResult.FindResultFor(this)!; + DirectiveResult directiveResult = parseResult.FindResultFor(this)!; for (int i = 0; i < directiveResult.Values.Count; i++) { @@ -35,9 +51,6 @@ private void SyncHandler(InvocationContext context) } } } - - // we need a cleaner, more flexible and intuitive way of continuing the execution - context.ParseResult.CommandResult.Command.Handler?.Invoke(context); } } } diff --git a/src/System.CommandLine/Invocation/CombinedCommandHandler.cs b/src/System.CommandLine/Invocation/CombinedCommandHandler.cs new file mode 100644 index 0000000000..a25c50e812 --- /dev/null +++ b/src/System.CommandLine/Invocation/CombinedCommandHandler.cs @@ -0,0 +1,81 @@ +using System.CommandLine.Parsing; +using System.Threading; +using System.Threading.Tasks; + +namespace System.CommandLine.Invocation +{ + internal sealed class ChainedCommandHandler : ICommandHandler + { + private readonly SymbolResultTree _symbols; + private readonly ICommandHandler? _commandHandler; + + internal ChainedCommandHandler(SymbolResultTree symbols, ICommandHandler? commandHandler) + { + _symbols = symbols; + _commandHandler = commandHandler; + } + + public int Invoke(InvocationContext context) + { + // We want to build a stack of (action, next) pairs. But we are not using any collection or LINQ. + // Each handler is closure (a lambda with state), where state is the "next" handler. + Action? chainedHandler = _commandHandler is not null + ? (ctx, next) => _commandHandler.Invoke(ctx) + : null; + ICommandHandler? chainedHandlerArgument = null; + + foreach (var pair in _symbols) + { + if (pair.Key is Directive directive && directive.HasHandler) + { + var syncHandler = directive.SyncHandler + ?? throw new NotSupportedException($"Directive {directive.Name} does not provide a synchronous handler."); + + if (chainedHandler is not null) + { + // capture the state in explicit way, to hint the compiler that the current state needs to be used + var capturedHandler = chainedHandler; + var capturedArgument = chainedHandlerArgument; + + chainedHandlerArgument = new AnonymousCommandHandler(ctx => capturedHandler.Invoke(ctx, capturedArgument)); + } + chainedHandler = syncHandler; + } + } + + chainedHandler!.Invoke(context, chainedHandlerArgument); + + return context.ExitCode; + } + + public async Task InvokeAsync(InvocationContext context, CancellationToken cancellationToken = default) + { + Func? chainedHandler = _commandHandler is not null + ? (ctx, next, ct) => _commandHandler.InvokeAsync(ctx, ct) + : null; + ICommandHandler? chainedHandlerArgument = null; + + foreach (var pair in _symbols) + { + if (pair.Key is Directive directive && directive.HasHandler) + { + var asyncHandler = directive.AsyncHandler + ?? throw new NotSupportedException($"Directive {directive.Name} does not provide an asynchronous handler."); + + if (chainedHandler is not null) + { + var capturedHandler = chainedHandler; + var capturedArgument = chainedHandlerArgument; + + chainedHandlerArgument = new AnonymousCommandHandler((ctx, ct) => capturedHandler.Invoke(ctx, capturedArgument, ct)); + } + chainedHandler = asyncHandler; + } + } + + await chainedHandler!.Invoke(context, chainedHandlerArgument, cancellationToken); + + return context.ExitCode; + } + } +} diff --git a/src/System.CommandLine/ParseDirective.cs b/src/System.CommandLine/ParseDirective.cs index e6235caae4..dedb445d78 100644 --- a/src/System.CommandLine/ParseDirective.cs +++ b/src/System.CommandLine/ParseDirective.cs @@ -1,6 +1,7 @@ using System.CommandLine.Invocation; using System.CommandLine.IO; using System.CommandLine.Parsing; +using System.Threading.Tasks; namespace System.CommandLine { @@ -12,17 +13,32 @@ public sealed class ParseDirective : Directive /// If the parse result contains errors, this exit code will be used when the process exits. public ParseDirective(int errorExitCode = 1) : base("parse") { - SetSynchronousHandler(SyncHandler); ErrorExitCode = errorExitCode; + + SetSynchronousHandler(PrintDiagramAndQuit); + SetAsynchronousHandler((context, next, cancellationToken) => + { + if (cancellationToken.IsCancellationRequested) + { + return Task.FromCanceled(cancellationToken); + } + + PrintDiagramAndQuit(context, null); + + return Task.CompletedTask; + }); } internal int ErrorExitCode { get; } - private void SyncHandler(InvocationContext context) + private void PrintDiagramAndQuit(InvocationContext context, ICommandHandler? next) { var parseResult = context.ParseResult; context.Console.Out.WriteLine(parseResult.Diagram()); context.ExitCode = parseResult.Errors.Count == 0 ? 0 : ErrorExitCode; + + // parse directive has a precedence over --help and --version and any command + // we don't invoke next here. } } } diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index afc9c0703a..43bf77e587 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -22,6 +22,7 @@ public class ParseResult private readonly IReadOnlyList _unmatchedTokens; private CompletionContext? _completionContext; private ICommandHandler? _handler; + private bool _useChainedHandler; internal ParseResult( CommandLineConfiguration configuration, @@ -30,13 +31,15 @@ internal ParseResult( List tokens, IReadOnlyList? unmatchedTokens, List? errors, - string? commandLineText = null, - ICommandHandler? handler = null) + string? commandLineText, + ICommandHandler? handler, + bool useChainedHandler) { Configuration = configuration; _rootCommandResult = rootCommandResult; CommandResult = commandResult; _handler = handler; + _useChainedHandler = useChainedHandler; // skip the root command when populating Tokens property if (tokens.Count > 1) @@ -241,21 +244,18 @@ internal ICommandHandler? Handler { get { - if (_handler is not null) - { - return _handler; - } + _handler ??= CommandResult.Command.Handler; - if (CommandResult.Command is { } command) + if (_useChainedHandler && _handler is not ChainedCommandHandler) { - return command.Handler; + // we can't create it when creating a ParseResult, because some Hosting extensions + // set Command.Handler via middleware, which is executed after parsing + _handler = new ChainedCommandHandler(_rootCommandResult.SymbolResultTree, _handler); } - return null; + return _handler; } - set => _handler = value; } - private SymbolResult SymbolToComplete(int? position = null) { var commandResult = CommandResult; diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 16242174b7..43f8e9658b 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -17,8 +17,8 @@ internal sealed class ParseOperation private int _index; private CommandResult _innermostCommandResult; - private bool _isHelpRequested, _isParseRequested; - private ICommandHandler? _handler; + private bool _isHelpRequested, _useChainedHandler; + private ICommandHandler? _commandHandler; public ParseOperation( List tokens, @@ -60,20 +60,20 @@ internal ParseResult Parse() Validate(); } - if (_handler is null) + if (_commandHandler is null) { if (_configuration.ParseErrorReportingExitCode.HasValue && _symbolResultTree.ErrorCount > 0) { - _handler = new AnonymousCommandHandler(ParseErrorResult.Apply); + _commandHandler = new AnonymousCommandHandler(ParseErrorResult.Apply); } else if (_configuration.MaxLevenshteinDistance > 0 && _rootCommandResult.Command.TreatUnmatchedTokensAsErrors && _symbolResultTree.UnmatchedTokens is not null) { - _handler = new AnonymousCommandHandler(TypoCorrection.ProvideSuggestions); + _commandHandler = new AnonymousCommandHandler(TypoCorrection.ProvideSuggestions); } } - return new ( + return new( _configuration, _rootCommandResult, _innermostCommandResult, @@ -81,7 +81,8 @@ internal ParseResult Parse() _symbolResultTree.UnmatchedTokens, _symbolResultTree.Errors, _rawInput, - _handler); + _commandHandler, + _useChainedHandler); } private void ParseSubcommand() @@ -187,19 +188,15 @@ private void ParseOption() if (!_symbolResultTree.TryGetValue(option, out SymbolResult? symbolResult)) { - // parse directive has a precedence over --help and --version - if (!_isParseRequested) + if (option is HelpOption) { - if (option is HelpOption) - { - _isHelpRequested = true; + _isHelpRequested = true; - _handler = new AnonymousCommandHandler(HelpOption.Handler); - } - else if (option is VersionOption) - { - _handler = new AnonymousCommandHandler(VersionOption.Handler); - } + _commandHandler = new AnonymousCommandHandler(HelpOption.Handler); + } + else if (option is VersionOption) + { + _commandHandler = new AnonymousCommandHandler(VersionOption.Handler); } optionResult = new OptionResult( @@ -311,6 +308,10 @@ void ParseDirective() { result = new (directive, token, _symbolResultTree); _symbolResultTree.Add(directive, result); + if (directive.HasHandler) + { + _useChainedHandler = true; + } } ReadOnlySpan withoutBrackets = token.Value.AsSpan(1, token.Value.Length - 2); @@ -319,13 +320,6 @@ void ParseDirective() { result.AddValue(withoutBrackets.Slice(indexOfColon + 1).ToString()); } - - _handler = directive.Handler; - - if (directive is ParseDirective) - { - _isParseRequested = true; - } } } diff --git a/src/System.CommandLine/SuggestDirective.cs b/src/System.CommandLine/SuggestDirective.cs index c0c445eefb..b9c345deca 100644 --- a/src/System.CommandLine/SuggestDirective.cs +++ b/src/System.CommandLine/SuggestDirective.cs @@ -2,6 +2,7 @@ using System.Linq; using System.CommandLine.IO; using System.CommandLine.Parsing; +using System.Threading.Tasks; namespace System.CommandLine { @@ -13,10 +14,21 @@ public sealed class SuggestDirective : Directive { public SuggestDirective() : base("suggest") { - SetSynchronousHandler(SyncHandler); + SetSynchronousHandler(ProvideCompletionsAndQuit); + SetAsynchronousHandler((context, next, cancellationToken) => + { + if (cancellationToken.IsCancellationRequested) + { + return Task.FromCanceled(cancellationToken); + } + + ProvideCompletionsAndQuit(context, null); + + return Task.CompletedTask; + }); } - private void SyncHandler(InvocationContext context) + private void ProvideCompletionsAndQuit(InvocationContext context, ICommandHandler? next) { ParseResult parseResult = context.ParseResult; string? parsedValues = parseResult.FindResultFor(this)!.Values.SingleOrDefault(); From ea619bf743a0d5a04c292a3281eb91b8dd853f6e Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 24 Feb 2023 18:03:37 +0100 Subject: [PATCH 12/13] Revert "provide more flexible mechanism for directive handlers: possibility to pass next handler and invoke it or not" This reverts commit fc987d4802440d9b407c8bc610919f1163a15ff2. --- ...ommandLine_api_is_not_changed.approved.txt | 6 +- .../DirectiveTests.cs | 88 ------------------- src/System.CommandLine/Directive.cs | 44 +++++++--- .../EnvironmentVariablesDirective.cs | 29 ++---- .../Invocation/CombinedCommandHandler.cs | 81 ----------------- src/System.CommandLine/ParseDirective.cs | 20 +---- src/System.CommandLine/ParseResult.cs | 22 ++--- .../Parsing/ParseOperation.cs | 44 ++++++---- src/System.CommandLine/SuggestDirective.cs | 16 +--- 9 files changed, 81 insertions(+), 269 deletions(-) delete mode 100644 src/System.CommandLine/Invocation/CombinedCommandHandler.cs diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt index e7e6d35eaf..110c0503c2 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt +++ b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt @@ -108,10 +108,10 @@ System.CommandLine public static System.Void Write(this IConsole console, System.String value) public static System.Void WriteLine(this IConsole console, System.String value) public class Directive : Symbol - .ctor(System.String name, System.String description = null, System.Action syncHandler = null, System.Func asyncHandler = null) + .ctor(System.String name, System.String description = null, System.Action syncHandler = null, System.Func asyncHandler = null) public System.Collections.Generic.IEnumerable GetCompletions(System.CommandLine.Completions.CompletionContext context) - public System.Void SetAsynchronousHandler(System.Func handler) - public System.Void SetSynchronousHandler(System.Action handler) + public System.Void SetAsynchronousHandler(System.Func handler) + public System.Void SetSynchronousHandler(System.Action handler) public class EnvironmentVariablesDirective : Directive .ctor() public static class Handler diff --git a/src/System.CommandLine.Tests/DirectiveTests.cs b/src/System.CommandLine.Tests/DirectiveTests.cs index a4e09f321d..951980988f 100644 --- a/src/System.CommandLine.Tests/DirectiveTests.cs +++ b/src/System.CommandLine.Tests/DirectiveTests.cs @@ -2,9 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; -using System.Globalization; using System.Linq; -using System.Threading.Tasks; using FluentAssertions; using Xunit; @@ -139,50 +137,6 @@ public void When_directives_are_not_enabled_they_are_treated_as_regular_tokens() .BeEquivalentTo("[hello]"); } - [Theory] - [InlineData(false)] - [InlineData(true)] - public async Task Directive_can_restore_the_state_after_running_continuation(bool async) - { - const string plCulture = "pl-PL", enUsCulture = "en-US"; - const string envVarName = "uniqueName", envVarValue = "just"; - - var before = CultureInfo.CurrentUICulture; - - try - { - CultureInfo.CurrentUICulture = new(enUsCulture); - - bool invoked = false; - Option option = new("-a"); - RootCommand root = new() { option }; - CommandLineBuilder builder = new(root); - builder.Directives.Add(new EnvironmentVariablesDirective()); - builder.Directives.Add(new CultureDirective()); - root.SetHandler(ctx => - { - invoked = true; - CultureInfo.CurrentUICulture.Name.Should().Be(plCulture); - Environment.GetEnvironmentVariable(envVarName).Should().Be(envVarValue); - }); - - if (async) - { - await builder.Build().InvokeAsync($"[culture:{plCulture}] [env:{envVarName}={envVarValue}]"); - } - else - { - builder.Build().Invoke($"[culture:{plCulture}] [env:{envVarName}={envVarValue}]"); - } - - invoked.Should().BeTrue(); - } - finally - { - CultureInfo.CurrentUICulture = before; - } - } - private static ParseResult Parse(Option option, Directive directive, string commandLine) { RootCommand root = new() { option }; @@ -191,47 +145,5 @@ private static ParseResult Parse(Option option, Directive directive, string comm return root.Parse(commandLine, builder.Build()); } - - private sealed class CultureDirective : Directive - { - public CultureDirective() : base("culture") - { - SetSynchronousHandler((ctx, next) => - { - CultureInfo cultureBefore = CultureInfo.CurrentUICulture; - - try - { - string cultureName = ctx.ParseResult.FindResultFor(this).Values.Single(); - - CultureInfo.CurrentUICulture = new CultureInfo(cultureName); - - next?.Invoke(ctx); - } - finally - { - CultureInfo.CurrentUICulture = cultureBefore; - } - }); - SetAsynchronousHandler(async (ctx, next, ct) => - { - CultureInfo cultureBefore = CultureInfo.CurrentUICulture; - - try - { - string cultureName = ctx.ParseResult.FindResultFor(this).Values.Single(); - - CultureInfo.CurrentUICulture = new CultureInfo(cultureName); - - await next?.InvokeAsync(ctx, ct); - } - finally - { - CultureInfo.CurrentUICulture = cultureBefore; - } - }); - } - } - } } \ No newline at end of file diff --git a/src/System.CommandLine/Directive.cs b/src/System.CommandLine/Directive.cs index 8362a37755..822b996d2b 100644 --- a/src/System.CommandLine/Directive.cs +++ b/src/System.CommandLine/Directive.cs @@ -3,6 +3,7 @@ using System.CommandLine.Invocation; using System.Threading.Tasks; using System.Threading; +using System.CommandLine.Parsing; namespace System.CommandLine { @@ -17,9 +18,6 @@ namespace System.CommandLine /// public class Directive : Symbol { - internal Action? SyncHandler; - internal Func? AsyncHandler; - /// /// Initializes a new instance of the Directive class. /// @@ -27,12 +25,10 @@ public class Directive : Symbol /// The description of the directive, shown in help. /// The synchronous action that is invoked when directive is parsed. /// The asynchronous action that is invoked when directive is parsed. - /// The second argument of both handlers is next handler than can be invoked. - /// Example: a custom directive might just change current culture and run actual command afterwards. public Directive(string name, string? description = null, - Action? syncHandler = null, - Func? asyncHandler = null) + Action? syncHandler = null, + Func? asyncHandler = null) { if (string.IsNullOrWhiteSpace(name)) { @@ -50,17 +46,37 @@ public Directive(string name, Name = name; Description = description; - SyncHandler = syncHandler; - AsyncHandler = asyncHandler; + if (syncHandler is not null) + { + SetSynchronousHandler(syncHandler); + } + else if (asyncHandler is not null) + { + SetAsynchronousHandler(asyncHandler); + } } - internal bool HasHandler => SyncHandler != null || AsyncHandler != null; + public void SetAsynchronousHandler(Func handler) + { + if (handler is null) + { + throw new ArgumentNullException(nameof(handler)); + } - public void SetAsynchronousHandler(Func handler) - => AsyncHandler = handler ?? throw new ArgumentNullException(nameof(handler)); + Handler = new AnonymousCommandHandler(handler); + } + + public void SetSynchronousHandler(Action handler) + { + if (handler is null) + { + throw new ArgumentNullException(nameof(handler)); + } + + Handler = new AnonymousCommandHandler(handler); + } - public void SetSynchronousHandler(Action handler) - => SyncHandler = handler ?? throw new ArgumentNullException(nameof(handler)); + internal ICommandHandler? Handler { get; private set; } private protected override string DefaultName => throw new NotImplementedException(); diff --git a/src/System.CommandLine/EnvironmentVariablesDirective.cs b/src/System.CommandLine/EnvironmentVariablesDirective.cs index ff91405bda..6f65d52c87 100644 --- a/src/System.CommandLine/EnvironmentVariablesDirective.cs +++ b/src/System.CommandLine/EnvironmentVariablesDirective.cs @@ -1,5 +1,5 @@ -using System.CommandLine.Parsing; -using System.Threading.Tasks; +using System.CommandLine.Invocation; +using System.CommandLine.Parsing; namespace System.CommandLine { @@ -10,28 +10,12 @@ public sealed class EnvironmentVariablesDirective : Directive { public EnvironmentVariablesDirective() : base("env") { - SetSynchronousHandler((context, next) => - { - SetEnvVars(context.ParseResult); - - next?.Invoke(context); - }); - SetAsynchronousHandler((context, next, cancellationToken) => - { - if (cancellationToken.IsCancellationRequested) - { - return Task.FromCanceled(cancellationToken); - } - - SetEnvVars(context.ParseResult); - - return next?.InvokeAsync(context, cancellationToken) ?? Task.CompletedTask; - }); + SetSynchronousHandler(SyncHandler); } - private void SetEnvVars(ParseResult parseResult) + private void SyncHandler(InvocationContext context) { - DirectiveResult directiveResult = parseResult.FindResultFor(this)!; + DirectiveResult directiveResult = context.ParseResult.FindResultFor(this)!; for (int i = 0; i < directiveResult.Values.Count; i++) { @@ -51,6 +35,9 @@ private void SetEnvVars(ParseResult parseResult) } } } + + // we need a cleaner, more flexible and intuitive way of continuing the execution + context.ParseResult.CommandResult.Command.Handler?.Invoke(context); } } } diff --git a/src/System.CommandLine/Invocation/CombinedCommandHandler.cs b/src/System.CommandLine/Invocation/CombinedCommandHandler.cs deleted file mode 100644 index a25c50e812..0000000000 --- a/src/System.CommandLine/Invocation/CombinedCommandHandler.cs +++ /dev/null @@ -1,81 +0,0 @@ -using System.CommandLine.Parsing; -using System.Threading; -using System.Threading.Tasks; - -namespace System.CommandLine.Invocation -{ - internal sealed class ChainedCommandHandler : ICommandHandler - { - private readonly SymbolResultTree _symbols; - private readonly ICommandHandler? _commandHandler; - - internal ChainedCommandHandler(SymbolResultTree symbols, ICommandHandler? commandHandler) - { - _symbols = symbols; - _commandHandler = commandHandler; - } - - public int Invoke(InvocationContext context) - { - // We want to build a stack of (action, next) pairs. But we are not using any collection or LINQ. - // Each handler is closure (a lambda with state), where state is the "next" handler. - Action? chainedHandler = _commandHandler is not null - ? (ctx, next) => _commandHandler.Invoke(ctx) - : null; - ICommandHandler? chainedHandlerArgument = null; - - foreach (var pair in _symbols) - { - if (pair.Key is Directive directive && directive.HasHandler) - { - var syncHandler = directive.SyncHandler - ?? throw new NotSupportedException($"Directive {directive.Name} does not provide a synchronous handler."); - - if (chainedHandler is not null) - { - // capture the state in explicit way, to hint the compiler that the current state needs to be used - var capturedHandler = chainedHandler; - var capturedArgument = chainedHandlerArgument; - - chainedHandlerArgument = new AnonymousCommandHandler(ctx => capturedHandler.Invoke(ctx, capturedArgument)); - } - chainedHandler = syncHandler; - } - } - - chainedHandler!.Invoke(context, chainedHandlerArgument); - - return context.ExitCode; - } - - public async Task InvokeAsync(InvocationContext context, CancellationToken cancellationToken = default) - { - Func? chainedHandler = _commandHandler is not null - ? (ctx, next, ct) => _commandHandler.InvokeAsync(ctx, ct) - : null; - ICommandHandler? chainedHandlerArgument = null; - - foreach (var pair in _symbols) - { - if (pair.Key is Directive directive && directive.HasHandler) - { - var asyncHandler = directive.AsyncHandler - ?? throw new NotSupportedException($"Directive {directive.Name} does not provide an asynchronous handler."); - - if (chainedHandler is not null) - { - var capturedHandler = chainedHandler; - var capturedArgument = chainedHandlerArgument; - - chainedHandlerArgument = new AnonymousCommandHandler((ctx, ct) => capturedHandler.Invoke(ctx, capturedArgument, ct)); - } - chainedHandler = asyncHandler; - } - } - - await chainedHandler!.Invoke(context, chainedHandlerArgument, cancellationToken); - - return context.ExitCode; - } - } -} diff --git a/src/System.CommandLine/ParseDirective.cs b/src/System.CommandLine/ParseDirective.cs index dedb445d78..e6235caae4 100644 --- a/src/System.CommandLine/ParseDirective.cs +++ b/src/System.CommandLine/ParseDirective.cs @@ -1,7 +1,6 @@ using System.CommandLine.Invocation; using System.CommandLine.IO; using System.CommandLine.Parsing; -using System.Threading.Tasks; namespace System.CommandLine { @@ -13,32 +12,17 @@ public sealed class ParseDirective : Directive /// If the parse result contains errors, this exit code will be used when the process exits. public ParseDirective(int errorExitCode = 1) : base("parse") { + SetSynchronousHandler(SyncHandler); ErrorExitCode = errorExitCode; - - SetSynchronousHandler(PrintDiagramAndQuit); - SetAsynchronousHandler((context, next, cancellationToken) => - { - if (cancellationToken.IsCancellationRequested) - { - return Task.FromCanceled(cancellationToken); - } - - PrintDiagramAndQuit(context, null); - - return Task.CompletedTask; - }); } internal int ErrorExitCode { get; } - private void PrintDiagramAndQuit(InvocationContext context, ICommandHandler? next) + private void SyncHandler(InvocationContext context) { var parseResult = context.ParseResult; context.Console.Out.WriteLine(parseResult.Diagram()); context.ExitCode = parseResult.Errors.Count == 0 ? 0 : ErrorExitCode; - - // parse directive has a precedence over --help and --version and any command - // we don't invoke next here. } } } diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 43bf77e587..afc9c0703a 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -22,7 +22,6 @@ public class ParseResult private readonly IReadOnlyList _unmatchedTokens; private CompletionContext? _completionContext; private ICommandHandler? _handler; - private bool _useChainedHandler; internal ParseResult( CommandLineConfiguration configuration, @@ -31,15 +30,13 @@ internal ParseResult( List tokens, IReadOnlyList? unmatchedTokens, List? errors, - string? commandLineText, - ICommandHandler? handler, - bool useChainedHandler) + string? commandLineText = null, + ICommandHandler? handler = null) { Configuration = configuration; _rootCommandResult = rootCommandResult; CommandResult = commandResult; _handler = handler; - _useChainedHandler = useChainedHandler; // skip the root command when populating Tokens property if (tokens.Count > 1) @@ -244,18 +241,21 @@ internal ICommandHandler? Handler { get { - _handler ??= CommandResult.Command.Handler; + if (_handler is not null) + { + return _handler; + } - if (_useChainedHandler && _handler is not ChainedCommandHandler) + if (CommandResult.Command is { } command) { - // we can't create it when creating a ParseResult, because some Hosting extensions - // set Command.Handler via middleware, which is executed after parsing - _handler = new ChainedCommandHandler(_rootCommandResult.SymbolResultTree, _handler); + return command.Handler; } - return _handler; + return null; } + set => _handler = value; } + private SymbolResult SymbolToComplete(int? position = null) { var commandResult = CommandResult; diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 43f8e9658b..16242174b7 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -17,8 +17,8 @@ internal sealed class ParseOperation private int _index; private CommandResult _innermostCommandResult; - private bool _isHelpRequested, _useChainedHandler; - private ICommandHandler? _commandHandler; + private bool _isHelpRequested, _isParseRequested; + private ICommandHandler? _handler; public ParseOperation( List tokens, @@ -60,20 +60,20 @@ internal ParseResult Parse() Validate(); } - if (_commandHandler is null) + if (_handler is null) { if (_configuration.ParseErrorReportingExitCode.HasValue && _symbolResultTree.ErrorCount > 0) { - _commandHandler = new AnonymousCommandHandler(ParseErrorResult.Apply); + _handler = new AnonymousCommandHandler(ParseErrorResult.Apply); } else if (_configuration.MaxLevenshteinDistance > 0 && _rootCommandResult.Command.TreatUnmatchedTokensAsErrors && _symbolResultTree.UnmatchedTokens is not null) { - _commandHandler = new AnonymousCommandHandler(TypoCorrection.ProvideSuggestions); + _handler = new AnonymousCommandHandler(TypoCorrection.ProvideSuggestions); } } - return new( + return new ( _configuration, _rootCommandResult, _innermostCommandResult, @@ -81,8 +81,7 @@ internal ParseResult Parse() _symbolResultTree.UnmatchedTokens, _symbolResultTree.Errors, _rawInput, - _commandHandler, - _useChainedHandler); + _handler); } private void ParseSubcommand() @@ -188,15 +187,19 @@ private void ParseOption() if (!_symbolResultTree.TryGetValue(option, out SymbolResult? symbolResult)) { - if (option is HelpOption) + // parse directive has a precedence over --help and --version + if (!_isParseRequested) { - _isHelpRequested = true; + if (option is HelpOption) + { + _isHelpRequested = true; - _commandHandler = new AnonymousCommandHandler(HelpOption.Handler); - } - else if (option is VersionOption) - { - _commandHandler = new AnonymousCommandHandler(VersionOption.Handler); + _handler = new AnonymousCommandHandler(HelpOption.Handler); + } + else if (option is VersionOption) + { + _handler = new AnonymousCommandHandler(VersionOption.Handler); + } } optionResult = new OptionResult( @@ -308,10 +311,6 @@ void ParseDirective() { result = new (directive, token, _symbolResultTree); _symbolResultTree.Add(directive, result); - if (directive.HasHandler) - { - _useChainedHandler = true; - } } ReadOnlySpan withoutBrackets = token.Value.AsSpan(1, token.Value.Length - 2); @@ -320,6 +319,13 @@ void ParseDirective() { result.AddValue(withoutBrackets.Slice(indexOfColon + 1).ToString()); } + + _handler = directive.Handler; + + if (directive is ParseDirective) + { + _isParseRequested = true; + } } } diff --git a/src/System.CommandLine/SuggestDirective.cs b/src/System.CommandLine/SuggestDirective.cs index b9c345deca..c0c445eefb 100644 --- a/src/System.CommandLine/SuggestDirective.cs +++ b/src/System.CommandLine/SuggestDirective.cs @@ -2,7 +2,6 @@ using System.Linq; using System.CommandLine.IO; using System.CommandLine.Parsing; -using System.Threading.Tasks; namespace System.CommandLine { @@ -14,21 +13,10 @@ public sealed class SuggestDirective : Directive { public SuggestDirective() : base("suggest") { - SetSynchronousHandler(ProvideCompletionsAndQuit); - SetAsynchronousHandler((context, next, cancellationToken) => - { - if (cancellationToken.IsCancellationRequested) - { - return Task.FromCanceled(cancellationToken); - } - - ProvideCompletionsAndQuit(context, null); - - return Task.CompletedTask; - }); + SetSynchronousHandler(SyncHandler); } - private void ProvideCompletionsAndQuit(InvocationContext context, ICommandHandler? next) + private void SyncHandler(InvocationContext context) { ParseResult parseResult = context.ParseResult; string? parsedValues = parseResult.FindResultFor(this)!.Values.SingleOrDefault(); From f37d8df8c45fea37f08b2a552b9f6dd09bc19714 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 24 Feb 2023 18:13:00 +0100 Subject: [PATCH 13/13] address code review feedback --- .../DirectiveTests.cs | 43 ++++++------------- .../Parsing/ParseOperation.cs | 2 +- .../Parsing/StringExtensions.cs | 14 +++--- 3 files changed, 22 insertions(+), 37 deletions(-) diff --git a/src/System.CommandLine.Tests/DirectiveTests.cs b/src/System.CommandLine.Tests/DirectiveTests.cs index 951980988f..938985e3c2 100644 --- a/src/System.CommandLine.Tests/DirectiveTests.cs +++ b/src/System.CommandLine.Tests/DirectiveTests.cs @@ -11,15 +11,13 @@ namespace System.CommandLine.Tests public class DirectiveTests { [Fact] - public void Directives_should_not_be_considered_as_unmatched_tokens_when_they_are_enabled() + public void Directives_should_be_considered_as_unmatched_tokens_when_they_are_not_matched() { - RootCommand root = new () { new Option("-y") }; - CommandLineBuilder builder = new (root); - builder.Directives.Add(new ("some")); + Directive directive = new("parse"); - var result = root.Parse($"{RootCommand.ExecutableName} [parse] -y", builder.Build()); + ParseResult result = Parse(new Option("-y"), directive, $"{RootCommand.ExecutableName} [nonExisting] -y"); - result.UnmatchedTokens.Should().BeEmpty(); + result.UnmatchedTokens.Should().ContainSingle("[nonExisting]"); } [Fact] @@ -99,14 +97,19 @@ public void Directives_must_have_a_non_empty_key(string directive) } [Theory] - [InlineData("[par se]")] - [InlineData("[ parse]")] - [InlineData("[parse ]")] - public void Directives_cannot_contain_spaces(string value) + [InlineData("[par se]", "[par", "se]")] + [InlineData("[ parse]", "[", "parse]")] + [InlineData("[parse ]", "[parse", "]")] + public void Directives_cannot_contain_spaces(string value, string firstUnmatchedToken, string secondUnmatchedToken) { Action create = () => new Directive(value); - create.Should().Throw(); + + Directive directive = new("parse"); + ParseResult result = Parse(new Option("-y"), directive, $"{value} -y"); + result.FindResultFor(directive).Should().BeNull(); + + result.UnmatchedTokens.Should().BeEquivalentTo(firstUnmatchedToken, secondUnmatchedToken); } [Fact] @@ -119,24 +122,6 @@ public void When_a_directive_is_specified_more_than_once_then_its_values_are_agg result.FindResultFor(directive).Values.Should().BeEquivalentTo("one", "two"); } - [Fact] - public void When_directives_are_not_enabled_they_are_treated_as_regular_tokens() - { - var config = new CommandLineConfiguration( - new RootCommand - { - new Argument>() - }); - - var result = config.RootCommand.Parse("[hello]", config); - - result.CommandResult - .Tokens - .Select(t => t.Value) - .Should() - .BeEquivalentTo("[hello]"); - } - private static ParseResult Parse(Option option, Directive directive, string commandLine) { RootCommand root = new() { option }; diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 16242174b7..63a7e020b4 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -297,7 +297,7 @@ void ParseDirective() if (token.Symbol is not Directive directive) { - // Directives_should_not_be_considered_as_unmatched_tokens + AddCurrentTokenToUnmatched(); return; } diff --git a/src/System.CommandLine/Parsing/StringExtensions.cs b/src/System.CommandLine/Parsing/StringExtensions.cs index 61654b09b7..32d11bc19c 100644 --- a/src/System.CommandLine/Parsing/StringExtensions.cs +++ b/src/System.CommandLine/Parsing/StringExtensions.cs @@ -82,11 +82,11 @@ internal static void Tokenize( var currentCommand = configuration.RootCommand; var foundDoubleDash = false; - var foundEndOfDirectives = configuration.Directives.Count == 0; + var foundEndOfDirectives = false; var tokenList = new List(args.Count); - var knownTokens = configuration.RootCommand.ValidTokens(configuration); + var knownTokens = configuration.RootCommand.ValidTokens(configuration.Directives); int i = FirstArgumentIsRootCommand(args, configuration.RootCommand, inferRootCommand) ? 0 @@ -185,7 +185,7 @@ configuration.TokenReplacer is { } replacer && if (cmd != configuration.RootCommand) { knownTokens = cmd.ValidTokens( - configuration: null); // config contains Directives, they are allowed only for RootCommand + directives: null); // config contains Directives, they are allowed only for RootCommand } currentCommand = cmd; tokenList.Add(Command(arg, cmd)); @@ -431,15 +431,15 @@ static IEnumerable SplitLine(string line) } } - private static Dictionary ValidTokens(this Command command, CommandLineConfiguration? configuration) + private static Dictionary ValidTokens(this Command command, IReadOnlyList? directives) { Dictionary tokens = new(StringComparer.Ordinal); - if (configuration?.Directives is not null) + if (directives is not null) { - for (int directiveIndex = 0; directiveIndex < configuration.Directives.Count; directiveIndex++) + for (int directiveIndex = 0; directiveIndex < directives.Count; directiveIndex++) { - Directive directive = configuration.Directives[directiveIndex]; + Directive directive = directives[directiveIndex]; tokens.Add( directive.Name, new Token(directive.Name, TokenType.Directive, directive, Token.ImplicitPosition));