prioritize wide strings over strings on MSVC #876
Conversation
tests/AppTest.cpp
Outdated
| #if defined CLI11_HAS_FILESYSTEM && CLI11_HAS_FILESYSTEM > 0 | ||
| TEST_CASE_METHOD(TApp, "filesystemWideName", "[app]") { | ||
| std::filesystem::path myfile{L"voil\u20ac.txt"}; | ||
|
|
||
| std::filesystem::path fpath; | ||
| app.add_option("--file", fpath)->check(CLI::ExistingFile, "existing file"); | ||
|
|
||
| CHECK_THROWS_AS(app.parse(L"--file voil\u20ac.txt"), CLI::ValidationError); | ||
|
|
||
| bool ok = static_cast<bool>(std::ofstream(myfile).put('a')); // create file | ||
| CHECK(ok); | ||
|
|
||
| // deactivate the check, so it should run now | ||
|
|
||
| CHECK_NOTHROW(app.parse(L"--file voil\u20ac.txt")); | ||
|
|
||
| CHECK(fpath == myfile); | ||
|
|
||
| CHECK(std::filesystem::exists(fpath)); | ||
| std::filesystem::remove(myfile); | ||
| CHECK(!std::filesystem::exists(fpath)); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
On Ubuntu 20.04, if you do run this test it fails
That means necessarily passing -DCMAKE_CXX_STANDARD:STRING=20 (or 17)
-------------------------------------------------------------------------------
filesystemWideName
-------------------------------------------------------------------------------
CLI11/tests/AppTest.cpp:1719
...............................................................................
CLI11/tests/AppTest.cpp:1719: FAILED:
due to unexpected exception with message:
filesystem error: Cannot convert character sequence: Invalid or incomplete
multibyte or wide character
There was a problem hiding this comment.
A couple of the CI builds are failing with the same error, I am going to take a closer look today. Seems there is some ambiguity in the standards about how filesystem::path is treated with respect to strings and some differences between compilers.
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #876 +/- ##
=======================================
Coverage 99.46% 99.46%
=======================================
Files 18 18
Lines 4086 4086
=======================================
Hits 4064 4064
Misses 22 22
☔ View full report in Codecov by Sentry. |
|
I swapped the prioritize wstring to only be active on MSVC, not sure if that should be all windows or just visual studio? |
phlptp
changed the title
henryiii
force-pushed
the
prioritize_wstring
branch
from
072acf7
to
e1cf74d
Compare
github-actions
bot
added
needs changelog
Great that you limited it to not affect Unix, I think that's the right decision! Ultimately, it should be all of windows and not just MSVC. std::filesystem::path is wide on Windows as a whole (I have a doubt for cygwin though) |
to handle
std::filesystem::pathbetter widestring operations should be preferred over regular strings.Fix Issue #875