Show default values of non-required options in help

[Tyrrrz]

Suggested approach:

• Resolve target command
• Instantiate the command but don't fill the values
• Get the values of the properties on the freshly-instantiated command, filter out those that are default and show them in help text

[domn1995]

To clarify the spec for this feature, how do we want to handle types without "nice" ToString() outputs? Just print out the result of ToString() and if it's ugly it's ugly? For types that implement IEnumerable we can just string.Join() the values of course, though.

Another way to say this is that any of the types supported by the built in value conversions get nicely formatted default values and the rest are not guaranteed. @Tyrrrz, what do you think?

[Tyrrrz]

Totally forgot about that :/

We could check if ToString() is overriden but even if ToString() is overridden on a type, there's no guarantee that it will return the value in the same form it expects to parse. I think for now we should just handle the basic types and IEnumerable<T> of those via string.Join.

[domn1995]

Yup, that sounds good. How do we want to handle types that do NOT override ToString()?

Ignore them? Print the type name? Print some placeholder value indicating a warning?

Another option is adding a Default property to CommandOptionAttribute where the user can supply a string to print as the default value. If they supply it, we just print that, otherwise we defer to our implementation.

With that last option, we could add an analyzer that checks whether the type doesn't override ToString() and show a warning suggesting they override or set the Default property of the CommandOptionAttribute.

[Tyrrrz]

Let's only handle these types:

CliFx/CliFx/Domain/CommandArgumentSchema.cs

Lines 37 to 50 in 7e4c6b2

	var primitiveConverter = PrimitiveConverters.GetValueOrDefault(targetType);
	if (primitiveConverter != null)
	return primitiveConverter(value);
	// Enum
	if (targetType.IsEnum)
	return Enum.Parse(targetType, value, true);
	// Nullable
	var nullableUnderlyingType = targetType.GetNullableUnderlyingType();
	if (nullableUnderlyingType != null)
	return !string.IsNullOrWhiteSpace(value)
	? ConvertScalar(value, nullableUnderlyingType)
	: null;

Another option is adding a Default property to CommandOptionAttribute where the user can supply a string to print as the default value. If they supply it, we just print that, otherwise we defer to our implementation.

That would break single source of truth, I don't like that. If the user decides to change the property type or the actual default value, they will have to remember to also update the default string value in attribute.

[domn1995]

That would break single source of truth, I don't like that. If the user decides to change the property type or the actual default value, they will have to remember to also update the default string value in attribute.

Yeah, I don't like that either, but it would be mostly remedied by an analyzer catching it.

Still not sure what behavior we want for the unsupported types... I think I'll print something like Default: unknown value for now and eventually have an analyzer that warns on it.

[Tyrrrz]

I vote we just don't do anything for unsupported types, i.e. leave the current behavior.

[domn1995]

I vote we just don't do anything for unsupported types, i.e. leave the current behavior.

I think I'm ok with that if we get an analyzer warning eventually 😎

[Tyrrrz]

@domn1995 after we merge this in, I'm gonna refactor code a bit so don't pick anything else until I'm done :)