Updates to match main-powederhouse

Author: KathleenDollard

src/System.CommandLine.Subsystems.Tests/ValueSubsystemTests.cs

@@ -10,25 +10,8 @@ namespace System.CommandLine.Subsystems.Tests;
 
 public class ValueSubsystemTests
 {
-    //[Fact]
-    //public void ValueSubsystem_is_activated_by_default()
-    //{
-    //    CliRootCommand rootCommand = [
-    //        new CliCommand("x")
-    //        {
-    //            new CliOption<string>("--opt1")
-    //        }];
-    //    var configuration = new CliConfiguration(rootCommand);
-    //    var subsystem = new ValueProvider();
-    //    var input = "x --opt1 Kirk";
-    //    var args = CliParser.SplitCommandLine(input).ToList();
+    // TODO: Add various default value tests
 
-    //    Subsystem.Initialize(subsystem, configuration, args);
-    //    var parseResult = CliParser.Parse(rootCommand, args[0], configuration);
-    //    var isActive = Subsystem.GetIsActivated(subsystem, parseResult);
-
-    //    isActive.Should().BeTrue();
-    //}
     /* Hold these tests until we determine if ValueSubsystem is replaceable
     [Fact]
     public void ValueSubsystem_returns_values_that_are_entered()

src/System.CommandLine.Subsystems/Directives/DiagramSubsystem.cs

@@ -67,17 +67,17 @@ private static void Diagram(
         {
             builder.Append('!');
         }
-    */
-    // TODO: Directives
-    /*
+        */
+        // TODO: Directives
+        /*
         switch (symbolResult)
         {
             case DirectiveResult { Directive: not DiagramDirective }:
                 break;
             */
 
-    // TODO: This logic is deeply tied to internal types/properties. These aren't things we probably want to expose like SymbolNode. See #2349 for alternatives
-    /*
+        // TODO: This logic is deeply tied to internal types/properties. These aren't things we probably want to expose like SymbolNode. See #2349 for alternatives
+        /*
         case ArgumentResult argumentResult:
             {
                 var includeArgumentName =
@@ -177,6 +177,5 @@ private static void Diagram(
             }
         }
     }
-}
 */
 }

src/System.CommandLine.Subsystems/Pipeline.cs

@@ -56,7 +56,6 @@ public static Pipeline CreateEmpty()
 
     private Pipeline()
     {
-        //Value = new ValueProvider();
         Response = new ResponseSubsystem();
         Invocation = new InvocationSubsystem();
         Validation = new ValidationSubsystem();
@@ -182,11 +181,6 @@ public ErrorReportingSubsystem? ErrorReporting
     /// </summary>
     public InvocationSubsystem? Invocation { get; }
 
-    ///// <summary>
-    ///// Gets the value subsystem which manages entered and default values.
-    ///// </summary>
-    //public ValueProvider Value { get; }
-
     /// <summary>
     /// Gets the response file subsystem
     /// </summary>

src/System.CommandLine.Tests/ParserTests.cs

@@ -139,7 +139,7 @@ public void Option_short_forms_can_be_bundled()
 
             var result = CliParser.Parse(command, "the-command -xyz");
 
-            result.CommandValueResult
+            result.CommandResult
                 .ValueResults
                 .Select(o => ((CliValueResult)o).ValueSymbol.Name)
                 .Should()
@@ -188,7 +188,7 @@ public void Option_long_forms_do_not_get_unbundled()
 
             var result = CliParser.Parse(command, "the-command --xyz");
 
-            result.CommandResult
+            result.CommandResultInternal
                 .Children
                 .Select(o => ((CliOptionResultInternal)o).Option.Name)
                 .Should()
@@ -210,7 +210,7 @@ public void Options_do_not_get_unbundled_unless_all_resulting_options_would_be_v
 
             ParseResult result = CliParser.Parse(outer, "outer inner -abc");
 
-            result.CommandResult
+            result.CommandResultInternal
                 .Tokens
                 .Select(t => t.Value)
                 .Should()
@@ -422,7 +422,7 @@ public void When_an_option_is_not_respecified_but_limit_is_reached_then_the_foll
                 .Should()
                 .BeEquivalentTo("carrot");
 
-            result.CommandResult
+            result.CommandResultInternal
                 .Tokens
                 .Select(t => t.Value)
                 .Should()
@@ -440,13 +440,13 @@ public void Command_with_multiple_options_is_parsed_correctly()
 
             var result = CliParser.Parse(command, "outer --inner1 argument1 --inner2 argument2");
 
-            result.CommandResult
+            result.CommandResultInternal
                 .Children
                 .Should()
                 .ContainSingle(o =>
                     ((CliOptionResultInternal)o).Option.Name == "--inner1" &&
                     o.Tokens.Single().Value == "argument1");
-            result.CommandResult
+            result.CommandResultInternal
                 .Children
                 .Should()
                 .ContainSingle(o =>

src/System.CommandLine/ParseResult.cs

@@ -52,7 +52,8 @@ internal ParseResult(
             Configuration = configuration;
             _rootCommandResult = rootCommandResult;
             // TODO: Why do we need this?
-            CommandResult = commandResultInternal;
+            CommandResultInternal = commandResultInternal;
+            CommandResult = commandResultInternal.CommandResult;
             this.valueResultDictionary = valueResultDictionary;
             // TODO: invocation
             /*
@@ -101,25 +102,10 @@ internal ParseResult(
         /// <summary>
         /// A result indicating the command specified in the command line input.
         /// </summary>
-        internal CliCommandResultInternal CommandResult { get; }
- 
-        private CliCommandResult? commandValueResult = null;
-        public CliCommandResult CommandValueResult
-        {
-            get
-            {
-                if (commandValueResult is null)
-                {
-                    commandValueResult = new CliCommandResult(CommandResult.Command, CommandResult.Tokens.Select(t=>t.Location) );
-                }
-                return commandValueResult;
-            }
-        }
-
-        public IEnumerable<CliValueResult> AllValueResults
-            => valueResultDictionary.Values;
-
+        // TODO: Update SymbolLookupByName to use CommandValueResult, then remove
+        internal CliCommandResultInternal CommandResultInternal { get; }
 
+        public CliCommandResult CommandResult { get; }
 
         /// <summary>
         /// The configuration used to produce the parse result.

src/System.CommandLine/Parsing/CliValueResult.cs

@@ -10,7 +10,7 @@ namespace System.CommandLine.Parsing;
 /// </summary>
 public class CliValueResult : CliSymbolResult
 {
-    private CliValueResult(
+    internal CliValueResult(
         CliValueSymbol valueSymbol,
         object? value,
         IEnumerable<Location> locations,
@@ -26,42 +26,6 @@ private CliValueResult(
         Error = error;
     }
 
-    /// <summary>
-    /// Creates a new ValueResult instance
-    /// </summary>
-    /// <param name="argument">The CliArgument the value is for.</param>
-    /// <param name="value">The entered value.</param>
-    /// <param name="locations">The locations list.</param>
-    /// <param name="outcome">True if parsing and converting the value was successful.</param>
-    /// <param name="error">The CliError if parsing or converting failed, otherwise null.</param>
-    internal CliValueResult(
-            CliArgument argument,
-            object? value,
-            IEnumerable<Location> locations,
-            ValueResultOutcome outcome,
-            // TODO: Error should be an Enumerable<Error> and perhaps should not be here at all, only on ParseResult
-            string? error = null)
-        : this((CliValueSymbol)argument, value, locations, outcome, error)
-    { }
-
-    /// <summary>
-    /// Creates a new ValueResult instance
-    /// </summary>
-    /// <param name="option">The CliOption the value is for.</param>
-    /// <param name="value">The entered value.</param>
-    /// <param name="locations">The locations list.</param>
-    /// <param name="outcome">True if parsing and converting the value was successful.</param>
-    /// <param name="error">The CliError if parsing or converting failed, otherwise null.</param>
-    internal CliValueResult(
-        CliOption option,
-        object? value,
-        IEnumerable<Location> locations,
-        ValueResultOutcome outcome,
-        // TODO: Error should be an Enumerable<Error> and perhaps should not be here at all, only on ParseResult
-        string? error = null)
-    : this((CliValueSymbol)option, value, locations, outcome, error)
-    { }
-
     /// <summary>
     /// The CliSymbol the value is for. This is always a CliOption or CliArgument.
     /// </summary>
@@ -89,15 +53,6 @@ internal CliValueResult(
             ? default
             : Value;
 
-
-    /// <summary>
-    /// Gets the locations at which the tokens that made up the value appeared.
-    /// </summary>
-    /// <remarks>
-    /// This needs to be a collection because collection types have multiple tokens and they will not be simple offsets when response files are used.
-    /// </remarks>
-    public IEnumerable<Location> Locations { get; }
-
     /// <summary>
     /// True when parsing and converting the value was successful
     /// </summary>

src/System.CommandLine/Parsing/SymbolLookupByName.cs

@@ -43,7 +43,7 @@ private List<CommandCache> BuildCache(ParseResult parseResult)
             return cache;
         }
         cache = [];
-        var commandResult = parseResult.CommandResult;
+        var commandResult = parseResult.CommandResultInternal;
         while (commandResult is not null)
         {
             var command = commandResult.Command;