command-line-api icon indicating copy to clipboard operation
command-line-api copied to clipboard

[Proposal] Unifying public parts of ArgumentResult and OptionResult to ValueResult and isolating parser support

Open KathleenDollard opened this issue 1 year ago • 2 comments

It may be valuable to have a single ValueResult. This is a working document to help with that decision.

A benefit is that binding and other calling code, including that directly using the core parser, would not need to worry about two types.

It is also desirable, or perhaps necessary, to separate the public surface from the internal surface of the parser.

Existing differences:

  • Option has an internal `IdentifierToken (passed to constructor)
  • Option has a public Implicit property and support for it via the CliToken
  • The symbols are held as Option and Argument symbols
  • Option delegates data/value work to it's arguments
  • Argument has OnlyTake as a public method for custom parsing. It is problematic after parsing.

An alternate design:

  • internal class ParsingOptionResult

    • ValueResult
    • internal values used during parsing
    • Determine exactly what Implicit means and who needs it (code differs from what I thought I understood)
  • internal class ParsingArgumentResult

    • ValueResult
    • internal values used during parsing
    • If we reinstate custom parsing, find a solution to OnlyTake that does not put it on the public ValueResult
  • internal class ParsingCommandResult

    • CommandResult
    • internal values used during parsing
  • public class ValueResult

    • GetRawValue()
    • GetValueOrDefault<T>()
    • ToString()
    • Symbol
    • Parent (this is needed as a symbol can be parented to multiple commands)
    • Location
    • Errors available, possibly as an extension method
    • Results available for other symbols, but possibly just on ParseResult, not as a method on all results
    • Unify GetValue and GetValueOrDefault or provide names that are more clear, as GetValue also returns the default value
  • public class CommandResult

    • Command
    • ToString()
    • Parent (this is needed as a symbol can be parented to multiple commands)
    • Location
    • Children
    • Errors available, possibly as an extension method

The internal types would be used during parsing (when the tokens are converted to results) and then the ValueResult instances collected into the public ParseResult.

KathleenDollard avatar Mar 21 '24 13:03 KathleenDollard

I worry about how "isolating parser support" will affect completions, especially when the completion list of an option should depend on arguments of the command. (Imagine supporting completions for msbuild project.csproj -target: where the set of available targets depends on what the project defines.)

KalleOlaviNiemitalo avatar Mar 21 '24 13:03 KalleOlaviNiemitalo

I think that would be as easy. The parser would parse that, and then the values would be available to the completions subsystem.

KathleenDollard avatar Mar 21 '24 22:03 KathleenDollard