-
Notifications
You must be signed in to change notification settings - Fork 361
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert #54
Conversation
Codecov Report
@@ Coverage Diff @@
## master #54 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 9 9
Lines 1298 1301 +3
Branches 257 257
=====================================
+ Hits 1298 1301 +3
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #54 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 9 9
Lines 1298 1304 +6
Branches 257 258 +1
=====================================
+ Hits 1298 1304 +6
Continue to review full report at Codecov.
|
@Warchant, what do you think? |
@henryiii Thanks, that looks good. However, one thing that looks strange to me is that all these methods ( Method CLI::App main;
int n;
main.add_option("--number", n, "description")
->check([](int n){
return n > 0 and n < 10; // just example
})
->transform([](int n){
return n + 3; // just example
}); Look at this function, it always receives non-parsed UPD: one way to do it is class OptionBase{...}
template <typename T>
class Option: public OptionBase {
Option* check(std::function<bool(T)> f) {...}
Option* transform(std::function<T(T)> f) {...}
// function that is executed to parse arg from std::string to T, it is actually lexical_cast
// if we have specialization for T then we may not call `parse` manually, otherwise (for complex types),
// we call
Option* parse(std::function<T(std::string)> f) {...}
}
class App {
...
std::vector<OptionBase_p> options;
...
} I did not see all the code, maybe it is not possible to do due to other design solutions. Is it? |
No, it's not possible in the current design due to the fact that Since all input comes from the command line, it always starts as a string, so converting and validating on the original string is generally safe. And, if I originally tried to make CLI11 templated very much like you outlined, and it failed, but don't remember exactly why. Maybe due to the fact you can't change the signature of an overloaded function, and CRTP doesn't work if you want to store and use the pointers as a single base class. I may revisit and add docs for adding specializations, though. |
PS: The current design also means a validator can throw a different/better error if it can't be converted, and a transform function can make a non-valid input into a valid one, like stripping the letters from complex numbers or vectors. |
I may revisit templated options. It does seem like it could work, so I at least need to document why it doesn't work if it really doesn't... |
This is a requested feature: The ability for a transform function. Here is the design suggested:
This is internally implemented by giving all validation functions a reference instead of a copy (slight API change), and making
transform
simply the exact same thing ascheck
, but with theconst
removed and the function rewritten as astring(string)
function. CLI11 has always supported a series of validators, so this is not a major change. The validators now run first before the value is filled.transform
API only tostring(string)
. Thoughts?Validation functions now return a string representing an error message. If it is empty, the validation succeeds. As always, they can also throw a
ValidationError
.