[Breaking] Remove CommandResult

[KathleenDollard]

ParseResult is created at the end of parsing to hold the results. It does not contain any information used by parsing and is a prime candidate holding the ValueResults, which can be held sparsely with the default value for the type supplied when there is no value entered (default values are handled in the ValueSubsystem, not during parsing).

ArgumentResult and OptionResult are important to parsing and we want to minimize risk changing them. We are adding a ValueResult, because we think CLI authors rarely care where a value came from. When they do, the symbol will be available.

The future role of CommandResult is unclear. It appears is unnecessary for retrieving values, and the symbol tree would be available when the shape is of interest.

Thus, the plan is to remove CommandResult from the public API.

Please suggest additional known scenarios we need to consider.

Known scenarios that will need alternatives

Please suggest additional known scenarios we need to consider and any issues with the proposed solutions.

Sibling command default values

If the default for a sibling command's argument or option is needed, that would now come via the ValueSubsystem. Only type defaults are provided by the core parser layer and the ParseResult.

If needed, we could expose a SymbolByName method on configuration/parser/rootcommand, possibly with proximity semantics.

Retrieving values from different options/arguments on a command and a command ancestor with the same name by name

This scenario seems unusual, although not pathological.

Imagine a case where the root command and child commands have different options or arguments that have the same name, and the CLI author needs to retrieve more than one.

Today and in the future the CLI author can retrieve each value via its symbol reference.

Today, the CLI author can retrieve the leaf-most result by name and ask for its parent, search the CommandResult.Children and retrieve the ancestor value by name.

In the future, the CLI author will need to retrieve the symbol from the tree and search via symbol.

If we hear there is more need of this, we can provide helper methods.

[KalleOlaviNiemitalo]

If a command has aliases, then can the command handler detect which alias was used, in order to include that in diagnostic messages? This is currently available as CommandResult.IdentifierToken.

[KalleOlaviNiemitalo]

Also, this seems likely to break what I'm doing for redaction in #1795.

[KathleenDollard]

It's great that you have that library, and when we are ready I hope you will help us refine some things in Powderhouse based on your scenario.

There are a couple of things that may make your scenario work really well. The ValueResult contains a location, which will be especially helpful with response files and nested response files. You'll have to wait a bit to see this because we have some merge issues with CI we are resolving with the new branch, but here is the shape:

    public Location(string text, string source, int start, Location? outerLocation, int offset = 0)
    {
        Text = text;
        Source = source;
        Start = start;
        Length = text.Length;
        Offset = offset;
        OuterLocation = outerLocation;
    }

    public string Text { get; }
    public string Source { get; }
    public int Start { get; }
    public int Offset { get; }
    public int Length { get; }
    public Location? OuterLocation { get; }

    public bool IsImplicit
        => Source == Implicit;

    public override string ToString()
        => $"{(OuterLocation is null ? "" : OuterLocation.ToString() + "; ")}{Text} from {Source}[{Start}, {Length}, {Offset}]";

}

That gives you a good view into where a value came from. It does not give you the shape of the tree.

We need that, and need to replace CommandResult for knowing the executing command, and the parents, with the following scenarios in mind:

• There may be noValueResults on an executing command
• The symbols in existing ValueResults maybe attached at multiple places in the tree
• The commands maybe attached at multiple places in the tree (I need to check that, but think its true

So, it is not quite as simple as using the symbols instead of the CommandResult, but I think it is worth exploring further because these are likely to be via a subsystem, and thus doesn't have to be the simplest part of the API as we tune the API for CLI authors.

[baronfel]

Chiming in that the .NET CLI currently uses CommandResult for a variety of reasons (good and bad :D ).

Some of the use cases are cross-cutting concerns like Telemetry Reporting on command usage, but we also do have to check the .Children for use cases like detecting if particular commands or options are in use from outside the Parser.

[KalleOlaviNiemitalo]

Another thing to consider is completions, especially the ability to implement richer protocols for the communication between the editor (shell) and the completion provider rather than rely on dotnet-suggest. (Such a system could support keeping the completion provider alive in a process for lower latency; telling the editor to complete file names that have a specific extension; showing descriptions and tooltips rather than only the insert string.) I think this is going to need a way for a subsystem to parse the command line and understand how each token in it relates to a defined command, option, or argument.

[KathleenDollard]

We'll keep the concept. Naming will be a bit weird for a while because CommandResult is an internal type and we need a publicly facing type with a different name. For now, I am paralleling ValueResult and calling the public one CommandValueResult.

Thanks for the use cases. We still may replace this type with methods, but not unless that fills the known use cases, and after all or most tests are reinstated as a backstop.