[FIX] Extension with dots not working in validator #2363
Conversation
eseiler
requested review from
a team and
SGSSGene
and removed request for
a team
Codecov Report
@@ Coverage Diff @@
## master #2363 +/- ##
=======================================
Coverage 98.21% 98.21%
=======================================
Files 265 265
Lines 10798 10806 +8
=======================================
+ Hits 10605 10613 +8
Misses 193 193
Continue to review full report at Codecov.
|
| bool ext_found{false}; | ||
|
|
||
| // Iterate over all possible extensions, starting with the longest. E.g., "foo.bar.baz", "bar.baz", "baz". | ||
| for (size_t pos = file_path.find("."); !ext_found && pos != std::string_view::npos; pos = file_path.find(".")) |
There was a problem hiding this comment.
What happens here? Why not just iterate over the valid extensions and check each time if file_path ends with that?
There was a problem hiding this comment.
Hah, yeah no idea. Probably the amalgamation of different ideas...
| * \param str The string to be searched. | ||
| * \param suffix The suffix to be searched for. | ||
| * \returns `true` if `suffix` is a suffix of `str`, otherwise `false`. | ||
| * \sa https://en.cppreference.com/w/cpp/string/basic_string_view/ends_with |
There was a problem hiding this comment.
This does not behave as described, because it ignores case 💅🏻
eseiler
force-pushed
the
fix/argparse_ext
branch
2 times, most recently
from
0006ecf
to
3c81880
Compare
SGSSGene
requested review from
a team and
marehr
and removed request for
a team
| // Drop the dot. | ||
| std::string drop_less_ext = path.extension().string().substr(1); | ||
| std::string const path_as_string{path.filename().string()}; | ||
| std::string_view file_path{path_as_string}; |
There was a problem hiding this comment.
Not sure about all of this string_views, do we really need them?
I think it would be fine to use strings here.
| std::ranges::equal(str.substr(str_length - suffix_length), suffix, [] (char const chr1, char const chr2) | ||
| { | ||
| return std::tolower(chr1) == std::tolower(chr2); | ||
| }); |
There was a problem hiding this comment.
Can we use the definition from https://en.cppreference.com/w/cpp/string/basic_string_view/ends_with ?
size() >= sv.size() && compare(size() - sv.size(), npos, sv) == 0One range less :)
Never mind we want case insensitivity.
| * \param suffix The suffix to be searched for. | ||
| * \returns `true` if `suffix` is a suffix of `str`, otherwise `false`. | ||
| */ | ||
| bool string_ends_with(std::string_view str, std::string_view suffix) const |
There was a problem hiding this comment.
| bool string_ends_with(std::string_view str, std::string_view suffix) const | |
| bool case_insensitive_string_ends_with(std::string_view str, std::string_view suffix) const |
or something xD
There was a problem hiding this comment.
We could also implement as new std::char_traits that make the string case-insensitive https://en.cppreference.com/w/cpp/string/char_traits
eseiler
force-pushed
the
fix/argparse_ext
branch
from
3c81880
to
7bd4c66
Compare
Resolves #2353