diff --git a/OpenQuestions.md b/OpenQuestions.md index 9dd1770bd4..874897f8bb 100644 --- a/OpenQuestions.md +++ b/OpenQuestions.md @@ -42,7 +42,7 @@ The first probably means we pass around `PipelineResult`. The second means that We started with `Validators` and then added the IValidator interface to allow conditions to do validation because they have the strong type. Checking for this first also avoids a dictionary lookup. -Our default validations will be on the Condition for the shortcut. Users can offer alternatives by creaing custom validators. The dictionary for custom validators will be lazy, and lookups will be pay for play when the user has custom validators. (This is not yet implemented.) +Our default validations will be on the Condition for the shortcut. Users can offer alternatives by creating custom validators. The dictionary for custom validators will be lazy, and lookups will be pay for play when the user has custom validators. (This is not yet implemented.) When present, custom validators have precedence. There is no cost when they are not present. @@ -56,4 +56,41 @@ Suggestion: Use internal constructors and leave conditions public ## Should `ValueCondition` be called `Condition`? -They may apply to commands. \ No newline at end of file +They may apply to commands. + +## Can we remove the "Annotations" in xxxxAnnotationExtensions + +We have other extensions, such as `AddCalculation`. Where should it go? + +They may shift to extension types in the future. + +It's a long in Solution Explorer + +## Calculated value design + +My first direction on the calculated value design was to derive from CliSymbol and treat them similarly to any other CliSymbol. This results in a technical challenge in the way the `Add` method works for CliSymbols - specifically it does not allow adding anything except options and arguments and the design results in infinite recursion if the exception is ignored. While we might be able to finesse this, it indicates just how thing the ice is if we try to "trick" things in the core parser layer. + +Instead calculated values are a new thing. They can contribute symbols when asked - their internal components can be expressed as symbols for help, for example. However, they are not a CliSymbol and for all uses must be separately treated. + +They are held on commands via annotations. Calculated values that should be are not logically located on a symbol should be on the root command. + +This will use collection annotations when they are available. For now they are List. + +We have a naming challenge that may indicate an underlying need to refactor: + +- ValueSource: Knows how to get data from disparate sources - constants, other symbols, environment variables. +- Calculation: Parameter/property on ValueSources allowing them to be relative to their source +- CalculatedValue (possibly CliCalculatedValue): A new thing that can be declared by the CliAuthor for late interpretation and type conversions. +- ValueCondition, ValueSymbol and other places where "Value" allows unification of Option and Argument (and is very, very helpful for that) + +## Tests that do not have `InternalsVisibleTo` + +Currently all the code we have is in the `internal` scope within the logical layer. As a result, we are not testing how the libary works from the outside. We could take either of these two approaches or do something else, but I think we need to do something soon to validate our design: + +- Isolate the very small amount of code that actually needs IVT into an additional test assembly for each logical layer. + - This would result in two new projects and IVT would be removed from the current test project. + - The benefit of this approach is that the largest amount of code would be running without IVT. +- Create "example" projects: + - This would result in one or two new projects, two if the dependencies are those that you would use when accessing either the raw parsing behavior or subsystems. Current tests would remain under IVT. + - The benefit of this approach is that we will be creating functional tests in the form of "how do I do xxx". Since they are tests, they would not get out of date with the code base. + - Another potential benefit would be a bounded space where we could focus on how our APIs work in practice. \ No newline at end of file diff --git a/src/System.CommandLine.Subsystems.Tests/DirectiveSubsystemTests.cs b/src/System.CommandLine.Subsystems.Tests/DirectiveSubsystemTests.cs index 2dbdebb506..ce4df79765 100644 --- a/src/System.CommandLine.Subsystems.Tests/DirectiveSubsystemTests.cs +++ b/src/System.CommandLine.Subsystems.Tests/DirectiveSubsystemTests.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using FluentAssertions; -using System.CommandLine.Directives; using System.CommandLine.Parsing; using Xunit; diff --git a/src/System.CommandLine.Subsystems.Tests/ErrorReportingFunctionalTests.cs b/src/System.CommandLine.Subsystems.Tests/ErrorReportingFunctionalTests.cs index 47e4808c18..6e4637c3bd 100644 --- a/src/System.CommandLine.Subsystems.Tests/ErrorReportingFunctionalTests.cs +++ b/src/System.CommandLine.Subsystems.Tests/ErrorReportingFunctionalTests.cs @@ -5,13 +5,6 @@ using System.CommandLine.Help; using System.CommandLine.Invocation; */ -using System.IO; -using FluentAssertions; -using Xunit; -using System.CommandLine.Tests.Utility; -using System.Linq; -using System.Threading.Tasks; - namespace System.CommandLine.Tests; public class ErrorReportingFunctionalTests diff --git a/src/System.CommandLine.Subsystems.Tests/System.CommandLine.Subsystems.Tests.csproj b/src/System.CommandLine.Subsystems.Tests/System.CommandLine.Subsystems.Tests.csproj index ba321497f8..c295972cc6 100644 --- a/src/System.CommandLine.Subsystems.Tests/System.CommandLine.Subsystems.Tests.csproj +++ b/src/System.CommandLine.Subsystems.Tests/System.CommandLine.Subsystems.Tests.csproj @@ -4,12 +4,8 @@ $(TargetFrameworkForNETSDK) false $(DefaultExcludesInProjectFolder);TestApps\** - Library enable annotations - False @@ -34,7 +30,7 @@ - + diff --git a/src/System.CommandLine.Subsystems.Tests/ValueProviderTests.cs b/src/System.CommandLine.Subsystems.Tests/ValueProviderTests.cs new file mode 100644 index 0000000000..28d87a3489 --- /dev/null +++ b/src/System.CommandLine.Subsystems.Tests/ValueProviderTests.cs @@ -0,0 +1,254 @@ +// 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 FluentAssertions; +using System.CommandLine.Parsing; +using System.CommandLine.ValueSources; +using Xunit; + +namespace System.CommandLine.Subsystems.Tests; + +public class ValueProviderTests +{ + [Fact] + public void Values_that_are_entered_are_retrieved() + { + var option = new CliOption("--intOpt"); + var rootCommand = new CliRootCommand { option }; + var configuration = new CliConfiguration(rootCommand); + var pipeline = Pipeline.Create(); + var input = "--intOpt 42"; + + var parseResult = CliParser.Parse(rootCommand, input, configuration); + var pipelineResult = new PipelineResult(parseResult, input, pipeline); + + pipelineResult.Should().NotBeNull(); + var optionValueResult = pipelineResult.GetValueResult(option); + var optionValue = pipelineResult.GetValue(option); + optionValueResult.Should().NotBeNull(); + optionValue.Should().Be(42); + } + + [Fact] + public void Values_that_are_not_entered_are_type_default_with_no_default_values() + { + var stringOption = new CliOption("--stringOption"); + var intOption = new CliOption("--intOption"); + var dateOption = new CliOption("--dateOption"); + var nullableIntOption = new CliOption("--nullableIntOption"); + var guidOption = new CliOption("--guidOption"); + var rootCommand = new CliRootCommand { stringOption, intOption, dateOption, nullableIntOption, guidOption }; + var configuration = new CliConfiguration(rootCommand); + var pipeline = Pipeline.Create(); + var input = ""; + + var parseResult = CliParser.Parse(rootCommand, input, configuration); + var pipelineResult = new PipelineResult(parseResult, input, pipeline); + + pipelineResult.Should().NotBeNull(); + var stringOptionValue = pipelineResult.GetValue(stringOption); + var intOptionValue = pipelineResult.GetValue(intOption); + var dateOptionValue = pipelineResult.GetValue(dateOption); + var nullableIntOptionValue = pipelineResult.GetValue(nullableIntOption); + var guidOptionValue = pipelineResult.GetValue(guidOption); + stringOptionValue.Should().BeNull(); + intOptionValue.Should().Be(0); + dateOptionValue.Should().Be(DateTime.MinValue); + nullableIntOptionValue.Should().BeNull(); + guidOptionValue.Should().Be(Guid.Empty); + } + + [Fact] + public void Default_value_is_used_when_user_did_not_enter_a_value() + { + var intOption = new CliOption("--intOption"); + intOption.SetDefault(42); + var rootCommand = new CliRootCommand { intOption }; + var configuration = new CliConfiguration(rootCommand); + var pipeline = Pipeline.Create(); + var input = ""; + + var parseResult = CliParser.Parse(rootCommand, input, configuration); + var pipelineResult = new PipelineResult(parseResult, input, pipeline); + + pipelineResult.Should().NotBeNull(); + var intOptionValue = pipelineResult.GetValue(intOption); + intOptionValue.Should().Be(42); + } + + /* Leaving calculated value tests in place, because they are examples of custom parser problems + [Fact] + public void Can_retrieve_calculated_value_() + { + // TODO: This is problematic because we probably can't and don't want to add new types + // of ValueSymbols. We need a different way to add these, which means we need to work + // out where they live (probably as annotations on the root command or a special symbol + // type) and how we add them (probably an extension method). + var calcSymbol = new CalculatedValue("calcValue", 42); + var rootCommand = new CliRootCommand { calcSymbol }; + var configuration = new CliConfiguration(rootCommand); + var pipeline = Pipeline.Create(); + var input = ""; + + var parseResult = CliParser.Parse(rootCommand, input, configuration); + var pipelineResult = new PipelineResult(parseResult, input, pipeline); + + pipelineResult.Should().NotBeNull(); + var intOptionValue = pipelineResult.GetValue(calcSymbol); + intOptionValue.Should().Be(42); + } + */ + + [Fact] + public void Circular_default_value_dependency_throw() + { + var opt1 = new CliOption("opt1"); + var opt2 = new CliOption("opt2"); + var opt3 = new CliOption("opt3"); + opt1.SetDefault(opt2); + opt2.SetDefault(opt3); + opt3.SetDefault(opt1); + var rootCommand = new CliRootCommand { opt1, opt2, opt3 }; + var configuration = new CliConfiguration(rootCommand); + var pipeline = Pipeline.Create(); + var input = ""; + + var parseResult = CliParser.Parse(rootCommand, input, configuration); + var pipelineResult = new PipelineResult(parseResult, input, pipeline); + + pipelineResult.Should().NotBeNull(); + Assert.Throws(() => _ = pipelineResult.GetValue(opt1)); + } + + [Fact] + public void Missing_value_throws_on_GetValue() + { + var opt1 = new CliOption("opt1"); + var opt2 = new CliOption("opt2"); + var opt3 = new CliOption("opt3"); + opt1.SetDefault(opt2); + opt2.SetDefault(opt3); + opt3.SetDefault(opt1); + var rootCommand = new CliRootCommand { opt1, opt2, opt3 }; + var configuration = new CliConfiguration(rootCommand); + var pipeline = Pipeline.Create(); + var input = ""; + + var parseResult = CliParser.Parse(rootCommand, input, configuration); + var pipelineResult = new PipelineResult(parseResult, input, pipeline); + + pipelineResult.Should().NotBeNull(); + Assert.Throws(() => _ = pipelineResult.GetValue(opt1)); + } + + /* Leaving calculated value tests in place, because they are examples of custom parser problems + // dotnet nuget why has two parameters, where the first is optional. This is historic, and we must support it + // We also believe folks are using the old custom parser to create compound types - the Point tests here + private (CliRootCommand root, CalculatedValue project, CalculatedValue package) CreateDotnetNugetWhyCli() + { + var whyArg = new CliArgument("whyArgs"); + whyArg.Arity = new ArgumentArity(1, 2); + var project = new CalculatedValue("project", ValueSource.Create(whyArg, ExtractProject)); + var package = new CalculatedValue("package", ValueSource.Create(whyArg, ExtractPackage)); + var whyCmd = new CliCommand("why") { whyArg }; + var nugetCmd = new CliCommand("nuget") { whyCmd }; + var dotnetCmd = new CliRootCommand() { nugetCmd }; + return (dotnetCmd, project, package); + + (bool success, string value) ExtractProject(object? input) + { + if (input is not string[] inputArray) + { + return (false, string.Empty); + } + return (true, + inputArray.Length == 1 + ? "" + : inputArray[0]); + } + + (bool success, string value) ExtractPackage(object? input) + { + if (input is not string[] inputArray) + { + return (false, string.Empty); + } + return (true, + inputArray.Length == 1 + ? inputArray[0] + : inputArray[1]); + } + } + + [Theory] + [InlineData("myProject.csproj myPackage", "myProject.csproj", "myPackage")] + [InlineData("myPackage", "", "myPackage")] + public void dotnet_nuget_why_can_retrieve_package_without_project(string args, string projectName, string packageName) + { + var (dotnetCmd, project, package) = CreateDotnetNugetWhyCli(); + var configuration = new CliConfiguration(dotnetCmd); + var pipeline = Pipeline.Create(); + var input = $"nuget why {args}"; + + var parseResult = CliParser.Parse(dotnetCmd, input, configuration); + var pipelineResult = new PipelineResult(parseResult, input, pipeline); + + pipelineResult.Should().NotBeNull(); + pipelineResult.GetValue(project) + .Should() + .Be(projectName); + pipelineResult.GetValue(package) + .Should() + .Be(packageName); + } + + private record struct Point2D(int X, int Y) + { + public static readonly Point2D Empty = new Point2D(0, 0); + } + + private (CliRootCommand root, CliValueSymbol point) CreatePointCli() + { + var xOpt = new CliOption("-x"); + var yOpt = new CliOption("-y"); + var point = new CalculatedValue("point", ValueSource.Create(MakePoint, otherSymbols: [xOpt, yOpt])); + var dotnetCmd = new CliRootCommand() { xOpt, yOpt }; + return (dotnetCmd, point); + + (bool success, Point2D point) MakePoint(IEnumerable input) + { + var inputInts = input.OfType().ToArray(); + if (inputInts.Length != 2) + { + return (false, Point2D.Empty); // since false is used, should return null, not empty point + } + return (true, + new Point2D(inputInts[0], inputInts[1])); + } + } + + [Theory] + [InlineData("-x 0 -y 0", "Point2D { X = 0, Y = 0 }")] + [InlineData("-x 1", "")] + [InlineData("-y 1", "")] + [InlineData("-x 1 -y 3", "Point2D { X = 1, Y = 3 }")] + [InlineData("-x -1 -y 2", "Point2D { X = -1, Y = 2 }")] + [InlineData("-x -2 -y -3", "Point2D { X = -2, Y = -3 }")] + public void Can_make_point_from_two_options(string input, string expected) + { + var (dotnetCmd, point) = CreatePointCli(); + var configuration = new CliConfiguration(dotnetCmd); + var pipeline = Pipeline.Create(); + + var parseResult = CliParser.Parse(dotnetCmd, input, configuration); + var pipelineResult = new PipelineResult(parseResult, input, pipeline); + + pipelineResult.Should().NotBeNull(); + var pointValue = pipelineResult.GetValue(point); + var output = $"{pointValue}"; + output + .Should() + .Be(expected); + } + */ +} diff --git a/src/System.CommandLine.Subsystems.Tests/ValueSourceTests.cs b/src/System.CommandLine.Subsystems.Tests/ValueSourceTests.cs index 68a482b6b7..f12bdf1706 100644 --- a/src/System.CommandLine.Subsystems.Tests/ValueSourceTests.cs +++ b/src/System.CommandLine.Subsystems.Tests/ValueSourceTests.cs @@ -26,13 +26,12 @@ public void SimpleValueSource_with_set_value_retrieved() { var valueSource = new SimpleValueSource(42); - if (valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value)) - { - value.Should() - .Be(42); - return; - } - Assert.Fail("Typed value not retrieved"); + var found = valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value); + + found.Should() + .BeTrue(); + value.Should() + .Be(42); } [Fact] @@ -40,13 +39,12 @@ public void SimpleValueSource_with_converted_value_retrieved() { ValueSource valueSource = 42; - if (valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value)) - { - value.Should() - .Be(42); - return; - } - Assert.Fail("Typed value not retrieved"); + var found = valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value); + + found.Should() + .BeTrue(); + value.Should() + .Be(42); } [Fact] @@ -54,13 +52,13 @@ public void SimpleValueSource_created_via_extension_value_retrieved() { var valueSource = ValueSource.Create(42); - if (valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value)) - { - value.Should() - .Be(42); - return; - } - Assert.Fail("Typed value not retrieved"); + var found = valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value); + + found.Should() + .BeTrue(); + value.Should() + .Be(42); + } [Fact] @@ -68,13 +66,12 @@ public void CalculatedValueSource_produces_value() { var valueSource = new CalculatedValueSource(() => (true, 42)); - if (valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value)) - { - value.Should() - .Be(42); - return; - } - Assert.Fail("Typed value not retrieved"); + var found = valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value); + + found.Should() + .BeTrue(); + value.Should() + .Be(42); } [Fact] @@ -84,13 +81,12 @@ public void CalculatedValueSource_implicitly_converted_produces_value() // ValueSource valueSource2 = (() => 42); ValueSource valueSource = (ValueSource)(() => (true, 42)); ; - if (valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value)) - { - value.Should() - .Be(42); - return; - } - Assert.Fail("Typed value not retrieved"); + var found = valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value); + + found.Should() + .BeTrue(); + value.Should() + .Be(42); } [Fact] @@ -98,29 +94,27 @@ public void CalculatedValueSource_from_extension_produces_value() { var valueSource = ValueSource.Create(() => (true, 42)); - if (valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value)) - { - value.Should() - .Be(42); - return; - } - Assert.Fail("Typed value not retrieved"); + var found = valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value); + + found.Should() + .BeTrue(); + value.Should() + .Be(42); } [Fact] public void RelativeToSymbolValueSource_produces_value_that_was_set() { var option = new CliOption("-a"); - var valueSource = new RelativeToSymbolValueSource(option); + var valueSource = new SymbolValueSource(option); + + var found = valueSource.TryGetTypedValue(EmptyPipelineResult("-a 42", option), out var value); + + found.Should() + .BeTrue(); + value.Should() + .Be(42); - if (valueSource.TryGetTypedValue(EmptyPipelineResult("-a 42", option), out var value)) - { - value.Should() - .Be(42); - return; - } - Assert.Fail("Typed value not retrieved"); - } [Fact] @@ -129,47 +123,43 @@ public void RelativeToSymbolValueSource_implicitly_converted_produces_value_that var option = new CliOption("-a"); ValueSource valueSource = option; - if (valueSource.TryGetTypedValue(EmptyPipelineResult("-a 42", option), out var value)) - { - value.Should() - .Be(42); - return; - } - Assert.Fail("Typed value not retrieved"); + var found = valueSource.TryGetTypedValue(EmptyPipelineResult("-a 42", option), out var value); + + found.Should() + .BeTrue(); + value.Should() + .Be(42); } [Fact] public void RelativeToSymbolValueSource_from_extension_produces_value_that_was_set() { var option = new CliOption("-a"); - var valueSource = new RelativeToSymbolValueSource(option); + var valueSource = new SymbolValueSource(option); - if (valueSource.TryGetTypedValue(EmptyPipelineResult("-a 42", option), out var value)) - { - value.Should() - .Be(42); - return; - } - Assert.Fail("Typed value not retrieved"); + var found = valueSource.TryGetTypedValue(EmptyPipelineResult("-a 42", option), out var value); + + found.Should() + .BeTrue(); + value.Should() + .Be(42); } [Fact] public void RelativeToEnvironmentVariableValueSource_produces_value_that_was_set() { var envName = "SYSTEM_COMMANDLINE_TESTING"; - var valueSource = new RelativeToEnvironmentVariableValueSource(envName); + var valueSource = new EnvironmentVariableValueSource(envName); Environment.SetEnvironmentVariable(envName, "42"); - if (valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value)) - { - value.Should() - .Be(42); - return; - } + var found = valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value); Environment.SetEnvironmentVariable(envName, null); - Assert.Fail("Typed value not retrieved"); - } + found.Should() + .BeTrue(); + value.Should() + .Be(42); + } [Fact] public void RelativeToEnvironmentVariableValueSource_from_extension_produces_value_that_was_set() @@ -178,13 +168,41 @@ public void RelativeToEnvironmentVariableValueSource_from_extension_produces_val var valueSource = ValueSource.CreateFromEnvironmentVariable(envName); Environment.SetEnvironmentVariable(envName, "42"); - if (valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value)) - { - value.Should() - .Be(42); - return; - } + var found = valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value); Environment.SetEnvironmentVariable(envName, null); - Assert.Fail("Typed value not retrieved"); + + found.Should() + .BeTrue(); + value.Should() + .Be(42); } + + + [Fact] + public void RelativeToSymbolValueSource_false_if_other_symbol_has_no_default_and_is_missing() + { + var option = new CliOption("-a"); + var valueSource = new SymbolValueSource(option); + + var found = valueSource.TryGetTypedValue(EmptyPipelineResult("", option), out var value); + found.Should() + .BeFalse(); + value.Should() + .Be(default); + } + + [Fact] + public void RelativeToEnvironmentVariable_false_if_environment_variable_missing() + { + var envName = "SYSTEM_COMMANDLINE_TESTING"; + var valueSource = new EnvironmentVariableValueSource(envName); + + var found = valueSource.TryGetTypedValue(EmptyPipelineResult(), out var value); + + found.Should() + .BeFalse(); + value.Should() + .Be(default); + } + } diff --git a/src/System.CommandLine.Subsystems.Tests/ValueSubsystemTests.cs b/src/System.CommandLine.Subsystems.Tests/ValueSubsystemTests.cs deleted file mode 100644 index 9fb7573a9d..0000000000 --- a/src/System.CommandLine.Subsystems.Tests/ValueSubsystemTests.cs +++ /dev/null @@ -1,126 +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 FluentAssertions; -using System.CommandLine.Directives; -using System.CommandLine.Parsing; -using Xunit; -using static System.CommandLine.Subsystems.Tests.TestData; - -namespace System.CommandLine.Subsystems.Tests; - -public class ValueSubsystemTests -{ - [Fact] - public void Values_that_are_entered_are_retrieved() - { - var option = new CliOption("--intOpt"); - var rootCommand = new CliRootCommand { option }; - var configuration = new CliConfiguration(rootCommand); - var pipeline = Pipeline.Create(); - var input = "--intOpt 42"; - - var parseResult = CliParser.Parse(rootCommand, input, configuration); - var pipelineResult = new PipelineResult(parseResult, input, pipeline); - - pipelineResult.Should().NotBeNull(); - var optionValueResult = pipelineResult.GetValueResult(option); - var optionValue = pipelineResult.GetValue(option); - optionValueResult.Should().NotBeNull(); - optionValue.Should().Be(42); - } - - [Fact] - public void Values_that_are_not_entered_are_type_default_with_no_default_values() - { - var stringOption = new CliOption("--stringOption"); - var intOption = new CliOption("--intOption"); - var dateOption = new CliOption("--dateOption"); - var nullableIntOption = new CliOption("--nullableIntOption"); - var guidOption = new CliOption("--guidOption"); - var rootCommand = new CliRootCommand { stringOption, intOption, dateOption, nullableIntOption, guidOption }; - var configuration = new CliConfiguration(rootCommand); - var pipeline = Pipeline.Create(); - var input = ""; - - var parseResult = CliParser.Parse(rootCommand, input, configuration); - var pipelineResult = new PipelineResult(parseResult, input, pipeline); - - pipelineResult.Should().NotBeNull(); - var stringOptionValue = pipelineResult.GetValue(stringOption); - var intOptionValue = pipelineResult.GetValue(intOption); - var dateOptionValue = pipelineResult.GetValue(dateOption); - var nullableIntOptionValue = pipelineResult.GetValue(nullableIntOption); - var guidOptionValue = pipelineResult.GetValue(guidOption); - stringOptionValue.Should().BeNull(); - intOptionValue.Should().Be(0); - dateOptionValue.Should().Be(DateTime.MinValue); - nullableIntOptionValue.Should().BeNull(); - guidOptionValue.Should().Be(Guid.Empty); - } - - // TODO: Add various default value tests - - /* Hold these tests until we determine if ValueSubsystem is replaceable - [Fact] - public void ValueSubsystem_returns_values_that_are_entered() - { - var consoleHack = new ConsoleHack().RedirectToBuffer(true); - CliOption option = new CliOption("--intValue"); - CliRootCommand rootCommand = [ - new CliCommand("x") - { - option - }]; - var configuration = new CliConfiguration(rootCommand); - var pipeline = Pipeline.CreateEmpty(); - pipeline.Value = new ValueSubsystem(); - const int expected = 42; - var input = $"x --intValue {expected}"; - - var parseResult = pipeline.Parse(configuration, input); // assigned for debugging - pipeline.Execute(configuration, input, consoleHack); - - pipeline.Value.GetValue(option).Should().Be(expected); - } - - [Fact] - public void ValueSubsystem_returns_default_value_when_no_value_is_entered() - { - var consoleHack = new ConsoleHack().RedirectToBuffer(true); - CliOption option = new CliOption("--intValue"); - CliRootCommand rootCommand = [option]; - var configuration = new CliConfiguration(rootCommand); - var pipeline = Pipeline.CreateEmpty(); - pipeline.Value = new ValueSubsystem(); - option.SetDefaultValue(43); - const int expected = 43; - var input = $""; - - pipeline.Execute(configuration, input, consoleHack); - - pipeline.Value.GetValue(option).Should().Be(expected); - } - - - [Fact] - public void ValueSubsystem_returns_calculated_default_value_when_no_value_is_entered() - { - var consoleHack = new ConsoleHack().RedirectToBuffer(true); - CliOption option = new CliOption("--intValue"); - CliRootCommand rootCommand = [option]; - var configuration = new CliConfiguration(rootCommand); - var pipeline = Pipeline.CreateEmpty(); - pipeline.Value = new ValueSubsystem(); - var x = 42; - option.SetDefaultValueCalculation(() => x + 2); - const int expected = 44; - var input = ""; - - var parseResult = pipeline.Parse(configuration, input); // assigned for debugging - pipeline.Execute(configuration, input, consoleHack); - - pipeline.Value.GetValue(option).Should().Be(expected); - } - */ -} diff --git a/src/System.CommandLine.Subsystems/PipelineResult.cs b/src/System.CommandLine.Subsystems/PipelineResult.cs index 55b5f4963e..edd0e04ab1 100644 --- a/src/System.CommandLine.Subsystems/PipelineResult.cs +++ b/src/System.CommandLine.Subsystems/PipelineResult.cs @@ -9,7 +9,7 @@ public class PipelineResult { // TODO: Try to build workflow so it is illegal to create this without a ParseResult private readonly List errors = []; - private ValueProvider valueProvider { get; } + private readonly ValueProvider valueProvider; public PipelineResult(ParseResult parseResult, string rawInput, Pipeline? pipeline, ConsoleHack? consoleHack = null) { @@ -36,10 +36,15 @@ public PipelineResult(ParseResult parseResult, string rawInput, Pipeline? pipeli public object? GetValue(CliValueSymbol option) => valueProvider.GetValue(option); + public bool TryGetValue(CliValueSymbol valueSymbol, out T? value) + => valueProvider.TryGetValue(valueSymbol, out value); + + public bool TryGetValue(CliValueSymbol option, out object? value) + => valueProvider.TryGetValue(option, out value); + public CliValueResult? GetValueResult(CliValueSymbol valueSymbol) => ParseResult.GetValueResult(valueSymbol); - public void AddErrors(IEnumerable errors) { if (errors is not null) diff --git a/src/System.CommandLine.Subsystems/System.CommandLine.Subsystems.csproj b/src/System.CommandLine.Subsystems/System.CommandLine.Subsystems.csproj index 2f5b0e4d91..c3185ba92a 100644 --- a/src/System.CommandLine.Subsystems/System.CommandLine.Subsystems.csproj +++ b/src/System.CommandLine.Subsystems/System.CommandLine.Subsystems.csproj @@ -6,6 +6,8 @@ enable System.CommandLine true + + diff --git a/src/System.CommandLine.Subsystems/Validation/ValidationContext.cs b/src/System.CommandLine.Subsystems/Validation/ValidationContext.cs index 3f22a12280..d75e64c745 100644 --- a/src/System.CommandLine.Subsystems/Validation/ValidationContext.cs +++ b/src/System.CommandLine.Subsystems/Validation/ValidationContext.cs @@ -3,7 +3,6 @@ using System.CommandLine.Parsing; using System.CommandLine.ValueSources; -using static System.Runtime.InteropServices.JavaScript.JSType; namespace System.CommandLine.Validation; diff --git a/src/System.CommandLine.Subsystems/ValueAnnotationExtensions.cs b/src/System.CommandLine.Subsystems/ValueAnnotationExtensions.cs index 676bcc9b7d..c76e61c4e1 100644 --- a/src/System.CommandLine.Subsystems/ValueAnnotationExtensions.cs +++ b/src/System.CommandLine.Subsystems/ValueAnnotationExtensions.cs @@ -21,8 +21,8 @@ public static class ValueAnnotationExtensions /// which calculates the actual default value, based on the default value annotation and default value calculation, /// whether directly stored on the symbol or from the subsystem's . /// - public static bool TryGetDefaultValueSource(this CliValueSymbol valueSymbol, [NotNullWhen(true)] out ValueSource? defaultValueSource) - => valueSymbol.TryGetAnnotation(ValueAnnotations.DefaultValueSource, out defaultValueSource); + public static bool TryGetDefault(this CliValueSymbol valueSymbol, out ValueSource? defaultValueSource) + => valueSymbol.TryGetAnnotation(ValueAnnotations.DefaultValue, out defaultValueSource); /// /// Sets the default value annotation on the @@ -30,185 +30,7 @@ public static bool TryGetDefaultValueSource(this CliValueSymbol valueSymbol, [No /// The type of the option value /// The option /// The default value for the option - public static void SetDefaultValueSource(this CliValueSymbol valueSymbol, ValueSource defaultValue) + public static void SetDefault(this CliValueSymbol valueSymbol, ValueSource defaultValue) => valueSymbol.SetAnnotation(ValueAnnotations.DefaultValue, defaultValue); - - /// - /// Sets the default value annotation on the - /// - /// The type of the option value - /// The option - /// The default value for the option - /// The , to enable fluent construction of symbols with annotations. - public static CliOption WithDefaultValue(this CliOption option, TValue defaultValue) - { - option.SetDefaultValue(defaultValue); - return option; - } - - /// - /// Sets the default value annotation on the - /// - /// The type of the option value - /// The option - /// The default value for the option - public static void SetDefaultValue(this CliOption option, TValue defaultValue) - { - option.SetAnnotation(ValueAnnotations.DefaultValue, defaultValue); - } - - /// - /// Get the default value annotation for the - /// - /// The type of the option value - /// The option - /// The option's default value annotation if any, otherwise - /// - /// This is intended to be called by CLI authors. Subsystems should instead call , - /// which calculates the actual default value, based on the default value annotation and default value calculation, - /// whether directly stored on the symbol or from the subsystem's . - /// - public static TValue? GetDefaultValueAnnotation(this CliOption option) - { - if (option.TryGetAnnotation(ValueAnnotations.DefaultValue, out TValue? defaultValue)) - { - return defaultValue; - } - return default; - } - - /// - /// Sets the default value annotation on the - /// - /// The type of the argument value - /// The argument - /// The default value for the argument - /// The , to enable fluent construction of symbols with annotations. - public static CliArgument WithDefaultValue(this CliArgument argument, TValue defaultValue) - { - argument.SetDefaultValue(defaultValue); - return argument; - } - - /// - /// Sets the default value annotation on the - /// - /// The type of the argument value - /// The argument - /// The default value for the argument - /// The , to enable fluent construction of symbols with annotations. - public static void SetDefaultValue(this CliArgument argument, TValue defaultValue) - { - argument.SetAnnotation(ValueAnnotations.DefaultValue, defaultValue); - } - - /// - /// Get the default value annotation for the - /// - /// The type of the argument value - /// The argument - /// The argument's default value annotation if any, otherwise - /// - /// This is intended to be called by CLI authors. Subsystems should instead call , - /// which calculates the actual default value, based on the default value annotation and default value calculation, - /// whether directly stored on the symbol or from the subsystem's . - /// - public static TValue? GetDefaultValueAnnotation(this CliArgument argument) - { - if (argument.TryGetAnnotation(ValueAnnotations.DefaultValue, out TValue? defaultValue)) - { - return (TValue?)defaultValue; - } - return default; - } - - /// - /// Sets the default value calculation for the - /// - /// The type of the option value - /// The option - /// The default value calculation for the option - /// The , to enable fluent construction of symbols with annotations. - public static CliOption WithDefaultValueCalculation(this CliOption option, Func defaultValueCalculation) - { - option.SetDefaultValueCalculation(defaultValueCalculation); - return option; - } - - /// - /// Sets the default value calculation for the - /// - /// The type of the option value - /// The option - /// The default value calculation for the option - public static void SetDefaultValueCalculation(this CliOption option, Func defaultValueCalculation) - { - option.SetAnnotation(ValueAnnotations.DefaultValueCalculation, defaultValueCalculation); - } - - /// - /// Get the default value calculation for the - /// - /// The type of the option value - /// The option - /// The option's default value calculation if any, otherwise - /// - /// This is intended to be called by CLI authors. Subsystems should instead call , - /// which calculates the actual default value, based on the default value annotation and default value calculation, - /// whether directly stored on the symbol or from the subsystem's . - /// - public static Func? GetDefaultValueCalculation(this CliOption option) - { - if (option.TryGetAnnotation(ValueAnnotations.DefaultValueCalculation, out Func? defaultValueCalculation)) - { - return defaultValueCalculation; - } - return default; - } - - /// - /// Sets the default value calculation for the - /// - /// The type of the argument value - /// The argument - /// The default value calculation for the argument - /// The , to enable fluent construction of symbols with annotations. - public static CliArgument WithDefaultValueCalculation(this CliArgument argument, Func defaultValueCalculation) - { - argument.SetDefaultValueCalculation(defaultValueCalculation); - return argument; - } - - /// - /// Sets the default value calculation for the - /// - /// The type of the argument value - /// The argument - /// The default value calculation for the argument - /// The , to enable fluent construction of symbols with annotations. - public static void SetDefaultValueCalculation(this CliArgument argument, Func defaultValueCalculation) - { - argument.SetAnnotation(ValueAnnotations.DefaultValueCalculation, defaultValueCalculation); - } - - /// - /// Get the default value calculation for the - /// - /// The type of the argument value - /// The argument - /// The argument's default value calculation if any, otherwise - /// - /// This is intended to be called by CLI authors. Subsystems should instead call , - /// which calculates the actual default value, based on the default value annotation and default value calculation, - /// whether directly stored on the symbol or from the subsystem's . - /// - public static Func? GetDefaultValueCalculation(this CliArgument argument) - { - if (argument.TryGetAnnotation(ValueAnnotations.DefaultValueCalculation, out Func? defaultValueCalculation)) - { - return defaultValueCalculation; - } - return default; - } } diff --git a/src/System.CommandLine.Subsystems/ValueProvider.cs b/src/System.CommandLine.Subsystems/ValueProvider.cs index f51a1edb63..f6ec04af34 100644 --- a/src/System.CommandLine.Subsystems/ValueProvider.cs +++ b/src/System.CommandLine.Subsystems/ValueProvider.cs @@ -7,7 +7,14 @@ namespace System.CommandLine; internal class ValueProvider { - private Dictionary cachedValues = []; + private struct CacheItem + { + internal object? Value; + internal bool WasFound; + internal bool IsCalculating; + } + + private Dictionary cachedValues = []; private PipelineResult pipelineResult; public ValueProvider(PipelineResult pipelineResult) @@ -15,65 +22,82 @@ public ValueProvider(PipelineResult pipelineResult) this.pipelineResult = pipelineResult; } - private void SetValue(CliSymbol symbol, object? value) + private void SetCachedValue(CliSymbol symbol, object? value, bool found) { - cachedValues[symbol] = value; + // TODO: MHutch: Feel free to optimize this and SetIsCalculating. We need the struct or a tuple here for "WasFound" which turns out we need. + cachedValues[symbol] = new CacheItem() + { + Value = value, + WasFound = found, + IsCalculating = false + }; } - private bool TryGetValue(CliSymbol symbol, out T? value) + private void SetIsCalculating(CliSymbol symbol) { - if (cachedValues.TryGetValue(symbol, out var objectValue)) + cachedValues[symbol] = new CacheItem() { - value = objectValue is null - ? default - : (T)objectValue; - return true; - } - value = default; - return false; + IsCalculating = true + }; } public T? GetValue(CliValueSymbol valueSymbol) - => GetValueInternal(valueSymbol); + { + if (TryGetValueInternal(valueSymbol, out var value)) + { + return value; + } + return default; + } + + public bool TryGetValue(CliValueSymbol valueSymbol, out T? value) + => TryGetValueInternal(valueSymbol, out value); - private T? GetValueInternal(CliValueSymbol valueSymbol) + private bool TryGetValueInternal(CliValueSymbol valueSymbol, out T? value) { - // TODO: Add guard to prevent reentrancy for the same symbol via RelativeToSymbol of custom ValueSource var _ = valueSymbol ?? throw new ArgumentNullException(nameof(valueSymbol)); - if (TryGetValue(valueSymbol, out var value)) + + if (cachedValues.TryGetValue(valueSymbol, out var cacheItem)) { - return value; + if (cacheItem.IsCalculating) + { + value = default; + // Guard against reentrancy. Placed here so we catch if a CalculatedValue or calculation causes reentrancy + throw new InvalidOperationException("Circular value source dependency."); + } + if (cacheItem.WasFound) + { + value = (T?)cacheItem.Value; + return true; + } } + // !!! CRITICAL: All returns from this method should set the cache value to clear this pseudo-lock (use CacheAndReturn) + SetIsCalculating(valueSymbol); + if (pipelineResult.ParseResult?.GetValueResult(valueSymbol) is { } valueResult) { - return UseValue(valueSymbol, valueResult.GetValue()); + value = valueResult.GetValue(); + return CacheAndReturn(valueSymbol, value, true); } - if (valueSymbol.TryGetDefaultValueSource(out ValueSource? defaultValueSource)) + if (valueSymbol.TryGetDefault(out ValueSource? defaultValueSource)) { if (defaultValueSource is not ValueSource typedDefaultValueSource) { - throw new InvalidOperationException("Unexpected ValueSource type"); + throw new InvalidOperationException("Unexpected ValueSource type for default value."); } - if (typedDefaultValueSource.TryGetTypedValue(pipelineResult, out T? defaultValue)) + if (typedDefaultValueSource.TryGetTypedValue(pipelineResult, out value)) { - return UseValue(valueSymbol, defaultValue); + return CacheAndReturn(valueSymbol, value, true); } } - return UseValue(valueSymbol, default(T)); + // TODO: Determine if we should cache default. If so, additional design is needed to avoid first hit returning false, and remainder returning true + value = default; + return CacheAndReturn(valueSymbol, value, false); - TValue UseValue(CliSymbol symbol, TValue value) + bool CacheAndReturn(CliValueSymbol valueSymbol, T? valueToCache, bool valueFound) { - SetValue(symbol, value); - return value; + SetCachedValue(valueSymbol, valueToCache, valueFound); + return valueFound; } } - - private static T? CalculatedDefault(CliValueSymbol valueSymbol, Func defaultValueCalculation) - { - var objectValue = defaultValueCalculation(); - var value = objectValue is null - ? default - : (T)objectValue; - return value; - } } diff --git a/src/System.CommandLine.Subsystems/ValueSources/CollectionValueSource.cs b/src/System.CommandLine.Subsystems/ValueSources/CollectionValueSource.cs new file mode 100644 index 0000000000..0b7262b602 --- /dev/null +++ b/src/System.CommandLine.Subsystems/ValueSources/CollectionValueSource.cs @@ -0,0 +1,82 @@ +// 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.ValueSources; + +/// +/// that returns the value of the specified other symbol. +/// If the calculation delegate is supplied, the returned value of the calculation is returned. +/// +/// The type to be returned, which is almost always the type of the symbol the ValueSource will be used for. +/// The , , or to include as sources. +/// A delegate that returns a value of the type of the collection source, which can be either a single value or a collection of values. +/// The description of this value, used to clarify the intent of the values that appear in error messages. +public sealed class CollectionValueSource + : ValueSource +{ + internal CollectionValueSource( + Func, (bool success, T? value)> calculation, + bool onlyUserEnteredValues = false, + string? description = null, + params CliValueSymbol[] otherSymbols) + { + OtherSymbols = otherSymbols; + OnlyUserEnteredValues = onlyUserEnteredValues; + Calculation = calculation; + Description = description; + } + + /// + /// The description that will be used in messages, such as value conditions + /// + public override string? Description { get; } + + /// + /// The other symbols that this ValueSource depends on. + /// + public IEnumerable OtherSymbols { get; } + + /// + /// If true, default values will not be used. + /// + // TODO: Find scenarios for good tests. This is based on intuition not a known scenario. Overall we are pretty aggressive about default values. + public bool OnlyUserEnteredValues { get; } + + /// + /// The calculation that determines a single value, which might an instance of a complex type, based on the values provided. + /// + public Func, (bool success, T? value)> Calculation { get; } + + /// + public override bool TryGetTypedValue(PipelineResult pipelineResult, out T? value) + { + // TODO: How do we test for only user values. (no defaults) + //if (OnlyUserEnteredValues && pipelineResult.GetValueResult(OtherSymbol) is null) + //{ + // value = default; + // return false; + //} + + var otherSymbolValues = OtherSymbols.Select(GetOtherSymbolValues).ToArray(); + (var success, var newValue) = Calculation(otherSymbolValues); + if (success) + { + value = newValue; + return true; + } + + value = default; + return false; + + object? GetOtherSymbolValues(CliValueSymbol otherSymbol) + { + if (pipelineResult.TryGetValue(otherSymbol, out var otherSymbolValue)) + { + return otherSymbolValue; + } + // TODO: I suspect we will need more data here, such as whether it exists and whether user entered + return null; + } + } +} + diff --git a/src/System.CommandLine.Subsystems/ValueSources/RelativeToEnvironmentVariableValueSource.cs b/src/System.CommandLine.Subsystems/ValueSources/EnvironmentVariableValueSource.cs similarity index 96% rename from src/System.CommandLine.Subsystems/ValueSources/RelativeToEnvironmentVariableValueSource.cs rename to src/System.CommandLine.Subsystems/ValueSources/EnvironmentVariableValueSource.cs index 07faef3ff4..d5f6aac679 100644 --- a/src/System.CommandLine.Subsystems/ValueSources/RelativeToEnvironmentVariableValueSource.cs +++ b/src/System.CommandLine.Subsystems/ValueSources/EnvironmentVariableValueSource.cs @@ -11,10 +11,10 @@ namespace System.CommandLine.ValueSources; /// The name of then environment variable. Note that for some systems, this is case sensitive. /// A delegate that returns the requested type. If it is not specified, standard type conversions are used. /// The description of this value, used to clarify the intent of the values that appear in error messages. -public sealed class RelativeToEnvironmentVariableValueSource +public sealed class EnvironmentVariableValueSource : ValueSource { - internal RelativeToEnvironmentVariableValueSource( + internal EnvironmentVariableValueSource( string environmentVariableName, Func? calculation = null, string? description = null) diff --git a/src/System.CommandLine.Subsystems/ValueSources/AggregateValueSource.cs b/src/System.CommandLine.Subsystems/ValueSources/FallbackValueSource.cs similarity index 86% rename from src/System.CommandLine.Subsystems/ValueSources/AggregateValueSource.cs rename to src/System.CommandLine.Subsystems/ValueSources/FallbackValueSource.cs index 751b789794..f9c10a0543 100644 --- a/src/System.CommandLine.Subsystems/ValueSources/AggregateValueSource.cs +++ b/src/System.CommandLine.Subsystems/ValueSources/FallbackValueSource.cs @@ -3,11 +3,11 @@ namespace System.CommandLine.ValueSources; -public sealed class AggregateValueSource : ValueSource +public sealed class FallbackValueSource : ValueSource { private List> valueSources = []; - internal AggregateValueSource(ValueSource firstSource, + internal FallbackValueSource(ValueSource firstSource, ValueSource secondSource, string? description = null, params ValueSource[] otherSources) @@ -45,10 +45,10 @@ internal static int GetPrecedence(ValueSource source) return source switch { SimpleValueSource => 0, - RelativeToSymbolValueSource => 1, + SymbolValueSource => 1, CalculatedValueSource => 2, //RelativeToConfigurationValueSource => 3, - RelativeToEnvironmentVariableValueSource => 4, + EnvironmentVariableValueSource => 4, _ => 5 }; } diff --git a/src/System.CommandLine.Subsystems/ValueSources/RelativeToSymbolValueSource.cs b/src/System.CommandLine.Subsystems/ValueSources/SymbolValueSource.cs similarity index 76% rename from src/System.CommandLine.Subsystems/ValueSources/RelativeToSymbolValueSource.cs rename to src/System.CommandLine.Subsystems/ValueSources/SymbolValueSource.cs index a53e13c573..2beb6d5d52 100644 --- a/src/System.CommandLine.Subsystems/ValueSources/RelativeToSymbolValueSource.cs +++ b/src/System.CommandLine.Subsystems/ValueSources/SymbolValueSource.cs @@ -11,13 +11,14 @@ namespace System.CommandLine.ValueSources; /// The option or argument to return, with the calculation supplied if it is not null. /// A delegate that returns the requested type. /// The description of this value, used to clarify the intent of the values that appear in error messages. -public sealed class RelativeToSymbolValueSource +public sealed class SymbolValueSource : ValueSource { - internal RelativeToSymbolValueSource( + // TODO: API differences between this adn RelativeToSymbols are very annoying + internal SymbolValueSource( CliValueSymbol otherSymbol, - bool onlyUserEnteredValues = false, Func? calculation = null, + bool onlyUserEnteredValues = false, string? description = null) { OtherSymbol = otherSymbol; @@ -40,19 +41,21 @@ public override bool TryGetTypedValue(PipelineResult pipelineResult, out T? valu return false; } - var otherSymbolValue = pipelineResult.GetValue(OtherSymbol); - - if (Calculation is null) + if (pipelineResult.TryGetValue(OtherSymbol, out var otherSymbolValue)) { - value = otherSymbolValue; - return true; - } - (var success, var newValue) = Calculation(otherSymbolValue); - if (success) - { - value = newValue; - return true; + if (Calculation is null) + { + value = (T?)otherSymbolValue; + return true; + } + (var success, var newValue) = Calculation(otherSymbolValue); + if (success) + { + value = newValue; + return true; + } } + value = default; return false; } diff --git a/src/System.CommandLine.Subsystems/ValueSources/ValueSource.cs b/src/System.CommandLine.Subsystems/ValueSources/ValueSource.cs index 43098896ed..327b25057b 100644 --- a/src/System.CommandLine.Subsystems/ValueSources/ValueSource.cs +++ b/src/System.CommandLine.Subsystems/ValueSources/ValueSource.cs @@ -34,18 +34,25 @@ public static ValueSource Create(CliValueSymbol otherSymbol, Func? calculation = null, bool userEnteredValueOnly = false, string? description = null) - => new RelativeToSymbolValueSource(otherSymbol, userEnteredValueOnly, calculation, description); + => new SymbolValueSource(otherSymbol, calculation, userEnteredValueOnly, description); + + public static ValueSource Create( + Func, (bool success, T? value)> calculation, + bool userEnteredValueOnly = false, + string? description = null, + params CliValueSymbol[] otherSymbols) + => new CollectionValueSource(calculation, userEnteredValueOnly, description, otherSymbols); public static ValueSource Create(ValueSource firstSource, ValueSource secondSource, string? description = null, params ValueSource[] otherSources) - => new AggregateValueSource(firstSource, secondSource, description, otherSources); + => new FallbackValueSource(firstSource, secondSource, description, otherSources); public static ValueSource CreateFromEnvironmentVariable(string environmentVariableName, Func? calculation = null, string? description = null) - => new RelativeToEnvironmentVariableValueSource(environmentVariableName, calculation, description); + => new EnvironmentVariableValueSource(environmentVariableName, calculation, description); } // TODO: Determine philosophy for custom value sources and whether they can build on existing sources. @@ -77,7 +84,7 @@ public override bool TryGetValue(PipelineResult pipelineResult, public static implicit operator ValueSource(T value) => new SimpleValueSource(value); public static implicit operator ValueSource(Func<(bool success, T? value)> calculated) => new CalculatedValueSource(calculated); - public static implicit operator ValueSource(CliValueSymbol symbol) => new RelativeToSymbolValueSource(symbol); + public static implicit operator ValueSource(CliValueSymbol symbol) => new SymbolValueSource(symbol); // Environment variable does not have an explicit operator, because converting to string was too broad } diff --git a/src/System.CommandLine/CliCommand.cs b/src/System.CommandLine/CliCommand.cs index 8adccb5d9d..18421e4613 100644 --- a/src/System.CommandLine/CliCommand.cs +++ b/src/System.CommandLine/CliCommand.cs @@ -28,7 +28,7 @@ public class CliCommand : CliSymbol, IEnumerable private ChildSymbolList? _arguments; private ChildSymbolList? _options; private ChildSymbolList? _subcommands; -// TODO: validators + // TODO: validators /* private List>? _validators; @@ -41,11 +41,11 @@ public class CliCommand : CliSymbol, IEnumerable /// The description of the command, shown in help. */ public CliCommand(string name)/*, string? description = null) */ - : base(name) + : base(name) { } -// TODO: help - //=> Description = description; + // TODO: help + //=> Description = description; /// /// Gets the child symbols. @@ -70,13 +70,13 @@ public IEnumerable Children /// public IList Arguments => _arguments ??= new(this); - internal bool HasArguments => _arguments?.Count > 0 ; + internal bool HasArguments => _arguments?.Count > 0; /// /// Represents all of the options for the command, inherited options that have been applied to any of the command's ancestors. /// // TODO: Consider value of lazy here. It sets up a desire to use awkward approach (HasOptions) for a perf win. Applies to Options and Subcommands also. - public IList Options => _options ??= new (this); + public IList Options => _options ??= new(this); internal bool HasOptions => _options?.Count > 0; @@ -86,7 +86,7 @@ public IEnumerable Children public IList Subcommands => _subcommands ??= new(this); internal bool HasSubcommands => _subcommands is not null && _subcommands.Count > 0; -/* + /* /// /// Validators to the command. Validators can be used /// to create custom validation logic. @@ -179,24 +179,24 @@ public void SetAction(Func> action) /// Adds a to the command. /// /// The option to add to the command. - public void Add(CliArgument argument) => Arguments.Add(argument); - + public void Add(CliArgument argument) => Arguments.Add(argument); + /// /// Adds a to the command. /// /// The option to add to the command. - public void Add(CliOption option) => Options.Add(option); + public void Add(CliOption option) => Options.Add(option); /// /// Adds a to the command. /// /// The Command to add to the command. - public void Add(CliCommand command) => Subcommands.Add(command); + public void Add(CliCommand command) => Subcommands.Add(command); // Hide from IntelliSense as it's only to support initializing via C# collection expression // More specific efficient overloads are available for all supported symbol types. - [DebuggerStepThrough] - [EditorBrowsable(EditorBrowsableState.Never)] + //[DebuggerStepThrough] + //[EditorBrowsable(EditorBrowsableState.Never)] public void Add(CliSymbol symbol) { if (symbol is CliCommand cmd) @@ -213,19 +213,19 @@ public void Add(CliSymbol symbol) } else { -// TODO: add a localized message here + // TODO: add a localized message here throw new ArgumentException(null, nameof(symbol)); } } -// TODO: umatched tokens - /* + // TODO: umatched tokens + /* /// /// Gets or sets a value that indicates whether unmatched tokens should be treated as errors. For example, /// if set to and an extra command or argument is provided, validation will fail. /// public bool TreatUnmatchedTokensAsErrors { get; set; } = true; -*/ + */ /// // Hide from IntelliSense as it's only to support C# collection initializer [DebuggerStepThrough] @@ -237,7 +237,7 @@ public void Add(CliSymbol symbol) [DebuggerStepThrough] [EditorBrowsable(EditorBrowsableState.Never)] IEnumerator IEnumerable.GetEnumerator() => Children.GetEnumerator(); -/* + /* /// /// Parses an array strings using the command. /// @@ -327,8 +327,8 @@ public override IEnumerable GetCompletions(CompletionContext con } return completions - .OrderBy(item => item.SortText.IndexOfCaseInsensitive(context.WordToComplete)) - .ThenBy(symbol => symbol.Label, StringComparer.OrdinalIgnoreCase); + .OrderBy(item => item.SortText.IndexOfCaseInsensitive(context.WordToComplete)) + .ThenBy(symbol => symbol.Label, StringComparer.OrdinalIgnoreCase); void AddCompletionsFor(CliSymbol identifier, AliasSet? aliases) { @@ -352,7 +352,7 @@ void AddCompletionsFor(CliSymbol identifier, AliasSet? aliases) } } } -*/ + */ internal bool EqualsNameOrAlias(string name) => Name.Equals(name, StringComparison.Ordinal) || (_aliases is not null && _aliases.Contains(name)); } }