incorrect validator order and default values not used

[wkjarosz]

I'm trying to create an option is bound to the following variable:

tuple<string,string,int,string> remap_args{"angularmap","angularmap",1,"bilinear"};

I have two problems.

1. If I use the following code (env_format_names and lookup_mode_names are sets with the allowed strings),

app.add_option("--remap", remap_args, "help string")
            ->check(CLI::Validator(CLI::IsMember(env_format_names)).application_index(0))
            ->check(CLI::Validator(CLI::IsMember(env_format_names)).application_index(1))
            ->check(CLI::Validator(CLI::PositiveNumber).application_index(2))
            ->check(CLI::Validator(CLI::IsMember(lookup_mode_names)).application_index(3))
            ->option_text("M M S L")
            ->group("Editing")
            ;

Then the arguments listed in CLI11's error string are out of order. It gives the following error (note that POSITIVE is listed in index 1 instead of 2:

--remap: 4 required [TEXT,TEXT,INT,TEXT]:{angularmap,cubemap,cylindrical,latlong,mirrorball}:POSITIVE:{angularmap,cubemap,cylindrical,latlong,mirrorball}:{bicubic,bilinear,nearest} missing

I can get it to correctly order these through trial and error by changing the order of the chained validators to:

app.add_option("--remap", remap_args, "help string")
            ->check(CLI::Validator(CLI::IsMember(env_format_names)).application_index(0))
            ->check(CLI::Validator(CLI::PositiveNumber).application_index(2))
            ->check(CLI::Validator(CLI::IsMember(env_format_names)).application_index(1))
            ->check(CLI::Validator(CLI::IsMember(lookup_mode_names)).application_index(3))
            ->option_text("M M S L")
            ->group("Editing")
            ;

This then produces the argument descriptions in the right order upon error:

--remap: 4 required [TEXT,TEXT,INT,TEXT]:{angularmap,cubemap,cylindrical,latlong,mirrorball}:{angularmap,cubemap,cylindrical,latlong,mirrorball}:POSITIVE:{bicubic,bilinear,nearest} missing

Is this a bug? Am I doing something wrong?

2. Also, I'd like to treat the 3rd and 4th arguments as optional (taking default values if not specified). By adding ->type_size(2,4) i can make CLI11 accept 2,3 or 4 arguments to this option:

app.add_option("--remap", remap_args, "help string")
            ->type_size(2,4)
            ->check(CLI::Validator(CLI::IsMember(env_format_names)).application_index(0))
            ->check(CLI::Validator(CLI::PositiveNumber).application_index(2))
            ->check(CLI::Validator(CLI::IsMember(env_format_names)).application_index(1))
            ->check(CLI::Validator(CLI::IsMember(lookup_mode_names)).application_index(3))
            ->option_text("M M S L")
            ->group("Editing")
            ;

Unfortunately, when if I pass in less than 4 arguments, the last two arguments do not retain the default values specified in the bound variable.

Is there a correct way to achieve this?

Thanks

[phlptp]

Wow, that is a complex scenario. I don't even know if I would consider it a bug, more a scenario beyond those envisioned.

The text generated in the warnings is in the order of check definitions, I don't think it was anticipated that the order would be otherwise, though that is something that maybe should be resolved to take into account the application_index if appropriate. But that is not currently done.

As far as the partial defaults I am going to have to dig into what the code is doing in that scenario, I think it treats a tuple as a single object and would initialize the optional parts with whatever an empty string converts to, which is the correct thing to do in all previously encountered cases. I will look into whether partial default is a practical possibility.

[wkjarosz]

Thanks for the quick response!

Can you clarify your response about the warning and order of checks? If I understand you correctly, I don't think the warning is written in the order of the checks (at least not in the order the checks are chained in the code).

When I chain the checks in index order, the warning message is incorrect:

->check(CLI::Validator(CLI::IsMember(env_format_names)).application_index(0))
            ->check(CLI::Validator(CLI::IsMember(env_format_names)).application_index(1))
            ->check(CLI::Validator(CLI::PositiveNumber).application_index(2))
            ->check(CLI::Validator(CLI::IsMember(lookup_mode_names)).application_index(3))

I had to manually reorder the checks by trial and error to get the correct order in the warning message:

->check(CLI::Validator(CLI::IsMember(env_format_names)).application_index(0))
            ->check(CLI::Validator(CLI::PositiveNumber).application_index(2))
            ->check(CLI::Validator(CLI::IsMember(env_format_names)).application_index(1))
            ->check(CLI::Validator(CLI::IsMember(lookup_mode_names)).application_index(3))

[phlptp]

The generation of the type name when using validators is

std::string get_type_name() const {
        std::string full_type_name = type_name_();
        if(!validators_.empty()) {
            for(auto &Validator : validators_) {
                std::string vtype = Validator.get_description();
                if(!vtype.empty()) {
                    full_type_name += ":" + vtype;
                }
            }
        }
        return full_type_name;
    }

when you run help is that different than the number error output?

So the validators go in order addition when using ->check(...)

If they don't I am not sure what is going on.

[jzakrzewski]

Well, it seems the author knew that the order may be different...:

/// Get a Validator by index NOTE: this may not be the order of definition
Validator *get_validator(int index) {

(no idea, why should it, though)

[phlptp]

check(), transform(), and each(), add the Validators in different spots. transform adds at the beginning of the Validator vector so those get handled first. Validators added through check(), and each(), are added at the end so those are handled after the transforms. So that is why the comment is there. I am traveling right now so not by my development system so I can't do much debugging yet.

[phlptp]

I am not able to replicate the issue

#include <CLI/CLI.hpp>
#include <iostream>
#include <string>
#include <tuple>

int main(int argc, const char *argv[]) {
    using namespace std;
    CLI::App app{"App"};

    tuple<string,string,int,string> remap_args{"angularmap","angularmap",1,"bilinear"};

    set<std::string> env_format_names {"angularmap","cubemap","cylindrical","latlong","mirrorball"};

    set<std::string> lookup_mode_names {"bicubic","bilinear","nearest"};
    app.add_option("--remap", remap_args, "help string")
        ->check(CLI::Validator(CLI::IsMember(env_format_names)).application_index(0))
        ->check(CLI::Validator(CLI::IsMember(env_format_names)).application_index(1))
        ->check(CLI::Validator(CLI::PositiveNumber).application_index(2))
        ->check(CLI::Validator(CLI::IsMember(lookup_mode_names)).application_index(3))
        ->option_text("M M S L")
        ->group("Editing")
        ;

    CLI11_PARSE(app, argc, argv);
    
}

with input

--remap 1 2 3 

produces

--remap: 4 required [TEXT,TEXT,INT,TEXT]:{angularmap,cubemap,cylindrical,latlong,mirrorball}:{angularmap,cubemap,cylindrical,latlong,mirrorball}:POSITIVE:{bicubic,bilinear,nearest} missing
Run with --help for more information.

which is as expected and different from what you are showing

[phlptp]

I suspect the only way to handle the partial default at present would be a custom callback for that option to do the conversion directly with a string vector of the arguments.