Default value and transformations

[jzakrzewski]

I do this:

cmd->add_option("--existing", "What to do if deployment already exists in the destination")
      ->transform(CLI::CheckedTransformer(std::unordered_map<std::string, ExistingBehaviour>{
          {"abort"s, ExistingBehaviour::abort},
          {"overwrite"s, ExistingBehaviour ::overwrite},
          {"delete"s, ExistingBehaviour::remove},
          {"remove"s, ExistingBehaviour::remove}}))
      ->default_val("abort");

and all is well, the transformer runs but later (in a subcommand callback) I do:

cmd->get_option("--existing")->as<ExistingBehaviour>()

And it throws.
It turns out that, even though the transformer runs, the original default ("abort") is stored instead of (apparently expected) "0". If I actually provide "--existing=abort" on the command line, it works. I could just put "0" as default but then a) help is broken, b) if I change the enum the hell breaks loose.

It looks to me like this function is responsible:

CLI11/include/CLI/Option.hpp

Lines 973 to 980 in 8ecce8f

	Option *default_val(std::string val) {
	default_str(val);
	auto old_results = results_;
	results_ = {val};
	run_callback();
	results_ = std::move(old_results);
	return this;
	}

[phlptp]

I think this is the same issue as #305. The root cause is that the default values are not passed through the validators/transforms, and related to that the as function gets the default through a different mechanism. I think PR #325 will be able to resolve this and several other issues once it is able to be merged, but we should add that use case as one of the tests to make sure.

[jzakrzewski]

It didn't look the same for me but you understand the code much better, so I believe you ;)
I just observed that it's going through validation and transformation but the transformation result is discarded.
I'll try #325 and report back in the nearest future.

[phlptp]

I am going to add the code you posted(unless you object) to a test case and make sure it passes.

[jzakrzewski]

Yeah, go ahead.
cmd is a pointer to CLI::App and ExistingBehaviour is an enum class if that's important.

BTW: I was positively surprised to see direct support for enum class.

[phlptp]

The test case is now passing in PR #325. Had most of the pieces ready but needed to incorporate everything into the as function yet. There is a few things to take care of on that PR yet before it is ready for merging with the main line, but it does pass all the tests.

[jzakrzewski]

Nice!
Thanks for the quick reaction.

[jzakrzewski]

Just to confirm - #325 works in a real codebase ;)