Description
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.
- SimpleCLI adds a --verbosity option which has a large list of accepted values, SimpleCLI customizes the default HelpBuilder to make the help text more readable for this option. If app Foo wants to create their own HelpBuilder implementation it has to add in the verbosity customization itself which doesn't seem right since SimpleCLI is the one creating the verbosity option.
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?
Activity
jonsequitur commentedon Jun 9, 2023
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: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?This property is available via the public property
Option.HelpName
.seanadams9 commentedon Jun 9, 2023
I have AcceptOnlyFromAmong setup the problem is there are so many it makes the first column abnormally long for a global option.
customization:
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 commentedon Jun 9, 2023
I see. Another option is to replace the whole options section using
GetLayout
orCustomizeLayout
.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.