Option constraint to accept only positive integers?

[mristin]

Hi,
First, thanks a lot for all the effort on this library!

Am I correct to assume that currently System.CommandLine does not support constraints on integer options? Is there a convention how to implement such constraints?

I looked int Option{T}.cs and OptionExtensions.cs, but I only see constraints related to strings, files and directories.

[twaalewijn]

I guess you could write your own extension method that adds a validator similar to the ones you've already found in OptionExtensions.cs:

public static Option<int> PositiveOnly(this Option<int> option)
{
    option.Argument.AddValidator(
        a =>
            a.Tokens
            .Select(t => t.Value)
            .Where(v => !int.TryParse(v, out int i) || i < 0)
            .Select(_ => $"Option '{option.RawAliases[0]}' is not a positive integer.")
            .FirstOrDefault());

    return option;
}

new Option<int>("-o").PositiveOnly()

[mristin]

Should I make a PR to add it to OptionExtensions.cs or you'd prefer to keep the extensions minimal?

[jonsequitur]

I think it's useful to keep the extensions minimal but one potential value of adding these is that over time we can localize error messages. This benefits end users and saves tool developers work, so it's worth considering.

@KathleenDollard Thoughts on how we should make this sort of decision?

[mristin]

Indeed, I think a generic "positivity" constraint would be useful in many use cases in the wild and indeed merits to be included either in the code or somewhere documented as a recipe.

[mristin]

Any update on this issue?

[mristin]

Hi @jonsequitur ,
I assume you are all very busy with the issues raining in and being so few people (two?) on the team. Hence I'll go ahead and make a pull request next week. Please block if you do not want to sponsor it.

[mristin]

@jonsequitur On a second thought, let me add first a section to the documentation on constraints called "Recipes" so that people can see how to add constraints in a principled way.

What do you think?

[jonsequitur]

I think this is a great idea.

[jonsequitur]

being so few people (two?) on the team

We don't have anyone working on this full-time. The community is the team. 😄

[KathleenDollard]

@mristin Thank you so much for the feedback on the docs. I absolutely love the idea of recipes.

As I explore app models, consistent validation is on my mind. For this, I think a range validation might be the most flexible. Would that work well enough for your scenario?

[mristin]

@KathleenDollard the problem with range validation is that it doesn't handle infinity well (theoretically you can pick Int32.MaxValue as infinity, but that is a work-around).

There are more pit-falls. For example, non-zero constraints can be confusing for the users in case of floats (in cases of +0.0 and -0.0). Probably the positivity constraint is the only one which would work in all cases. If we ignore +0.0 and -0.0, then non-negative as well. IMO, range would be too much (with edge cases such as inclusive, exclusive, inf etc.). It might suddenly lead to a numeric library included in System.CommandLine :-).

[henrygab]

This is really complex for the simple scenarios -- for example, this requires the user to re-parse the tokens and understand how to iterate though the internal data structures. Ouch.

Strawman goals / Scenarios (click to expand)

Goals

• Solution is invisible to clients which do not require the functionality
• Minimize code duplication
• Requires clients to writes minimal logic required for their particular use case
• Chainable / buildable ... user can add multiple constraints to an Option

Scenarios

• User wants to further restrict an already-supported type, validating individual values
• User wants to further restrict sets of values (for -arity > 1)

---

Strawman client code usage (click to expand)

OK, this uses really long names, solely to be more descriptive.
Framework-provided names would likely be much shorter.

new Option<int>("-o")
    .AddConstraintPerValue(IsPositiveValue)
    .AddConstraintPerSet(ValuesAllEvenOrAllOdd);
new Option<string>("-s")
    .AddConstraintPerValue(StartsWithVowel)
    .AddConstraintPerSet(AllStartWithSameLetter);

For per-value constraints, the functions are trivial:

public static bool IsPositiveValue(int value) { return value > 0; } 

public static bool StartsWithVowel(string value) {
    if (String.IsNullOrEmpty(value)) { return false; }
    char tmp = Char.ToLowerInvariant(value[0]);
    return (tmp == 'a' || tmp == 'e' || tmp == 'i' || tmp == 'o' || tmp == 'u' || tmp == 'y');
}

For the slightly more complex validation of sets of values, the functions are correspondingly only slightly complex:

public static bool ValuesAllEvenOrAllOdd(IOrderedEnumerable<int> values ) {
    bool any = false;
    bool areEven;
    foreach (var v in values) {
        if (!any) { areEven = ((v % 2) == 0); }
        if (areEven && ((v % 2) != 0)) { return false; }
        else if ((v % 2) == 0) { return false; }
    }
    // return any; // returns false for zero entries
    return true; // returns true for zero entries
}
public static bool AllStartWithSameLetter(IOrderedEnumerable<string> values) {
    bool any = false;
    char firstLetter;
    foreach (var v in values) {
        if (String.IsNullOrEmpty(v)) { return false; }
        if (!any) { firstLetter = v[0]; }
        if (v[0] != firstLetter) { return false; }
    }
    // return any; // returns false for zero entries
    return true; // returns true for zero entries
}

---

I think this would be significantly improved experience for clients. Please consider making it simpler for the client code by enabling something such as the above?