Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Command line options inherited from branch settings are not being bound. #1612

Open
robcao opened this issue Aug 18, 2024 · 3 comments
Open
Labels
area-CLI Command-Line Interface bug Something isn't working needs triage

Comments

@robcao
Copy link

robcao commented Aug 18, 2024

Information

  • OS: Windows
  • Version: 0.49.1
  • Terminal: Windows Terminal

Describe the bug
A clear and concise description of what the bug is.

I believe this is the same bug as mentioned in #428, where command options from parent settings aren't being taken into account / parsed.

I'm trying to pass in command options that are stored in a parent settings class, but these command options all appear to be ignored.

using Spectre.Console.Cli;

namespace Repro;

public class BranchSettings : CommandSettings
{
	[CommandOption("--my-value")]
	public string MyValue { get; set; } = string.Empty;
}

public class BranchedCommandSettings : BranchSettings
{
}

public class BranchedCommand : Command<BranchedCommandSettings>
{
	public override int Execute(CommandContext context, BranchedCommandSettings settings) => 0;
}

And my program file:

using Repro;
using Spectre.Console.Cli;

CommandApp app = new();

app.Configure(config =>
{
	config.ValidateExamples();
	config.AddBranch<BranchSettings>("branch", user =>
	{
		user.SetDescription("Branch.");

		user.AddCommand<BranchedCommand>("command")
			.WithDescription("Command under a branch.")
			.WithExample("branch", "command", "--my-value", "abc");
	});
});

return app.Run(args);

To Reproduce

I have created a minimum reproducible example here: https://github.com/robcao/spectre-console-branch-inheritance

To reproduce, download the example, navigate to the repro directory, and run dotnet run.

The application fails with the following exception:

dotnet run

Error: Validation of examples failed.

Error: Unknown option 'my-value'.

       branch command --my-value abc
                      ^^^^^^^^^^ Unknown option

Expected behavior
A clear and concise description of what you expected to happen.

I expect command options defined on parent setting classes to be bound.

When running dotnet run branch command --my-value abc, I would expect the following:

  • ValidateExamples should not fail
  • In the BranchedCommand .Execute method, the value of BranchedCommandSettings.MyValue should be abc.

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.


Please upvote 👍 this issue if you are interested in it.

@robcao robcao added bug Something isn't working needs triage labels Aug 18, 2024
@github-project-automation github-project-automation bot moved this to Todo 🕑 in Spectre Console Aug 18, 2024
@FrankRay78 FrankRay78 added the area-CLI Command-Line Interface label Sep 16, 2024
@martijnvanschie
Copy link

I'm experiencing the same issue. It seams that the derived options are not honored in the parent class.
Or perhaps I'm not doing it wrong, but this is the most basic version that looks like the example from the documentation.

These are the settings classes.

public class DefaultCustomerSettings : CommandSettings
{
    [CommandOption("-o|--output <FORMAT>")]
    [Description("Output format.  Allowed values: json, jsonc, none, table, tsv, yaml, yamlc.")]
    [DefaultValue(OutputFormat.json)]
    public OutputFormat Output { get; set; }
}

public class CreateCustomerSettings : DefaultCustomerSettings
{
    [CommandArgument(0, "<CUSTOMER_NAME>")]
    public string CustomerName { get; set; }
}

Then the command

public override async Task<int> ExecuteAsync(CommandContext context, CreateCustomerSettings settings)
{
    AnsiConsole.MarkupLine($"Creating new customer [[{settings.CustomerName}]].");
    await _customerService.CreateNewCustomerAsync(settings.CustomerName);
    AnsiConsole.MarkupLine("Done");
    return 0;
}

But the console does not show the options. And the value is always the default value.
As you can see, the output option is not show.

c:\Test> dotnet run -- customer create -h

DESCRIPTION:
Create customer the the customers table.

USAGE:
    aztai customer create <CUSTOMER_NAME> [OPTIONS]

ARGUMENTS:
    <CUSTOMER_NAME>

OPTIONS:
    -h, --help    Prints help information

@carlin-q-scott
Copy link

I found a workaround that will probably help someone find the root cause of the issue.

Replace the properties inherited from your base CommandSettings class and forward them to it:

public class BranchSettings : CommandSettings
{
	[CommandOption("--my-value")]
	public string MyValue { get; set; } = string.Empty;
}

public class BranchedCommandSettings : BranchSettings
{
	[CommandOption("--my-value")]
	public string MyValue
	{
		get => base.MyValue;
		set => base.MyValue = value;
	}
}

Most likely the reflection code used by this library forgot to include the flag for inheritted members.

@carlin-q-scott
Copy link

carlin-q-scott commented Nov 18, 2024

I found the code responsible for this behavior, and it wasn't because of the reflection flag because that decision was intentional.

// Any previous command has this option defined?
if (command.HaveParentWithOption(option))
{
// Do we allow it to exist on this command as well?
if (command.AllowParentOption(option))
{
option.IsShadowed = true;
parameters.Add(option);
}
}
else
{
// No parent have this option.
parameters.Add(option);
}

public static bool AllowParentOption(this CommandInfo command, CommandOption option)
{
// Got an immediate parent?
if (command?.Parent != null)
{
// Is the current node's settings type the same as the previous one?
if (command.SettingsType == command.Parent.SettingsType)
{
var parameters = command.Parent.Parameters.OfType<CommandOption>().ToArray();
if (parameters.Length > 0)
{
foreach (var parentOption in parameters)
{
// Is this the same one?
if (option.HaveSameBackingPropertyAs(parentOption))
{
// Is it part of the same settings class.
if (option.Property.DeclaringType == command.SettingsType)
{
// Allow it.
return true;
}
// Don't allow it.
return false;
}
}
}
}
}
return false;
}

I think this is saying that if there's a parent command (aka branch), then the setting property will only be set it if the parent has the same settings type. I think we're all confused by this decision. This also may not have been their intent based on this comment:

// Things get a little bit complicated now.
// Only consider a setting's base type part of the
// setting, if there isn't a parent command that implements
// the setting's base type. This might come back to bite us :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CLI Command-Line Interface bug Something isn't working needs triage
Projects
Status: Todo 🕑
Development

No branches or pull requests

4 participants