Tyrrrz / CliFx

Class-first framework for building command-line interfaces
MIT License
1.5k stars 61 forks source link

Positional arguments listed out-of-order when inheriting from base command class #117

Closed TAGC closed 2 years ago

TAGC commented 2 years ago

Version

2.0.6

Details

In some cases it's useful to have a base class implement ICommand and contain properties representing positional arguments that are common across multiple commands. For example, if you're writing a CLI app to manage customer orders, it might be useful to have something like:

public abstract class AbstractOrderCommand : ICommand
{
    [CommandParameter(order: 0, Description = "The order ID")]
    public long OrderId { get; init; }

    public abstract ValueTask ExecuteAsync(IConsole console);
}

However, if you create a command that inherits from this base class and implements additional positional arguments (at order: 1, order: 2, etc.), these get listed out of order in the help description. For example, given:

public class AddSupplementaryProductsCommand : AbstractOrderCommand
{
    [CommandParameter(order: 1, Description = "The set of products to add as supplementaries to the order")]
    public IReadOnlyCollection<string> SupplementaryProducts { get; init; } = null!;

    // ...
}

When printing the help doc for this command, the following gets displayed:

USAGE
  order add supplementary <supplementaryproducts...> <orderid> [options]
  order add supplementary [command] [...]

DESCRIPTION
  Add one or more supplementary products to an existing order

PARAMETERS
* orderid           The order ID
* supplementaryproducts  The set of products to add as supplementaries to the order

It should instead be:

USAGE
  order add supplementary <orderid> <supplementaryproducts...> [options]
  order add supplementary [command] [...]

DESCRIPTION
  Add one or more supplementary products to an existing order

PARAMETERS
* orderid           The order ID
* supplementaryproducts  The set of products to add as supplementaries to the order

Steps to reproduce

To minimally reproduce, create a CLI app that implements these commands:

public abstract class CommandBase : ICommand
{
    [CommandParameter(order: 0)]
    public long Foo { get; init; }

    public abstract ValueTask ExecuteAsync(IConsole console);
}

[Command("inherit")]
public class InheritFromBaseCommand : CommandBase
{
    [CommandParameter(order: 1)]
    public long Bar { get; init; }

    public override ValueTask ExecuteAsync(IConsole console)
    {
        throw new NotImplementedException();
    }
}

[Command("non-inherit")]
public class DoesNotInheritFromBaseCommand : ICommand
{
    [CommandParameter(order: 0)]
    public long Foo { get; init; }

    [CommandParameter(order: 1)]
    public long Bar { get; init; }

    public ValueTask ExecuteAsync(IConsole console)
    {
        throw new NotImplementedException();
    }
}

Compare the outputs when displaying the help doc for the "non-inherit" command (working as expected)...

PS > .\Example.exe non-inherit
Missing parameter(s):
<foo> <bar>

USAGE
  order non-inherit <foo> <bar> [options]

PARAMETERS
* foo
* bar

OPTIONS
  -h|--help         Shows help text.

Against the output for the "inherit" command:

PS > .\Example.exe inherit
Missing parameter(s):
<bar> <foo>

USAGE
  order inherit <bar> <foo> [options]

PARAMETERS
* foo
* bar

OPTIONS
  -h|--help         Shows help text.
TAGC commented 2 years ago

From a quick inspection, it looks like the issue is with this bit of code, which determines the list of parameters that a CommandSchema is constructed with: https://github.com/Tyrrrz/CliFx/blob/a9ef693dc153894e469f15cfad214cfa6857d94f/CliFx/Schema/CommandSchema.cs#L90-L93

This list of parameters is enumerated in that order by HelpConsoleFormatter: https://github.com/Tyrrrz/CliFx/blob/a9ef693dc153894e469f15cfad214cfa6857d94f/CliFx/Formatting/HelpConsoleFormatter.cs#L74-L82

If I print out the properties of each type of command in the minimal repro code, I get the properties back in different orders:

public static async Task<int> Main()
{
    var doesNotInheritFromBaseCommandProperties = typeof(DoesNotInheritFromBaseCommand).GetProperties();
    var inheritFromBaseCommandProperties = typeof(InheritFromBaseCommand).GetProperties();
    return 1;
}
doesNotInheritFromBaseCommandProperties
{System.Reflection.PropertyInfo[2]}
    [0]: {Int64 Foo}
    [1]: {Int64 Bar}
inheritFromBaseCommandProperties
{System.Reflection.PropertyInfo[2]}
    [0]: {Int64 Bar}
    [1]: {Int64 Foo}

Since ParameterSchema has the Order property associated with it, it might be possible to order the list of parameters by this property before passing it to the CommandSchema constructor e.g.

var parameterSchemas = type.GetProperties() 
    .Select(ParameterSchema.TryResolve) 
    .Where(p => p is not null)
    .OrderBy(p => p.Order)
    .ToArray(); 
Tyrrrz commented 2 years ago

Hi, thanks for the report. Yes, the parameters need to be sorted by order in the help text renderer, it's a mistake that they aren't. PRs are welcome.