[MISC] Add poison_config

[eseiler]

See #220

To-do

• Decide whether to merge
• Decide on static assert message, Forgot sharg::config? is a placeholder
• Appease gcc11
• Appease gcc12

Summary
Improves error message when using add_option/add_flag/add_positional_option without an explicit sharg::config, e.g.

sharg::parser parser{/* ... */};
parser.add_option(value, /* sharg::config */{.short_id = 'i', .long_id = "int", .description = "Int"});

Pros:

• Helps the user :)

Cons:

• More complex code
• Requires maintenance
• If you get the call signature wrong (e.g., adding a third parameter), the poison overload will be shown as a candidate.

Most of the added code is to just check that both configs are in sync.

Since the 3 parser functions are the main API, it might be worth it to add some extra code that mitigates possible compiler errors. Though it should be rare to encounter this specific error if copy/pasting from the tutorial/snippets.

All in all, we need to weigh pros and cons.

Error message before

[ 75%] Building CXX object CMakeFiles/readme_sneak_peek_snippet.dir/readme_sneak_peek.cpp.o
/develop/sharg-parser/test/snippet/readme_sneak_peek.cpp: In function ‘int main(int, char**)’:
/develop/sharg-parser/test/snippet/readme_sneak_peek.cpp:9:22: error: no matching function for call to ‘sharg::parser::add_option(int&, <brace-enclosed initializer list>)’
    9 |     parser.add_option(val,
      |     ~~~~~~~~~~~~~~~~~^~~~~
   10 |                       {.short_id = 'i',
      |                       ~~~~~~~~~~~~~~~~~
   11 |                        .long_id = "int",
      |                        ~~~~~~~~~~~~~~~~~
   12 |                        .description = "Desc.",
      |                        ~~~~~~~~~~~~~~~~~~~~~~~
   13 |                        .validator = sharg::arithmetic_range_validator{0, 10}});
      |                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /develop/sharg-parser/include/sharg/all.hpp:58,
                 from ../../../test/snippet/readme_sneak_peek.cpp:1:
/develop/sharg-parser/include/sharg/parser.hpp:247:10: note: candidate: ‘template<class option_type, class validator_type>  requires ((parsable<option_type>) || (parsable<typename std::__detail::__iter_traits_impl<typename std::remove_cvref<decltype(std::ranges::__cust_access::__begin((declval<_Container&>)()))>::type, std::indirectly_readable_traits<typename std::remove_cvref<decltype(std::ranges::__cust_access::__begin((declval<_Container&>)()))>::type> >::__iter_traits<typename std::remove_cvref<decltype(std::ranges::__cust_access::__begin((declval<_Container&>)()))>::type, std::indirectly_readable_traits<typename std::remove_cvref<decltype(std::ranges::__cust_access::__begin((declval<_Container&>)()))>::type> >::value_type>)) && (invocable<validator_type, option_type>) void sharg::parser::add_option(option_type&, const sharg::config<validator_t>&)’
  247 |     void add_option(option_type & value, config<validator_type> const & config)
      |          ^~~~~~~~~~
/develop/sharg-parser/include/sharg/parser.hpp:247:10: note:   template argument deduction/substitution failed:
/develop/sharg-parser/test/snippet/readme_sneak_peek.cpp:9:22: note:   couldn’t deduce template parameter ‘validator_type’
    9 |     parser.add_option(val,
      |     ~~~~~~~~~~~~~~~~~^~~~~
   10 |                       {.short_id = 'i',
      |                       ~~~~~~~~~~~~~~~~~
   11 |                        .long_id = "int",
      |                        ~~~~~~~~~~~~~~~~~
   12 |                        .description = "Desc.",
      |                        ~~~~~~~~~~~~~~~~~~~~~~~
   13 |                        .validator = sharg::arithmetic_range_validator{0, 10}});

Error message after

[ 75%] Building CXX object CMakeFiles/readme_sneak_peek_snippet.dir/readme_sneak_peek.cpp.o
In file included from /develop/sharg-parser/include/sharg/all.hpp:58,
                 from ../../../test/snippet/readme_sneak_peek.cpp:1:
/develop/sharg-parser/include/sharg/parser.hpp: In instantiation of ‘void sharg::parser::add_option(option_type&, const sharg::detail::poison_config&) [with option_type = int]’:
/develop/sharg-parser/test/snippet/readme_sneak_peek.cpp:9:22:   required from here
/develop/sharg-parser/include/sharg/parser.hpp:266:31: error: static assertion failed: Forgot sharg::config?
  266 |         static_assert(detail::dependent_false_v<option_type>, "Forgot sharg::config?");
      |                       ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/develop/sharg-parser/include/sharg/parser.hpp:266:31: note: ‘sharg::detail::dependent_false_v<int>’ evaluates to false

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
sharg-parser	✅ Ready (Inspect)	Visit Preview	Feb 7, 2024 3:08pm

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (fc521fe) 91.90% compared to head (5547cdf) 91.90%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #225   +/-   ##
=======================================
  Coverage   91.90%   91.90%           
=======================================
  Files          17       17           
  Lines        1618     1618           
=======================================
  Hits         1487     1487           
  Misses        131      131           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[eseiler]

Alternative approach to try:

sharg_config is templated because of the validator. Why don't we use a (purely virtual) validator base class, all validators must inherit from base, and then store a pointer to the base class in sharg_config removing the template?

[eseiler]

Virtual could work https://github.com/eseiler/sharg-parser/tree/misc/virtual_validator

But it's not so nice, we need to use std::any and a validator needs to any_cast it to its option value type.

[h-2]

Thanks for putting so much effort in improving the diagnostics. But in the end, this is a C++ problem and not something you need to feel responsible about.

That having been said, I do think there is merit in type-erasing the validator and making the entire config a non-template, because it makes using and debugging it a lot simpler!
The best way I see doing this is having the validator always get the command line argument as a string.¹ This makes implementing validators a tiny bit more work, but it avoids all the trouble with std::any and it is also intuitive, because everybody knows that the arguments are strings initially.
This would also allow chaining adaptors more easily, e.g. the previously suggested (by me 😇 )" input-file-or---validator would just need to look at the string, see if it is "-" and otherwise call the input_file_validator.
It would even allow creating validators that are currently not possible, e.g. an integer-validator that also accepts the string "NULL".

¹ If needed, one could mandate an additional operator() for ranges of strings, but that should cover everything?

edit: I forgot to say: if the parameter-type is fixed, there are multiple ways to implement this easily, e.g. virtual functions, std::function or similar methods.

[eseiler]

Thanks for putting so much effort in improving the diagnostics. But in the end, this is a C++ problem and not something you need to feel responsible about.

Indeed, the only reason I see to do that is that it's the API and a very unhelpful error message

The best way I see doing this is having the validator always get the command line argument as a string.¹

Yes, that would make it rather easy.
I don't know yet how much work that would be:

• Currently, parsing and validation happens separately. No idea yet how easy it will be to change that.
• It would be preferable (maybe) if the validator parses and returns the value on successful validation. Though, I don't have a problem with just parsing the string twice. It probably won't even work, because then the return value is a template again :)
• It might be that the default_validator is always used if there is no other one. Then we maybe have to do the parsing twice and kinda need the parsing code in two locations. But just skipping validation if it's the default validator is easier (if it doesn't already work like that).

¹ If needed, one could mandate an additional operator() for ranges of strings, but that should cover everything?

We can just validate each element of a range instead of the whole range at once. But this also depends on the parsing itself is solved.

edit: I forgot to say: if the parameter-type is fixed, there are multiple ways to implement this easily, e.g. virtual functions, std::function or similar methods.

Yep, the option_value_type is the problem. If this type is fixed, it's trivial to get rid of the validator_type.