Default value conversion error if the application locale is set

[agalanin-at-nvidia]

Short description
Application fails with CLI::ConversionError in some locales if the application locale is set.

Root cause
CLI11 uses std::stringstream to format default value and strtol/strtod to parse them back. The first method adds locale-specific thousand separators but the second method parses the number only up to the first separator. This causes an exception when the same value is converted to string representation and then parsed back.

Examples: in en_US locale 1234.56 is "1,234.56" and in de_DE locale 1234.56 is "1.234,56".

Reproduction

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

int main(int argc, const char **argv)
{
    std::locale::global(std::locale(""));
    CLI::App app{"Bug report app"};
    long foo;

    app.add_option("FOO", foo, "Foo option")
        ->envname("FOO")
        ->default_val(1234567);
    CLI11_PARSE(app, argc, argv);

    return 0;
}

Output:

$ g++ -Wall -Wextra -I CLI11/include/ cli11-bug-locale-string.cpp -o cli11-bug-locale-string
$ for locale in C en_US de_DE fr_FR de_CH; do echo "locale $locale"; LC_NUMERIC=$locale.UTF-8 ./cli11-bug-locale-string && echo OK; echo; done
locale C
OK

locale en_US
terminate called after throwing an instance of 'CLI::ConversionError'
what():  The value FOO is not an allowed value for given default value("1,234,567") produces an error : Could not convert: FOO = 1,234,567
Aborted (core dumped)

locale de_DE
terminate called after throwing an instance of 'CLI::ConversionError'
what():  The value FOO is not an allowed value for given default value("1.234.567") produces an error : Could not convert: FOO = 1.234.567
Aborted (core dumped)

locale fr_FR
terminate called after throwing an instance of 'CLI::ConversionError'
what():  The value FOO is not an allowed value for given default value("1 234 567") produces an error : Could not convert: FOO = 1 234 567
Aborted (core dumped)

locale de_CH
OK

Fix description
The fix strips group separators from the input string. This extends a bit the logic introduced in change #968 for _ and ' separators.
There are no unit tests for the change because it requires to have all mentioned locales to be installed in the system.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (e4ee3af) to head (11c66c9).
Report is 92 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##              main     #1160     +/-   ##
===========================================
  Coverage   100.00%   100.00%             
===========================================
  Files           17        17             
  Lines         4546      5066    +520     
  Branches         0      1027   +1027     
===========================================
+ Hits          4546      5066    +520     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[agalanin-at-nvidia]

The other fix option is to force all formatting/parsing operations to be in "C" locale but it may impact the current users of the library who may already use locale-specific parsing.

[phlptp]

I like the solution, I am starting to play with ways to get some tests in place. If I get something working I will try to merge with your branch.

[phlptp]

I made a PR into the branch for this PR, if that is merged then it should clear up some of the missing coverage. There will likely need to be a second test with an unsigned int, and floating point to get the rest.

[agalanin-at-nvidia]

Thanks, @phlptp, I added your test case into the PR.

BTW, I found that the issue is more complex than I thought. I've hacked main function in catch2 to set locale to "de_DE.UTF-8" and found that it breaks not only tests when the input is hard-coded as a string (it's expected that these tests will fail) but also tests for tuples:

-------------------------------------------------------------------------------
TupleDefault
-------------------------------------------------------------------------------
../tests/AppTest.cpp:494
...............................................................................

../tests/AppTest.cpp:494: FAILED:
  {Unknown expression after the reported line}
due to unexpected exception with message:
  The value -i is not an allowed value for given default value("[99,5,test2,87,
  total3]") produces an error : Could not convert: -i = 99,5,test2,87,total3

-------------------------------------------------------------------------------
TupleComplex
-------------------------------------------------------------------------------
../tests/AppTest.cpp:520
...............................................................................

../tests/AppTest.cpp:520: FAILED:
  {Unknown expression after the reported line}
due to unexpected exception with message:
  The value -i is not an allowed value for given default value("[99,5,test2,87,
  [total3,total4]]") produces an error : Could not convert: -i = 99,5,test2,87,
  total3,total4