From d5d965e5c65b226dee5ef7bdac0829f89ee8fbb5 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 3 Nov 2022 16:50:18 +0100 Subject: [PATCH 1/6] remove CompletionSourceList, expose a ICollection instead: - it has Add and Clear - it has no indexer, but we don't need it fixes #1911 --- ...ommandLine_api_is_not_changed.approved.txt | 15 ++----- src/System.CommandLine/Argument.cs | 8 ++-- .../CompletionSourceExtensions.cs | 6 +-- .../CompletionSourceList.cs | 45 ------------------- 4 files changed, 11 insertions(+), 63 deletions(-) delete mode 100644 src/System.CommandLine/CompletionSourceList.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 bddfec9264..f0699c8638 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 @@ -1,7 +1,7 @@ System.CommandLine public abstract class Argument : Symbol, System.CommandLine.Binding.IValueDescriptor, System.CommandLine.Completions.ICompletionSource public ArgumentArity Arity { get; set; } - public CompletionSourceList Completions { get; } + public System.Collections.Generic.ICollection Completions { get; } public System.Boolean HasDefaultValue { get; } public System.String HelpName { get; set; } public System.Type ValueType { get; } @@ -109,16 +109,9 @@ System.CommandLine .ctor() .ctor(System.String message, System.Exception innerException) public static class CompletionSourceExtensions - public static System.Void Add(this CompletionSourceList completionSources, System.Func> complete) - public static System.Void Add(this CompletionSourceList completionSources, System.CommandLine.Completions.CompletionDelegate complete) - public static System.Void Add(this CompletionSourceList completionSources, System.String[] completions) - public class CompletionSourceList, System.Collections.Generic.IEnumerable, System.Collections.Generic.IReadOnlyCollection, System.Collections.Generic.IReadOnlyList, System.Collections.IEnumerable - .ctor() - public System.Int32 Count { get; } - public System.CommandLine.Completions.ICompletionSource Item { get; } - public System.Void Add(System.CommandLine.Completions.ICompletionSource source) - public System.Void Clear() - public System.Collections.Generic.IEnumerator GetEnumerator() + public static System.Void Add(this System.Collections.Generic.ICollection completionSources, System.Func> complete) + public static System.Void Add(this System.Collections.Generic.ICollection completionSources, System.CommandLine.Completions.CompletionDelegate complete) + public static System.Void Add(this System.Collections.Generic.ICollection completionSources, System.String[] completions) 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) diff --git a/src/System.CommandLine/Argument.cs b/src/System.CommandLine/Argument.cs index 8cdce978a8..ae4c5206b2 100644 --- a/src/System.CommandLine/Argument.cs +++ b/src/System.CommandLine/Argument.cs @@ -17,7 +17,7 @@ public abstract class Argument : Symbol, IValueDescriptor private Func? _defaultValueFactory; private ArgumentArity _arity; private TryConvertArgument? _convertArguments; - private CompletionSourceList? _completions = null; + private List? _completions = null; private List>? _validators = null; /// @@ -72,10 +72,10 @@ internal TryConvertArgument? ConvertArguments } /// - /// Gets the list of completion sources for the argument. + /// Gets the collection of completion sources for the argument. /// - public CompletionSourceList Completions => - _completions ??= new CompletionSourceList + public ICollection Completions => + _completions ??= new () { CompletionSource.ForType(ValueType) }; diff --git a/src/System.CommandLine/CompletionSourceExtensions.cs b/src/System.CommandLine/CompletionSourceExtensions.cs index 73263d0d86..07746c06dd 100644 --- a/src/System.CommandLine/CompletionSourceExtensions.cs +++ b/src/System.CommandLine/CompletionSourceExtensions.cs @@ -18,7 +18,7 @@ public static class CompletionSourceExtensions /// The list of completion sources to add to. /// The delegate to be called when calculating completions. public static void Add( - this CompletionSourceList completionSources, + this ICollection completionSources, Func> complete) { if (completionSources is null) @@ -40,7 +40,7 @@ public static void Add( /// The list of completion sources to add to. /// The delegate to be called when calculating completions. public static void Add( - this CompletionSourceList completionSources, + this ICollection completionSources, CompletionDelegate complete) { if (completionSources is null) @@ -62,7 +62,7 @@ public static void Add( /// The list of completion sources to add to. /// A list of strings to be suggested for command line completions. public static void Add( - this CompletionSourceList completionSources, + this ICollection completionSources, params string[] completions) { if (completionSources is null) diff --git a/src/System.CommandLine/CompletionSourceList.cs b/src/System.CommandLine/CompletionSourceList.cs deleted file mode 100644 index 4230c091ad..0000000000 --- a/src/System.CommandLine/CompletionSourceList.cs +++ /dev/null @@ -1,45 +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.CommandLine.Completions; - -namespace System.CommandLine -{ - /// - /// A list of completion sources to be used when providing completions for completion. - /// - public class CompletionSourceList : IReadOnlyList - { - private readonly List _sources = new(); - - /// - /// Adds a completion source to the list. - /// - /// The source to add. - public void Add(ICompletionSource source) - { - _sources.Add(source); - } - - /// - public IEnumerator GetEnumerator() => _sources.GetEnumerator(); - - IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); - - /// - /// Clears the completion sources. - /// - public void Clear() - { - _sources.Clear(); - } - - /// - public int Count => _sources.Count; - - /// - public ICompletionSource this[int index] => _sources[index]; - } -} \ No newline at end of file From f99bf0fd9e0b704a4edffac085d621b556265465 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 3 Nov 2022 17:42:56 +0100 Subject: [PATCH 2/6] replace CompletionDelegate with a Func, fixes #1927 --- ...stem_CommandLine_api_is_not_changed.approved.txt | 11 +++-------- src/System.CommandLine/ArgumentExtensions.cs | 6 +++--- .../CompletionSourceExtensions.cs | 4 ++-- .../Completions/AnonymousCompletionSource.cs | 4 ++-- .../Completions/CompletionDelegate.cs | 13 ------------- src/System.CommandLine/OptionExtensions.cs | 6 +++--- 6 files changed, 13 insertions(+), 31 deletions(-) delete mode 100644 src/System.CommandLine/Completions/CompletionDelegate.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 f0699c8638..6d73b05f80 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 @@ -35,7 +35,7 @@ System.CommandLine public static class ArgumentExtensions public static TArgument AddCompletions(this TArgument argument, System.String[] values) public static TArgument AddCompletions(this TArgument argument, System.Func> complete) - public static TArgument AddCompletions(this TArgument argument, System.CommandLine.Completions.CompletionDelegate complete) + public static TArgument AddCompletions(this TArgument argument, System.Func> complete) public static Argument ExistingOnly(this Argument argument) public static Argument ExistingOnly(this Argument argument) public static Argument ExistingOnly(this Argument argument) @@ -110,7 +110,7 @@ System.CommandLine .ctor(System.String message, System.Exception innerException) public static class CompletionSourceExtensions public static System.Void Add(this System.Collections.Generic.ICollection completionSources, System.Func> complete) - public static System.Void Add(this System.Collections.Generic.ICollection completionSources, System.CommandLine.Completions.CompletionDelegate complete) + public static System.Void Add(this System.Collections.Generic.ICollection completionSources, System.Func> complete) public static System.Void Add(this System.Collections.Generic.ICollection completionSources, System.String[] completions) public static class ConsoleExtensions public static System.Void Write(this IConsole console, System.String value) @@ -210,7 +210,7 @@ System.CommandLine public static class OptionExtensions public static TOption AddCompletions(this TOption option, System.String[] values) public static TOption AddCompletions(this TOption option, System.Func> complete) - public static TOption AddCompletions(this TOption option, System.CommandLine.Completions.CompletionDelegate complete) + public static TOption AddCompletions(this TOption option, System.Func> complete) public static Option ExistingOnly(this Option option) public static Option ExistingOnly(this Option option) public static Option ExistingOnly(this Option option) @@ -277,11 +277,6 @@ System.CommandLine.Completions public abstract class CompletionContext public System.CommandLine.ParseResult ParseResult { get; } public System.String WordToComplete { get; } - public delegate CompletionDelegate : System.MulticastDelegate, System.ICloneable, System.Runtime.Serialization.ISerializable - .ctor(System.Object object, System.IntPtr method) - public System.IAsyncResult BeginInvoke(CompletionContext context, System.AsyncCallback callback, System.Object object) - public System.Collections.Generic.IEnumerable EndInvoke(System.IAsyncResult result) - public System.Collections.Generic.IEnumerable Invoke(CompletionContext context) public class CompletionItem .ctor(System.String label, System.String kind = Value, System.String sortText = null, System.String insertText = null, System.String documentation = null, System.String detail = null) public System.String Detail { get; } diff --git a/src/System.CommandLine/ArgumentExtensions.cs b/src/System.CommandLine/ArgumentExtensions.cs index e43d286cd1..af69cba401 100644 --- a/src/System.CommandLine/ArgumentExtensions.cs +++ b/src/System.CommandLine/ArgumentExtensions.cs @@ -35,7 +35,7 @@ public static TArgument AddCompletions( /// /// The type of the argument. /// The argument for which to add completions. - /// A that will be called to provide completions. + /// A function that will be called to provide completions. /// The option being extended. public static TArgument AddCompletions( this TArgument argument, @@ -52,11 +52,11 @@ public static TArgument AddCompletions( /// /// The type of the argument. /// The argument for which to add completions. - /// A that will be called to provide completions. + /// A function that will be called to provide completions. /// The configured argument. public static TArgument AddCompletions( this TArgument argument, - CompletionDelegate complete) + Func> complete) where TArgument : Argument { argument.Completions.Add(complete); diff --git a/src/System.CommandLine/CompletionSourceExtensions.cs b/src/System.CommandLine/CompletionSourceExtensions.cs index 07746c06dd..5c50e34748 100644 --- a/src/System.CommandLine/CompletionSourceExtensions.cs +++ b/src/System.CommandLine/CompletionSourceExtensions.cs @@ -38,10 +38,10 @@ public static void Add( /// Adds a completion source using a delegate. /// /// The list of completion sources to add to. - /// The delegate to be called when calculating completions. + /// The function to be called when calculating completions. public static void Add( this ICollection completionSources, - CompletionDelegate complete) + Func> complete) { if (completionSources is null) { diff --git a/src/System.CommandLine/Completions/AnonymousCompletionSource.cs b/src/System.CommandLine/Completions/AnonymousCompletionSource.cs index 7f8237c8be..ab691e5f75 100644 --- a/src/System.CommandLine/Completions/AnonymousCompletionSource.cs +++ b/src/System.CommandLine/Completions/AnonymousCompletionSource.cs @@ -8,9 +8,9 @@ namespace System.CommandLine.Completions { internal class AnonymousCompletionSource : ICompletionSource { - private readonly CompletionDelegate _complete; + private readonly Func> _complete; - public AnonymousCompletionSource(CompletionDelegate complete) + public AnonymousCompletionSource(Func> complete) { _complete = complete ?? throw new ArgumentNullException(nameof(complete)); } diff --git a/src/System.CommandLine/Completions/CompletionDelegate.cs b/src/System.CommandLine/Completions/CompletionDelegate.cs deleted file mode 100644 index ca5d3b499b..0000000000 --- a/src/System.CommandLine/Completions/CompletionDelegate.cs +++ /dev/null @@ -1,13 +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.Generic; - -namespace System.CommandLine.Completions -{ - /// - /// Provides command line completion. - /// - /// A list of completions. - public delegate IEnumerable CompletionDelegate(CompletionContext context); -} \ No newline at end of file diff --git a/src/System.CommandLine/OptionExtensions.cs b/src/System.CommandLine/OptionExtensions.cs index d27166f2f4..8eb72c1288 100644 --- a/src/System.CommandLine/OptionExtensions.cs +++ b/src/System.CommandLine/OptionExtensions.cs @@ -55,7 +55,7 @@ public static TOption AddCompletions( /// /// The type of the option. /// The option for which to add completions. - /// A that will be called to provide completions. + /// A function that will be called to provide completions. /// The option being extended. public static TOption AddCompletions( this TOption option, @@ -72,11 +72,11 @@ public static TOption AddCompletions( /// /// The type of the option. /// The option for which to add completions. - /// A that will be called to provide completions. + /// A function that will be called to provide completions. /// The option being extended. public static TOption AddCompletions( this TOption option, - CompletionDelegate complete) + Func> complete) where TOption : Option { option.Argument.Completions.Add(complete); From 09b627a1c7cef9bc62b68ffecfd299e38b807081 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 3 Nov 2022 21:38:56 +0100 Subject: [PATCH 3/6] remove DirectiveCollection, use dictionary instead, fixes #1912 --- ...tem_CommandLine_api_is_not_changed.approved.txt | 2 +- .../CommandLine/Perf_Parser_ParseResult.cs | 2 +- .../DirectiveConfigurationExtensions.cs | 2 +- .../CommandLineBuilderExtensions.cs | 4 ++-- src/System.CommandLine.Tests/DirectiveTests.cs | 14 +++++++------- .../Builder/CommandLineBuilderExtensions.cs | 6 +++--- src/System.CommandLine/ParseResult.cs | 4 ++-- .../Parsing/ParseResultVisitor.cs | 14 ++++++++++++-- 8 files changed, 29 insertions(+), 19 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 6d73b05f80..e7ca6e3736 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 @@ -222,7 +222,7 @@ System.CommandLine public static ParseResult Parse(this Option option, System.String[] args) public class ParseResult public System.CommandLine.Parsing.CommandResult CommandResult { get; } - public DirectiveCollection Directives { 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 0c2bc1b896..be4be31cbe 100644 --- a/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs +++ b/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs @@ -43,7 +43,7 @@ public IEnumerable GenerateTestParseResults() [Benchmark] [ArgumentsSource(nameof(GenerateTestInputs))] - public DirectiveCollection ParseResult_Directives(string input) + public IReadOnlyDictionary> ParseResult_Directives(string input) => _testParser.Parse(input).Directives; [Benchmark] diff --git a/src/System.CommandLine.Hosting/DirectiveConfigurationExtensions.cs b/src/System.CommandLine.Hosting/DirectiveConfigurationExtensions.cs index 638bd6939e..a09f629f24 100644 --- a/src/System.CommandLine.Hosting/DirectiveConfigurationExtensions.cs +++ b/src/System.CommandLine.Hosting/DirectiveConfigurationExtensions.cs @@ -16,7 +16,7 @@ public static IConfigurationBuilder AddCommandLineDirectives( if (name is null) throw new ArgumentNullException(nameof(name)); - if (!commandline.Directives.TryGetValues(name, out var directives)) + if (!commandline.Directives.TryGetValue(name, out var directives)) return config; var kvpSeparator = new[] { '=' }; diff --git a/src/System.CommandLine.Rendering/CommandLineBuilderExtensions.cs b/src/System.CommandLine.Rendering/CommandLineBuilderExtensions.cs index ab64436f2e..185d24c9f8 100644 --- a/src/System.CommandLine.Rendering/CommandLineBuilderExtensions.cs +++ b/src/System.CommandLine.Rendering/CommandLineBuilderExtensions.cs @@ -28,7 +28,7 @@ public static CommandLineBuilder UseAnsiTerminalWhenAvailable( internal static bool PreferVirtualTerminal( this BindingContext context) { - if (context.ParseResult.Directives.TryGetValues( + if (context.ParseResult.Directives.TryGetValue( "enable-vt", out var trueOrFalse)) { @@ -45,7 +45,7 @@ internal static bool PreferVirtualTerminal( public static OutputMode OutputMode(this BindingContext context) { - if (context.ParseResult.Directives.TryGetValues( + if (context.ParseResult.Directives.TryGetValue( "output", out var modeString) && Enum.TryParse( diff --git a/src/System.CommandLine.Tests/DirectiveTests.cs b/src/System.CommandLine.Tests/DirectiveTests.cs index 9bddbe564b..0a6b5e8bf5 100644 --- a/src/System.CommandLine.Tests/DirectiveTests.cs +++ b/src/System.CommandLine.Tests/DirectiveTests.cs @@ -28,7 +28,7 @@ public void Raw_tokens_still_hold_directives() var result = option.Parse("[parse] -y"); - result.Directives.Contains("parse").Should().BeTrue(); + result.Directives.ContainsKey("parse").Should().BeTrue(); result.Tokens.Should().Contain(t => t.Value == "[parse]"); } @@ -39,7 +39,7 @@ public void Directives_should_parse_into_the_directives_collection() var result = option.Parse("[parse] -y"); - result.Directives.Contains("parse").Should().BeTrue(); + result.Directives.ContainsKey("parse").Should().BeTrue(); } [Fact] @@ -49,8 +49,8 @@ public void Multiple_directives_are_allowed() var result = option.Parse("[parse] [suggest] -y"); - result.Directives.Contains("parse").Should().BeTrue(); - result.Directives.Contains("suggest").Should().BeTrue(); + result.Directives.ContainsKey("parse").Should().BeTrue(); + result.Directives.ContainsKey("suggest").Should().BeTrue(); } [Fact] @@ -76,7 +76,7 @@ public void Directives_can_have_a_value_which_is_everything_after_the_first_colo var result = option.Parse($"{directive} -y"); - result.Directives.TryGetValues(expectedKey, out var values).Should().BeTrue(); + result.Directives.TryGetValue(expectedKey, out var values).Should().BeTrue(); values.Should().BeEquivalentTo(expectedValue); } @@ -87,7 +87,7 @@ public void Directives_without_a_value_specified_have_a_value_of_empty_string() var result = option.Parse("[parse] -y"); - result.Directives.TryGetValues("parse", out var values).Should().BeTrue(); + result.Directives.TryGetValue("parse", out var values).Should().BeTrue(); values.Should().BeEmpty(); } @@ -124,7 +124,7 @@ public void When_a_directive_is_specified_more_than_once_then_its_values_are_agg var result = option.Parse("[directive:one] [directive:two] -a"); - result.Directives.TryGetValues("directive", out var values).Should().BeTrue(); + result.Directives.TryGetValue("directive", out var values).Should().BeTrue(); values.Should().BeEquivalentTo("one", "two"); } diff --git a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs index 0b60fb1329..bccb9af67d 100644 --- a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs +++ b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs @@ -251,7 +251,7 @@ public static CommandLineBuilder UseEnvironmentVariableDirective( { builder.AddMiddleware((context, next) => { - if (context.ParseResult.Directives.TryGetValues("env", out var keyValuePairs)) + if (context.ParseResult.Directives.TryGetValue("env", out var keyValuePairs)) { for (var i = 0; i < keyValuePairs.Count; i++) { @@ -498,7 +498,7 @@ public static CommandLineBuilder UseParseDirective( { builder.AddMiddleware(async (context, next) => { - if (context.ParseResult.Directives.Contains("parse")) + if (context.ParseResult.Directives.ContainsKey("parse")) { context.InvocationResult = new ParseDirectiveResult(errorExitCode); } @@ -547,7 +547,7 @@ public static CommandLineBuilder UseSuggestDirective( { builder.AddMiddleware(async (context, next) => { - if (context.ParseResult.Directives.TryGetValues("suggest", out var values)) + if (context.ParseResult.Directives.TryGetValue("suggest", out var values)) { int position; diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 1a9eba1aef..624c2f0a63 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -23,7 +23,7 @@ internal ParseResult( Parser parser, RootCommandResult rootCommandResult, CommandResult commandResult, - DirectiveCollection directives, + IReadOnlyDictionary> directives, TokenizeResult tokenizeResult, IReadOnlyList? unmatchedTokens, List? errors, @@ -99,7 +99,7 @@ internal ParseResult( /// Gets the directives found while parsing command line input. /// /// If is set to , then this collection will be empty. - public DirectiveCollection Directives { get; } + public IReadOnlyDictionary> Directives { get; } /// /// Gets the tokens identified while parsing command line input. diff --git a/src/System.CommandLine/Parsing/ParseResultVisitor.cs b/src/System.CommandLine/Parsing/ParseResultVisitor.cs index 2ed41340ed..97834fe642 100644 --- a/src/System.CommandLine/Parsing/ParseResultVisitor.cs +++ b/src/System.CommandLine/Parsing/ParseResultVisitor.cs @@ -14,7 +14,7 @@ internal sealed class ParseResultVisitor private readonly TokenizeResult _tokenizeResult; private readonly string? _rawInput; - private readonly DirectiveCollection _directives = new(); + private readonly Dictionary> _directives = new(); private List? _unmatchedTokens; private readonly List _errors; @@ -222,7 +222,17 @@ private void VisitOptionArgumentNode( private void VisitDirectiveNode(DirectiveNode directiveNode) { - _directives.Add(directiveNode.Name, directiveNode.Value); + if (!_directives.TryGetValue(directiveNode.Name, out var values)) + { + values = new List(); + + _directives.Add(directiveNode.Name, values); + } + + if (directiveNode.Value is not null) + { + ((List)values).Add(directiveNode.Value); + } } private void Stop() From 996aa5305166b837c759764704427e1fc0c60f22 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 4 Nov 2022 16:15:30 +0100 Subject: [PATCH 4/6] replace HelpSectionDelegate with Action, fixes #1932 --- ...ommandLine_api_is_not_changed.approved.txt | 21 +++++++------------ src/System.CommandLine.Tests/UseHelpTests.cs | 8 +++---- .../Help/HelpBuilder.Default.cs | 14 ++++++------- src/System.CommandLine/Help/HelpBuilder.cs | 6 +++--- .../Help/HelpSectionDelegate.cs | 11 ---------- 5 files changed, 22 insertions(+), 38 deletions(-) delete mode 100644 src/System.CommandLine/Help/HelpSectionDelegate.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 e7ca6e3736..9a11f096da 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 @@ -301,24 +301,24 @@ System.CommandLine.Help .ctor(System.CommandLine.LocalizationResources localizationResources, System.Int32 maxWidth = 2147483647) public System.CommandLine.LocalizationResources LocalizationResources { get; } public System.Int32 MaxWidth { get; } - public System.Void CustomizeLayout(System.Func> getLayout) + public System.Void CustomizeLayout(System.Func>> getLayout) public System.Void CustomizeSymbol(System.CommandLine.Symbol symbol, System.Func firstColumnText = null, System.Func secondColumnText = null, System.Func defaultValue = null) public TwoColumnHelpRow GetTwoColumnRow(System.CommandLine.Symbol symbol, HelpContext context) public System.Void Write(HelpContext context) public System.Void WriteColumns(System.Collections.Generic.IReadOnlyList items, HelpContext context) static class Default - public static HelpSectionDelegate AdditionalArgumentsSection() - public static HelpSectionDelegate CommandArgumentsSection() - public static HelpSectionDelegate CommandUsageSection() + public static System.Action AdditionalArgumentsSection() + public static System.Action CommandArgumentsSection() + public static System.Action CommandUsageSection() public static System.String GetArgumentDefaultValue(System.CommandLine.Argument argument) public static System.String GetArgumentDescription(System.CommandLine.Argument argument) public static System.String GetArgumentUsageLabel(System.CommandLine.Argument argument) public static System.String GetIdentifierSymbolDescription(System.CommandLine.IdentifierSymbol symbol) public static System.String GetIdentifierSymbolUsageLabel(System.CommandLine.IdentifierSymbol symbol, HelpContext context) - public static System.Collections.Generic.IEnumerable GetLayout() - public static HelpSectionDelegate OptionsSection() - public static HelpSectionDelegate SubcommandsSection() - public static HelpSectionDelegate SynopsisSection() + public static System.Collections.Generic.IEnumerable> GetLayout() + public static System.Action OptionsSection() + public static System.Action SubcommandsSection() + public static System.Action SynopsisSection() public static class HelpBuilderExtensions public static System.Void CustomizeSymbol(this HelpBuilder builder, System.CommandLine.Symbol symbol, System.String firstColumnText = null, System.String secondColumnText = null, System.String defaultValue = null) public static System.Void Write(this HelpBuilder helpBuilder, System.CommandLine.Command command, System.IO.TextWriter writer) @@ -328,11 +328,6 @@ System.CommandLine.Help public HelpBuilder HelpBuilder { get; } public System.IO.TextWriter Output { get; } public System.CommandLine.ParseResult ParseResult { get; } - public delegate HelpSectionDelegate : System.MulticastDelegate, System.ICloneable, System.Runtime.Serialization.ISerializable - .ctor(System.Object object, System.IntPtr method) - public System.IAsyncResult BeginInvoke(HelpContext context, System.AsyncCallback callback, System.Object object) - public System.Void EndInvoke(System.IAsyncResult result) - public System.Void Invoke(HelpContext context) public class TwoColumnHelpRow, System.IEquatable .ctor(System.String firstColumnText, System.String secondColumnText) public System.String FirstColumnText { get; } diff --git a/src/System.CommandLine.Tests/UseHelpTests.cs b/src/System.CommandLine.Tests/UseHelpTests.cs index ad1e83c7f4..8e9e4f9280 100644 --- a/src/System.CommandLine.Tests/UseHelpTests.cs +++ b/src/System.CommandLine.Tests/UseHelpTests.cs @@ -301,7 +301,7 @@ public void Help_sections_can_be_replaced() console.Out.ToString().Should().Be($"one{NewLine}{NewLine}two{NewLine}{NewLine}three{NewLine}{NewLine}{NewLine}"); - IEnumerable CustomLayout(HelpContext _) + IEnumerable> CustomLayout(HelpContext _) { yield return ctx => ctx.Output.WriteLine("one"); yield return ctx => ctx.Output.WriteLine("two"); @@ -327,7 +327,7 @@ public void Help_sections_can_be_supplemented() output.Should().Be(expected); - IEnumerable CustomLayout(HelpContext _) + IEnumerable> CustomLayout(HelpContext _) { yield return ctx => ctx.Output.WriteLine("first"); @@ -358,7 +358,7 @@ public void Layout_can_be_composed_dynamically_based_on_context() .CustomizeLayout(c => c.Command == commandWithTypicalHelp ? HelpBuilder.Default.GetLayout() - : new HelpSectionDelegate[] + : new Action[] { c => c.Output.WriteLine("Custom layout!") } @@ -417,7 +417,7 @@ public void Help_customized_sections_can_be_wrapped() string result = console.Out.ToString(); result.Should().Be($" 123 123{NewLine} 456 456{NewLine} 78 789{NewLine} 0{NewLine}{NewLine}{NewLine}"); - IEnumerable CustomLayout(HelpContext _) + IEnumerable> CustomLayout(HelpContext _) { yield return ctx => ctx.HelpBuilder.WriteColumns(new[] { new TwoColumnHelpRow("12345678", "1234567890") }, ctx); } diff --git a/src/System.CommandLine/Help/HelpBuilder.Default.cs b/src/System.CommandLine/Help/HelpBuilder.Default.cs index 67f19f5d84..83f5ba4564 100644 --- a/src/System.CommandLine/Help/HelpBuilder.Default.cs +++ b/src/System.CommandLine/Help/HelpBuilder.Default.cs @@ -141,7 +141,7 @@ public static string GetIdentifierSymbolUsageLabel(IdentifierSymbol symbol, Help /// /// Gets the default sections to be written for command line help. /// - public static IEnumerable GetLayout() + public static IEnumerable> GetLayout() { yield return SynopsisSection(); yield return CommandUsageSection(); @@ -154,7 +154,7 @@ public static IEnumerable GetLayout() /// /// Writes a help section describing a command's synopsis. /// - public static HelpSectionDelegate SynopsisSection() => + public static Action SynopsisSection() => ctx => { ctx.HelpBuilder.WriteHeading(ctx.HelpBuilder.LocalizationResources.HelpDescriptionTitle(), ctx.Command.Description, ctx.Output); @@ -163,7 +163,7 @@ public static HelpSectionDelegate SynopsisSection() => /// /// Writes a help section describing a command's usage. /// - public static HelpSectionDelegate CommandUsageSection() => + public static Action CommandUsageSection() => ctx => { ctx.HelpBuilder.WriteHeading(ctx.HelpBuilder.LocalizationResources.HelpUsageTitle(), ctx.HelpBuilder.GetUsage(ctx.Command), ctx.Output); @@ -172,7 +172,7 @@ public static HelpSectionDelegate CommandUsageSection() => /// /// Writes a help section describing a command's arguments. /// - public static HelpSectionDelegate CommandArgumentsSection() => + public static Action CommandArgumentsSection() => ctx => { TwoColumnHelpRow[] commandArguments = ctx.HelpBuilder.GetCommandArgumentRows(ctx.Command, ctx).ToArray(); @@ -190,13 +190,13 @@ public static HelpSectionDelegate CommandArgumentsSection() => /// /// Writes a help section describing a command's subcommands. /// - public static HelpSectionDelegate SubcommandsSection() => + public static Action SubcommandsSection() => ctx => ctx.HelpBuilder.WriteSubcommands(ctx); /// /// Writes a help section describing a command's options. /// - public static HelpSectionDelegate OptionsSection() => + public static Action OptionsSection() => ctx => { // by making this logic more complex, we were able to get some nice perf wins elsewhere @@ -249,7 +249,7 @@ public static HelpSectionDelegate OptionsSection() => /// /// Writes a help section describing a command's additional arguments, typically shown only when is set to . /// - public static HelpSectionDelegate AdditionalArgumentsSection() => + public static Action AdditionalArgumentsSection() => ctx => ctx.HelpBuilder.WriteAdditionalArguments(ctx); } } \ No newline at end of file diff --git a/src/System.CommandLine/Help/HelpBuilder.cs b/src/System.CommandLine/Help/HelpBuilder.cs index 6b83e5019a..a1fde17674 100644 --- a/src/System.CommandLine/Help/HelpBuilder.cs +++ b/src/System.CommandLine/Help/HelpBuilder.cs @@ -15,7 +15,7 @@ public partial class HelpBuilder private const string Indent = " "; private Dictionary? _customizationsBySymbol; - private Func>? _getLayout; + private Func>>? _getLayout; /// Resources used to localize the help output. /// The maximum width in characters after which help output is wrapped. @@ -103,7 +103,7 @@ public void CustomizeSymbol(Symbol symbol, /// Customizes the help sections that will be displayed. /// /// A delegate that returns the sections in the order in which they should be written. - public void CustomizeLayout(Func> getLayout) + public void CustomizeLayout(Func>> getLayout) { _getLayout = getLayout ?? throw new ArgumentNullException(nameof(getLayout)); } @@ -330,7 +330,7 @@ bool IsOptional(Argument argument) => argument.Arity.MinimumNumberOfValues == 0; } - private IEnumerable GetLayout(HelpContext context) + private IEnumerable> GetLayout(HelpContext context) { if (_getLayout is null) { diff --git a/src/System.CommandLine/Help/HelpSectionDelegate.cs b/src/System.CommandLine/Help/HelpSectionDelegate.cs deleted file mode 100644 index e69a417925..0000000000 --- a/src/System.CommandLine/Help/HelpSectionDelegate.cs +++ /dev/null @@ -1,11 +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. - -namespace System.CommandLine.Help -{ - /// - /// Specifies help formatting behavior for a section of command line help. - /// - /// if anything was written; otherwise, . - public delegate void HelpSectionDelegate(HelpContext context); -} \ No newline at end of file From 869b25060b0fafac73a03d856e9d4740b076ea98 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 4 Nov 2022 17:27:58 +0100 Subject: [PATCH 5/6] replace ParseArgument with Func, fixes #1939 --- ...stem_CommandLine_api_is_not_changed.approved.txt | 13 ++++--------- src/System.CommandLine/Argument{T}.cs | 4 ++-- src/System.CommandLine/Option{T}.cs | 4 ++-- src/System.CommandLine/Parsing/ParseArgument{T}.cs | 13 ------------- 4 files changed, 8 insertions(+), 26 deletions(-) delete mode 100644 src/System.CommandLine/Parsing/ParseArgument{T}.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 9a11f096da..c5c137291d 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 @@ -17,8 +17,8 @@ System.CommandLine .ctor(System.String name, System.String description = null) .ctor(System.String name, Func getDefaultValue, System.String description = null) .ctor(Func getDefaultValue) - .ctor(System.String name, ParseArgument parse, System.Boolean isDefault = False, System.String description = null) - .ctor(ParseArgument parse, System.Boolean isDefault = False) + .ctor(System.String name, Func parse, System.Boolean isDefault = False, System.String description = null) + .ctor(Func parse, System.Boolean isDefault = False) public System.Type ValueType { get; } public struct ArgumentArity : System.ValueType, System.IEquatable public static ArgumentArity ExactlyOne { get; } @@ -202,8 +202,8 @@ System.CommandLine public class Option : Option, IValueDescriptor, System.CommandLine.Binding.IValueDescriptor, System.CommandLine.Completions.ICompletionSource .ctor(System.String name, System.String description = null) .ctor(System.String[] aliases, System.String description = null) - .ctor(System.String name, ParseArgument parseArgument, System.Boolean isDefault = False, System.String description = null) - .ctor(System.String[] aliases, ParseArgument parseArgument, System.Boolean isDefault = False, System.String description = null) + .ctor(System.String name, Func parseArgument, System.Boolean isDefault = False, System.String description = null) + .ctor(System.String[] aliases, Func parseArgument, System.Boolean isDefault = False, System.String description = null) .ctor(System.String name, Func getDefaultValue, System.String description = null) .ctor(System.String[] aliases, Func getDefaultValue, System.String description = null) public ArgumentArity Arity { get; set; } @@ -417,11 +417,6 @@ System.CommandLine.Parsing public Token Token { get; } public System.Object GetValueOrDefault() public T GetValueOrDefault() - public delegate ParseArgument : System.MulticastDelegate, System.ICloneable, System.Runtime.Serialization.ISerializable - .ctor(System.Object object, System.IntPtr method) - public System.IAsyncResult BeginInvoke(ArgumentResult result, System.AsyncCallback callback, System.Object object) - public T EndInvoke(System.IAsyncResult result) - public T Invoke(ArgumentResult result) public class ParseError public System.String Message { get; } public SymbolResult SymbolResult { get; } diff --git a/src/System.CommandLine/Argument{T}.cs b/src/System.CommandLine/Argument{T}.cs index 1746281d69..51321274c4 100644 --- a/src/System.CommandLine/Argument{T}.cs +++ b/src/System.CommandLine/Argument{T}.cs @@ -70,7 +70,7 @@ public Argument(Func getDefaultValue) : this() /// Thrown when is null. public Argument( string? name, - ParseArgument parse, + Func parse, bool isDefault = false, string? description = null) : this(name, description) { @@ -108,7 +108,7 @@ public Argument( /// /// A custom argument parser. /// to use the result as default value. - public Argument(ParseArgument parse, bool isDefault = false) : this(null!, parse, isDefault) + public Argument(Func parse, bool isDefault = false) : this(null!, parse, isDefault) { } diff --git a/src/System.CommandLine/Option{T}.cs b/src/System.CommandLine/Option{T}.cs index 5b049a42f0..a67e56e2e5 100644 --- a/src/System.CommandLine/Option{T}.cs +++ b/src/System.CommandLine/Option{T}.cs @@ -27,7 +27,7 @@ public Option( /// public Option( string name, - ParseArgument parseArgument, + Func parseArgument, bool isDefault = false, string? description = null) : base(name, description, @@ -37,7 +37,7 @@ public Option( /// public Option( string[] aliases, - ParseArgument parseArgument, + Func parseArgument, bool isDefault = false, string? description = null) : base(aliases, description, new Argument(parseArgument ?? throw new ArgumentNullException(nameof(parseArgument)), isDefault)) diff --git a/src/System.CommandLine/Parsing/ParseArgument{T}.cs b/src/System.CommandLine/Parsing/ParseArgument{T}.cs deleted file mode 100644 index 5a4f59c2b4..0000000000 --- a/src/System.CommandLine/Parsing/ParseArgument{T}.cs +++ /dev/null @@ -1,13 +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. - -namespace System.CommandLine.Parsing; - -/// -/// Performs custom parsing of an argument. -/// -/// The type which the argument is to be parsed as. -/// The argument result. -/// The parsed value. -/// Validation errors can be returned by setting . -public delegate T ParseArgument(ArgumentResult result); \ No newline at end of file From bc3146830eb47073af3ad40cde8171eb6cdf1a56 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 4 Nov 2022 18:07:54 +0100 Subject: [PATCH 6/6] Replace ValidateSymbolResult delegate with Action, fixes #1944 --- ...stem_CommandLine_api_is_not_changed.approved.txt | 11 +++-------- src/System.CommandLine/Argument.cs | 10 +++++----- src/System.CommandLine/Command.cs | 10 +++++----- src/System.CommandLine/Option.cs | 8 ++++---- .../Parsing/ValidateSymbolResult.cs | 13 ------------- 5 files changed, 17 insertions(+), 35 deletions(-) delete mode 100644 src/System.CommandLine/Parsing/ValidateSymbolResult.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 c5c137291d..f325f26377 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 @@ -5,7 +5,7 @@ System.CommandLine public System.Boolean HasDefaultValue { get; } public System.String HelpName { get; set; } public System.Type ValueType { get; } - public System.Void AddValidator(System.CommandLine.Parsing.ValidateSymbolResult validate) + public System.Void AddValidator(System.Action validate) public System.Collections.Generic.IEnumerable GetCompletions(System.CommandLine.Completions.CompletionContext context) public System.Object GetDefaultValue() public System.Void SetDefaultValue(System.Object value) @@ -60,7 +60,7 @@ System.CommandLine public System.Void AddCommand(Command command) public System.Void AddGlobalOption(Option option) public System.Void AddOption(Option option) - public System.Void AddValidator(System.CommandLine.Parsing.ValidateSymbolResult validate) + public System.Void AddValidator(System.Action validate) public System.Collections.Generic.IEnumerable GetCompletions(System.CommandLine.Completions.CompletionContext context) public System.Collections.Generic.IEnumerator GetEnumerator() public static class CommandExtensions @@ -194,7 +194,7 @@ System.CommandLine public ArgumentArity Arity { get; set; } public System.Boolean IsRequired { get; set; } public System.Type ValueType { get; } - public System.Void AddValidator(System.CommandLine.Parsing.ValidateSymbolResult validate) + public System.Void AddValidator(System.Action validate) public System.Collections.Generic.IEnumerable GetCompletions(System.CommandLine.Completions.CompletionContext context) public System.Boolean HasAliasIgnoringPrefix(System.String alias) public System.Void SetDefaultValue(System.Object value) @@ -474,8 +474,3 @@ System.CommandLine.Parsing public System.IAsyncResult BeginInvoke(System.String tokenToReplace, ref System.Collections.Generic.IReadOnlyList replacementTokens, ref System.String& errorMessage, System.AsyncCallback callback, System.Object object) public System.Boolean EndInvoke(ref System.Collections.Generic.IReadOnlyList replacementTokens, ref System.String& errorMessage, System.IAsyncResult result) public System.Boolean Invoke(System.String tokenToReplace, ref System.Collections.Generic.IReadOnlyList replacementTokens, ref System.String& errorMessage) - public delegate ValidateSymbolResult : System.MulticastDelegate, System.ICloneable, System.Runtime.Serialization.ISerializable - .ctor(System.Object object, System.IntPtr method) - public System.IAsyncResult BeginInvoke(T symbolResult, System.AsyncCallback callback, System.Object object) - public System.Void EndInvoke(System.IAsyncResult result) - public System.Void Invoke(T symbolResult) diff --git a/src/System.CommandLine/Argument.cs b/src/System.CommandLine/Argument.cs index ae4c5206b2..a5466ec8eb 100644 --- a/src/System.CommandLine/Argument.cs +++ b/src/System.CommandLine/Argument.cs @@ -18,7 +18,7 @@ public abstract class Argument : Symbol, IValueDescriptor private ArgumentArity _arity; private TryConvertArgument? _convertArguments; private List? _completions = null; - private List>? _validators = null; + private List>? _validators = null; /// /// Initializes a new instance of the Argument class. @@ -104,14 +104,14 @@ private protected override string DefaultName } } - internal List> Validators => _validators ??= new (); + internal List> Validators => _validators ??= new (); /// - /// Adds a custom to the argument. Validators can be used + /// Adds a custom validator to the argument. Validators can be used /// to provide custom errors based on user input. /// - /// The delegate to validate the parsed argument. - public void AddValidator(ValidateSymbolResult validate) => Validators.Add(validate); + /// The action to validate the parsed argument. + public void AddValidator(Action validate) => Validators.Add(validate); /// /// Gets the default value for the argument. diff --git a/src/System.CommandLine/Command.cs b/src/System.CommandLine/Command.cs index 3c5d8a4eaf..1f8d037b3e 100644 --- a/src/System.CommandLine/Command.cs +++ b/src/System.CommandLine/Command.cs @@ -22,7 +22,7 @@ public class Command : IdentifierSymbol, IEnumerable private List? _arguments; private List - /// The delegate to validate the symbols during parsing. - public void AddValidator(ValidateSymbolResult validate) => (_validators ??= new()).Add(validate); + /// The action to validate the symbols during parsing. + public void AddValidator(Action validate) => (_validators ??= new()).Add(validate); /// /// Gets or sets a value that indicates whether unmatched tokens should be treated as errors. For example, diff --git a/src/System.CommandLine/Option.cs b/src/System.CommandLine/Option.cs index 9f17f0242f..40f09877de 100644 --- a/src/System.CommandLine/Option.cs +++ b/src/System.CommandLine/Option.cs @@ -16,7 +16,7 @@ namespace System.CommandLine public abstract class Option : IdentifierSymbol, IValueDescriptor { private string? _name; - private List>? _validators; + private List>? _validators; private readonly Argument _argument; internal Option( @@ -112,15 +112,15 @@ public override string Name } } - internal List> Validators => _validators ??= new(); + internal List> Validators => _validators ??= new(); internal bool HasValidators => _validators is not null && _validators.Count > 0; /// /// Adds a validator that will be called when the option is matched by the parser. /// - /// A delegate used to validate the produced during parsing. - public void AddValidator(ValidateSymbolResult validate) => Validators.Add(validate); + /// An action used to validate the produced during parsing. + public void AddValidator(Action validate) => Validators.Add(validate); /// /// Indicates whether a given alias exists on the option, regardless of its prefix. diff --git a/src/System.CommandLine/Parsing/ValidateSymbolResult.cs b/src/System.CommandLine/Parsing/ValidateSymbolResult.cs deleted file mode 100644 index a0cabfea1e..0000000000 --- a/src/System.CommandLine/Parsing/ValidateSymbolResult.cs +++ /dev/null @@ -1,13 +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. - -namespace System.CommandLine.Parsing; - -/// -/// A delegate used to validate symbol results during parsing. -/// -/// The type of the . -/// The symbol result -/// To display an error, set . -public delegate void ValidateSymbolResult(T symbolResult) - where T : SymbolResult; \ No newline at end of file