dotnet / command-line-api

Command line parsing, invocation, and rendering of terminal output.
https://github.com/dotnet/command-line-api/wiki
MIT License
3.35k stars 376 forks source link

HelpBuilder Extensibility #2215

Open seanadams9 opened 1 year ago

seanadams9 commented 1 year ago

I'm transitioning a CLI library (for this thread, lets call it SimpleCLI) from McMaster CLI to System.CommandLine (latest daily build). SimpleCLI adds telemetry + other common functionalities to enable faster internal CLI creation.

Things were going great until it was time to add in help text customizations. In McMaster CLI the HelpBuilder equivalent class all methods are virtual, giving the caller the ability to customize whatever they need. It also has no reliance on private fields/internal properties and classes.

I'm willing to make the changes and submit a PR but want to understand any feedback before doing so.

1. CustomizationsBySymbol should be virtual and _customizationsBySymbol public.

Would like to do something like this:

switch (parseResult.Action)
{
    case HelpAction helpAction:
        helpAction.Builder.CustomizeSymbol(verbosityOption,
            firstColumnText: text => "--verbosity <debug>",
            secondColumnText: text => "Configure the minimum log level to show.\n" +
            "Allowed values: trace, debug, info, warn, warning, error, critical, fatal, none, off.");
        result = helpAction.Invoke(parseResult);
        break;

    default:
        result = parseResult.Action.Invoke(parseResult);
        break;
}

This isn't possible today wuth CustomizeSymbol not overridable. Making it overridable means the customizationsBySymbol needs to be public so the HelpBuilder implementation has access to those customizations.

2. HelpBuilder shouldn't rely on internal classes/properties or private fields.

I need to customize a few sections, copied over the current HelpBuilder so I would have all the functionality before removing things I don't actually need to touch. This led to copying over internal extension classes and working around private fields/internal properties.

Worst example of this is the SymbolExtensions which relies on an internal option.Argument

Original: return option.Argument

Workaround using reflection to create the same CliArgument

Type sourceType = option.GetType().GetGenericArguments()[0];
Type targetType = typeof(CliArgument<>).MakeGenericType(sourceType);
object[] constructorArgs = new object[] { option.Name };
CliArgument targetArgument = (CliArgument)Activator.CreateInstance(targetType, constructorArgs);
return new[] { targetArgument };

To be fully customizable everything the default is doing should be easily accessible.

3. Why is the default MaxWidth set to int.MaxValue?

As a first time user of System.CommandLine and just letting the standard HelpBuilder write help I thought it was a bug when descriptions that went over the Console.BufferWidth went down to beginning of the next line instead of the start of the 2nd column on the next line. Seems like the default should just be Console.BufferWidth?

jonsequitur commented 1 year ago

Thanks for the detailed writeup!

We're planning to significantly overhaul the HelpBuilder API and this is very useful feedback.

A couple of quick points not directly related to the HelpBuilder API:

Would like to do something like this ... secondColumnText: text => "Configure the minimum log level to show.\n" + "Allowed values: trace, debug, info, warn, warning, error, critical, fatal, none, off.");

The Option.AcceptOnlyFromAmong method configures both validation and completions, and completions, if available, are usually included in the default help output. Did you configure completions and if so, are you not seeing these in your help output?

Worst example of this is the SymbolExtensions which relies on an internal option.Argument

This property is available via the public property Option.HelpName.

seanadams9 commented 1 year ago

I have AcceptOnlyFromAmong setup the problem is there are so many it makes the first column abnormally long for a global option.

Options:
  -?, -h, --help                                               Show help and usage information
  --verbosity                                                  Configure the minimum log level to show.
  <critical|debug|error|fatal|info|information|none|off|trace
  |warn|warning>

customization:

Options:
  -?, -h, --help       Show help and usage information
  --verbosity <debug>  Configure the minimum log level to show.
                       Allowed values: trace, debug, info, warn, warning, error, critical, fatal, none, off.

For Option.HelpName, that would work if I were just looking for the name but GetSymbolDefaultValue(CliSymbol symbol) where that is used needs the full argument so it doesn't write out Hidden args or args without a default value.

Thoughts on MaxWidth?

jonsequitur commented 1 year ago

I see. Another option is to replace the whole options section using GetLayout or CustomizeLayout.

The reason the default is set to int.MaxWidth is that there isn't always a console available, e.g. if you're redirecting output. In these scenarios this effectively turns word wrapping off.

If it's wrapping incorrectly when there is a console present though, that's a bug. When a console is available, it should be passing the correct width in. We'll take a look.