kathleendollard / command-line-api-starfruit Goto Github PK

Some of the kinds of CLI creation for validation is predictable:

[Range(7, 150)]
public int MyValue { get; set; }

Which could be handled by an IValidation that produced the right validation. But more complex stuff, like the folks requesting FileInfo relative to another directory might be better served by a construct that combined type with validation. This would ensure validation was aligned with the argument type. Something like

// Derive classes that contain the situation specific data needed for the CommandMaker step
public class WrappedArgumentType
{
   public Type Type {get; set; }
   
}

Initially I thought we might have numerous source formats, such as JSON and made the standard rules based on name only. This lets you use your own namespace, leverage System.ComponentModel etc.

Current thinking is that JSON configuration of a model may be an anti-pattern.

A newer understanding is to make sure the two primary scenarios: Reflection and Roslyn Source Generation work well. They are both best served with strongly typed attributes. Us supplying those attributes will reduce fragmentation in the ecosystem. Since rules are flexible, attribute variations can be considered later. Also, it would be easy enough for a JSON source to use the name of the type (stripped of "Attribute") as a name in the definition.

As a result, strongly typed rules have replaced main rules in the central "GeneralAppModel" project. Named rules have moved to a sample as this illustrates how to create a set of rules for a quite different scenario.

Since named rules came first, we can stay alert to places that abstraction still bleeds in.

I think Range may be an easy initial validator to supply.

This issue is to have a home for considering how to incorporate AppModels into DragonFruit style projects.

On approach:

        static void NewMain2(Dotnet myArgs)
        {
            // Execute this
            switch myArgs
            {
               case Install i => RunDotnetToolInstall();
            }
        }

Updated title and description...

We need to determine if this is supported in System.CommandLine binding

If we add this, it will be via a Command attribute on the parameter now, and perhaps later implied by type.

Also removing BeEquivalent per #21

Have the main Playground sample be DragonFruit like (no string args)

Arguments having a limited set of allowed values (separate from enums) is a System.CommandLine feature that should be supported.

System.CommandLine only sets properties that have a public setter, or that have a constructor with a corresponding name.

Currently AppModels user all properties. This should be restricted to those that will actually have their values set.

• More extension overloads for creating root commands
• Fill an existing command (for hosting)
• Easy access to defined strategies

This signature

  public RuleGroup<T> Add(IRule rule)

skips strong typing on in setting rules, such as setting them in StandardRules. Or, we could remove the Add on RuleGroup and have special purpose methods as was started in RuleSetArgument.

This was originally done this way because rules were anticipated to do more than one thing. Its not looking that way.

New standard rules:

Classification as Command:

• Has CommandAttribute
• Name ends with "Command"
• Is a Type (as opposed to parameter or property)

Classification as Argument:

• Has ArgumentAttribute
• Name ends with "Arg"

Classification as Option:

• Not classified as Argument or Command

SubCommands found based on:

• Type derived directly from parent commands type
• Recommended, but not required that they also be nested

Argument details:

• Name via ArgumentAttribute Name property
• Name via property or parameter name with "Arg" removed
• Description via ArgumentAttribute Description property (via XML docs and other means future)
• IsHidden via ArgumentAttribute IsHidden property
• Required via ArgumentAttribute Required property
• Aliases via ArgumentAttribute Aliases property

[ ] DefaultValue via DefaultValueAttribute - explore putting in ArgumentAttribute
[ ] AllowedValues via AllowedValuesAttribute - explore putting in ArgumentAttribute
[ ] Arity via ArityAttribute - explore putting in ArgumentAttribute

Option details:

• Name via OptionAttribute Name property
• Name via property or parameter name
• While its use is probably rare, if Name ends with "Option" it will be removed
• Description via OptionAttribute Description property (via XML docs and other means future)
• IsHidden via OptionAttribute IsHidden property
• Required via OptionAttribute Required property
• Aliases via OptionAttribute Aliases property

[ ] DefaultValue via DefaultValueAttribute - explore putting in OptionAttribute
[ ] Arity via ArityAttribute - explore putting in OptionAttribute

Command details:

• Name via CommandAttribute Name property
• Name via type or method name
• While its use is probably rare, if Name ends with "Command" it will be removed
• Description via CommandAttribute Description property (via XML docs and other means future)
• IsHidden via CommandAttribute IsHidden property
• Required via CommandAttribute Required property
• Aliases via CommandAttribute Aliases property
• TreatUnmatchedTokensAsErrors via CommandAttribute TreatUnmatchedTokensAsErrors property

This will require completing the ComplexAttributeRule

When using CreateInstance you need to create a binder with the correct type for the executing command. This information is currently not available.

Design: CommandDescriptors record their Command. The descriptor tree can be perused to find the command, and the type in the Raw property. This design could in the future be optimized to a dictionary, if perusal proves slow in a large tree (but only one perusal is expected)

There are a couple of things that might be improved:

• Do we really need the strategy and rulegroup extensions? This pattern might make more sense for strategies other than the current relatively full one, but it's a lot of ceremony for the strategy definition
• Directly instantiating the rules seems super clunky, especially for those with intense type parameters

Customers will write code like that in StandardRules.cs as the second most common thing they do, second only to defining CLIs by building methods and types.

The presence of the DerivedTypeRule means that Strategies are specific, not general. But the same rule will be available in source generation and if with a bit of a brain stretch we can imagine a key or structural approach that indicates which JSON nodes are children of other JSON nodes.

There may well be Specific rules that force a Strategy to be specific, but I would like to make that as much of a niche case as possible.

This is also a simple case for putting in the first abstract general/derived specific cases. We will also have them for ArgumentTypeInfo (a new rule) and probably Validation.

Boolean attributes should be settable to true by just including the attribute.
They also need a property that can be set to false for clarity.

Currently this requires special behavior on the part of the attribute. This should be expressable in rules.

The single test is a killer to compare and doesn't allow variations to check report output

I've hacked a bit of our fluent assertion stuff, including ReflectionAppModel.Utils.TestType(..) where I believe it should have been failing, but required me to do extra explicit checks.

(side note: We can't do this for properties because property defaults are rewritten, aka smoke and mirrors)

The intent is that option argument rules are separate from argument rules.

The current behavior is that normal argument rules are used for both.

Should we fix this?

The GeneralAppMode project needs to be agnostic on the thing that it will evaluate. This is generally true but a dependency on Attribute snuck into the AttributeRules.

Replace this with Attribute methods in the ReflectionAppModel project. This may include creating a new class for source specific utilities, and moving the existing abstract classes from DescriptorMakerBase.

The CommandAssertions seem to be a good balance of finding a common point of comparison and reporting failures, while retaining readable tests with definitions and checks near each other/easy to find.

@jonsequitor I'd like to review these tests with you prior to changes.

For example, there should be select symbol rules. Add a check for that and any others that seem obvious. This can grow later.

I changed the name. Errors should be reported in Report, and probably block usage if an invalid descriptor would result.

This is distinct from checking the descriptor. We need to be able to separately report and check:

• Strategy and Rules (perhaps rules separately)
• Descriptors (done)
• System.CommandLine (sanity checks, should be in that repo if done)

Not sure what is going on here. But binding is not working with unless the capitals match

ArgumentType is problematic because we don't actually know how the ArgumentType is best represented in the Specific layer, and it must be retrieved via the specific layer (as is done with Reflection). The Type type is actually a Reflection construct, invalid in JSON and probably invalid in Roslyn.

The ArgumentTypeInfo class has the goal of having both a general and a specific way to identify the class. In practice, we anticipate Reflection->Runtime types and Roslyn->Source generation without those streams crossing, and JSON is an intellectual exercise for the present. So it seems silly to force the type into a general format and back again - since that format can't be any better than a string name, with associated hassles about how full of a name to use.

There are a few gnarly details still to work out.

Currently NamedAttributeWithPropertyRule reuses the common retrieval of matching attributes. Currently the first is used. It should be first checking for PropertyName match.

A case that would otherwise fail would be

[Argument]
[Description("Boo!")]
public string MyString {get;set;}

if the rules are

            .Add(new NamedAttributeWithPropertyRule<string>("Argument", "Description"))
            .Add(new NamedAttributeWithPropertyRule<string>("Description", "Description"))
            .Add(new NamedAttributeWithPropertyRule<string>("Description", "Value"))

Currently command line args are expected to be gathered from the Environment instance. This is awkward and samples. Need an alternative strategy

Not sure if this is a bug or oversight, but it doesn't seem to be working.

This will probably be via a new property on OptionAttribute.

It will be quite annoying to CLI developers if they have to backtrack from System.CommandLine error messages to determine failures in entering their design. Of course, if it's possible the design of the Rule itself should prohibit mistakes. This lists things we can check and we can work out later whether the protection can be inherent in the rule:

[ ] Default value is assignable to argument type
[ ] Allowed values are assignable to argument type

Arguments are not being filled with data for Playground samples

I believe this is because the property or parameter name ends with "Arg" which is stripped out before creating the CommandLine syntax tree.

We should probably switch everything to explicit binding.

Prior to creating the System.CommandLine symbols, a check of the correctness of the descriptor would be both straightforward and valuable. It would be much better to get a model friendly message than a System.CommandLine exception.

This issue will collect the things we should check for

• Name can't be null, empty or whitespace. If we need roots, we need to create roots

Work is partly done for OptionArguments to have different rules than arguments. For example, Option might have an OptionRequired and a ArgumentRequired or have an ArgumentName

Currently the mechanism to ignore a candidate is to list it's name in "names to ignore". This is not very flexible and this should probably be raised to a rule.

For example, this could support a "Ignore" attribute.

One of the forms CLI design can be expressed in is:

[executable] [area] [action/desire] [specific/object]

In very complex cases, area may have layers.

This would most comfortably be expressed with the area as a class, analogous to a Controller in ASP.NET. I initially rejected this because of the case dotnet [project] add package/reference. However, I'm reconsidering it because it would have significant value in organizing and helping to design CLI. (Project is not actually stated, but will be optional in the future and is the logical breakdown).

Should we consider munging things a bit to force areas to appear together for logical grouping of the code. My first idea is to use an underscore, because it reads very nicely and underscores are out of fashion for other purposes.

You'll see in this sample that I have a preference for description/help text to be isolated. This makes it easier to be consistent in that text, might aid localization, and makes the code running your application much prettier.

Something like (this is about 1/8 of the entire dotnet CLI):

public class ProjectCommand
{
    public string? ProjectArg { get; set; }

    public int Add_Reference(string projectPathArg,
                            [Aliases("-f")] string? framework,
                            bool interactive)
    { return default; }

    public int Add_Package(string packageNameArg,
                            string? version,
                            string? framework,
                            [Aliases("-n")] bool? noRestore,
                            [Aliases("-s")] string? source,
                            DirectoryInfo? packageDirectory,
                            bool interactive)
    { return default; }

    public int Remove_Reference(string projectPathArg, string? framework)
    { return default; }

    public int Remove_Package(string packageNameArg, bool interactive)
    { return default; }

    public int List_Package(string? framework, string? source, bool outdated, bool deprecated,
                            bool includeTransitive, bool includePrerelease, bool highestPatch, bool highestMinor,
                            FileInfo? config, bool interactive)
    { return default; }

    public int List_Reference()
    { return default; }
}

We need to add the remaining System.CommandLine features that can be mapped fairly easily.

Question: How can we use Generic Host alongside with the Starfruit?

Context: A lot of open-source projects incorporate Attribute-first approach. But it looks like not so many projects could be used together with Generic Host that easily. AFAIK, Starfruit app model is designed with the idea to hide the complexity and get rid of the dependency on System.CommandLine. Currently, it is only possible to wire up Generic Host via CommandLineBuilder.UseHost method

Examples of open-source projects with attribute-first API model:

• https://github.com/commandlineparser/commandline
• https://github.com/natemcmaster/CommandLineUtils
• https://github.com/Tyrrrz/CliFx
• https://jasperfx.github.io/oakton/documentation/getting_started/

Let's do this explicitly for now. Perhaps it should be a rule, but I'd rather see it as a System.CommandLine option so the user could be more involved.