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

[robcao]

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.

[martijnvanschie]

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]

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]

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

spectre.console/src/Spectre.Console.Cli/Internal/Modelling/CommandModelBuilder.cs

Lines 125 to 139 in cecfdc3

	// 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);
	}

spectre.console/src/Spectre.Console.Cli/Internal/Modelling/CommandInfoExtensions.cs

Lines 24 to 56 in cecfdc3

	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:

spectre.console/src/Spectre.Console.Cli/Internal/Modelling/CommandModelBuilder.cs

Lines 91 to 94 in cecfdc3

	// 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 :)

[bwets]

Im having this issue too, and can confirm that the workaround of reimplementing the options on descendants works. It is a massive pain though... Any idea if a fix is coming? I have been debugging in the sources, but franckly this is quitte complex and I doubt I would be able to produce a decent PR to fix this, or a PR that doesnt break other stuff. Help!

Side question: did anyone try rolling back to an earlier version that didnt have this? If so, was there an impact on other stuff?

[patriksvensson]

@FrankRay78 Can you take a look at this and see if this has something to do with your changes?

[FrankRay78]

Self-assigned. I'm sure I can sort this out, however I've got a couple of other maintainer tasks to complete first.

[FrankRay78]

I tried @robcao's MRE with each of the following NuGet versions, and this bug happens with each one:

0.49.0
0.48.0
0.47.0
0.46.0
0.45.0

So it's not a recent regression.

[bwets]

Any news on this? Its a pretty major bug of the base functionality of Spectre.Console... I would assume some more focus would be put in this?

[FrankRay78]

@bwets someone will shortly jump in and say 'it will be fixed when a PR is submitted', but I'll save you that response

Honestly, there's a heap I would like fixed, I think we are at 200+ issues, but short on reviewer and maintainer capacity.

If you want to have a go at fixing it, I would be happy to guide/review. But otherwise, me personally, might not get around to fixing it for another 6 months.

I've got multiple other Cli PR's open, not been reviewed yet, which I personally would like merged. But patience seems to be the best course when no body is getting paid here.

Sorry I don't have a better answer.

[carlin-q-scott]

@FrankRay78 Did you look at my documentation of the issue earlier in this thread? I'd fix it myself, but I'm unsure if the "bug" is intentional. Code was added to block inheritance of CommandOption Properties. I could remove code to fix the issue, but is that the right thing to do?

[FrankRay78]

@carlin-q-scott I'm squeezed at the moment, which is why I'm not jumping in with code, but if you have time, why not pull down master, apply the fix, and see what, if any, tests break? If all green, that tells me we just want to add a few to ensure coverage. If some break, we'll need to consider if the breaking change is warranted - I'll seek guidance from the other maintainers. But I'll be happy to advise and help.