Conversation
|
With ros/ros_comm#1324 being merged in the |
|
@dirk-thomas Do you think I should check for something like It'd be easier if you create |
The completion script could check the ROS distro and only conditionally add the new completion options. While not "great" that would be fine with me to get this working for newer distros.
As mentioned in my previous comment I don't think the overhead of branching (and having to maintain these branches in the future) is justified by the improved completion script. |
92a9d76 to
1753350
Compare
1753350 to
510e2ef
Compare
|
Rebased and updated checking for For testing in my local checkout of diff --git a/clients/rospy/package.xml b/clients/rospy/package.xml
index a56d141..ebc057c 100644
--- a/clients/rospy/package.xml
+++ b/clients/rospy/package.xml
@@ -1,6 +1,6 @@
<package>
<name>rospy</name>
- <version>1.13.6</version>
+ <version>1.13.7</version>
<description>
rospy is a pure Python client library for ROS. The rospy client
API enables Python programmers to quickly interface with ROS <aAnd the auto-completion works as expected. The reason I didn't check the ros distro ( |
| fi | ||
|
|
||
| VERSION_ECHO="1.13.7" | ||
| dpkg --compare-versions ${VERSION_ECHO} le ${VERSION} |
There was a problem hiding this comment.
Can we rely on this to be available across all targeted platforms (e.g. Fedora, macOS)?
There was a problem hiding this comment.
Good call.
Honestly, I google how to reliably compare versions. I picked this option because it was really simple, but it probably isn't available in other OSs.
I'm open to suggestions. What do you think of the two options here: https://stackoverflow.com/questions/16989598/bash-comparing-version-numbers/24067243 ?
I think sort -V should work in most systems nowadays, although some people say it's only available in the GNU sort version. If you think it's better to perform the check w/o sort -V, I can implement the other option.
There was a problem hiding this comment.
Actually, it seems the FreeBSD supports sort -V. Do you know if macOS supports it? I don't have a Mac at hand atm.
There was a problem hiding this comment.
The man pages are online:
https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man1/sort.1.html
I don't see a -V option.
There was a problem hiding this comment.
Seems like a custom bash function is the most portable way to do it (without using Python):
There was a problem hiding this comment.
Actually, that online man page might be out-of-date, on my macbook I see the option:
% man sort | grep '\-V'
-V, --version-sort
There was a problem hiding this comment.
@dirk-thomas @wjwwood I'm happy to create a new PR, but I'm not sure if you'd be ok with using sort -V or you prefer the approach from https://stackoverflow.com/a/4025065/671658 🤔
There was a problem hiding this comment.
I'm happy to create a new PR
Absolutely, please make sure to target the default branch (which is noetic-devel) first and the patch can be backorder afterwards.
but I'm not sure if you'd be ok with using sort -V or you prefer the approach from https://stackoverflow.com/a/4025065/671658
As long as it is proven to work on currently supported platforms anything is fine with me.
|
Closing due to inactivity. |
This PR update the auto-complete code for
rosconsoleadding theechocommand added in ros/ros_comm#1324