refactor: replace regex matching with find_last_of to split plugin name #271
Conversation
ipa-fez
force-pushed
the
refactor/no_regex
branch
from
c33b84a to
4e5da06
Compare
| auto last_slash = lookup_name.find_last_of('/'); | ||
| auto last_colon = lookup_name.find_last_of(':'); | ||
| if (last_slash == std::string::npos && last_colon == std::string::npos) { | ||
| // no matches | ||
| return lookup_name; | ||
| } | ||
| if (last_slash == std::string::npos) { | ||
| // only colon matched | ||
| return lookup_name.substr(last_colon + 1); | ||
| } | ||
| if (last_colon == std::string::npos) { | ||
| // only slash matched | ||
| return lookup_name.substr(last_slash + 1); | ||
| } | ||
| // both matched, return shorter suffix | ||
| return lookup_name.substr(std::max(last_slash, last_colon) + 1); |
There was a problem hiding this comment.
Thank you for the PR!
Please add unit tests for ClassLoader<T>::getName so we can make sure the code works the same before and after the refactor
https://github.com/ros/pluginlib/blob/rolling/test/utest.cpp
There was a problem hiding this comment.
Please also add #include <algorithm> for max to make the linter happy: https://build.ros2.org/job/Rpr__pluginlib__ubuntu_noble_amd64/17/testReport/junit/pluginlib/cpplint/build_include_what_you_use__4____tmp_ws_src_pluginlib_include_pluginlib_class_loader_imp_hpp_524_/
There was a problem hiding this comment.
I added some tests and the missing include. Let me know if I should add any additional test cases.
ipa-fez
force-pushed
the
refactor/no_regex
branch
2 times, most recently
from
3c0d559 to
1f8f369
Compare
Signed-off-by: Felix Zeltner <28302109+ipa-fez@users.noreply.github.com>
ipa-fez
force-pushed
the
refactor/no_regex
branch
from
1f8f369 to
a2457b0
Compare
|
@sloretz do you mind to review it again ? |
|
Pulls: #271
|
|
3 participants
This gets rid of the regex based string splitting and just finds the appropriate substring using a delimiter directly.
I came across this because clang-19 raises some
-Wmaybe-uninitializedwarnings inside the regex code here.