OptionAttribute overload with custom name and only one shortname

[honguyenminh]

Currently the option attribute only have an overload for (string name, char[]? shortNames)

Is it possible to also have an overload with (string name, char? shortName)? This can easily be the case, since most of us probably don't need to have multiple shortnames if we just want a custom name.

[mayuki]

Thanks, you're right, that's a good point. Maybe params would work too. I will consider it.

[honguyenminh]

Yeah params would be great. This way API contract shouldn't be broken, and old code should just compile as usual. Tell me if you want me to implement the change, should be a quick PR.

[mayuki]

Sorry, but I am in the middle of an important task. It would be very helpful if you could be a contributor.

[honguyenminh]

Alright, so, in its purest and simplest form, ctors for OptionAttribute would only look something like:

public OptionAttribute(string name, params char[] shortNames)
{
    Name = name;
    ShortNames = shortNames;

    if (string.IsNullOrWhiteSpace(name))
        throw new ArgumentException("Option name cannot be empty.");
}

public OptionAttribute(params char[] shortNames)
{
    ShortNames = shortNames;
}

Yes there is no default empty constructor. However this will break the API contract. TLDR It's the nullable on the arrays.

At https://github.com/mayuki/Cocona/blob/831d70494d35d98bdb2a4ab4f5be5e1b1db9b6e4/src/Cocona.Core/OptionAttribute.cs#L41 Yes, I agree that no one, in their right mind, would ever use this properly with nulls. The ONLY case where people would use null array here is to set a name, but not setting any short names. Then they would call OptionAttribute("name", null) but this is pretty much a hack. The new one is nicer, you can set a name without any short names, and no null check is needed, but this will break existing code that uses the hack above. Consumer will have to change their code to OptionAttribute("name"). We can also make the char array nullable, API contract won't be broken, but I don't like that approach. The choice is yours. Not nullable is best with shortest code and no additional checks, but breaks API contract. Nullable won't, but doesn't look good, and we'll have to check for null.

https://github.com/mayuki/Cocona/blob/831d70494d35d98bdb2a4ab4f5be5e1b1db9b6e4/src/Cocona.Core/OptionAttribute.cs#L54 Why is this even nullable? We have the empty constructor. There's no need, for anyone, ever, to pass null here. So yeah, this shouldn't be a problem, it breaks the contract, but no one uses the contract, so...