colcon / colcon-cd

A shell function for colcon to change the current working directory
http://colcon.readthedocs.io
Apache License 2.0
4 stars 4 forks source link

add colcon_cd function #1

Closed dirk-thomas closed 5 years ago

dirk-thomas commented 5 years ago

Requires colcon/colcon-core#240 and colcon/colcon-output#23.

dirk-thomas commented 5 years ago

A behavior of always preferring the workspace related to the current working directory seems more intuitive to me.

Atm there is no way to determine a workspace root. See colcon/colcon-core#139 which is requiring a similar functionality.

And then falling back to a previous workspace if there was one.

For another underlay the only information about it is in the COLCON/AMENT_PREFIX_PATH which points to the install prefix which doesn't provide a way to derive the source directories of the packages in a reliable (non-heuristic) way.

The advantage of not requiring a workspace at all is that you can use the command anywhere in your file system.

deitry commented 5 years ago

Why this is not implemented as colcon subverb, like colcon cd? I respect colcon because there is no commands with underscores as it was with catkin_make and so on. Adding one will lead to unnecessary diversity and confusion.

dirk-thomas commented 5 years ago

Why this is not implemented as colcon subverb, like colcon cd?

@deitry Because a (Python) subprocess can't change the working directory of a shell. Therefore this functionality must be implemented as a shell function.

Regarding the naming of the function: in sh it is invalid to have dashes in function names.

dirk-thomas commented 5 years ago

I think we could use a better error message about discovering multiple packages with the same name, and consider a fall-back behaviour to cd to the first instance.

Yeah, it should definitely handle duplicate package names - done in: 2244869fbb08b41a1bea7695388113ed1eb7bf56.

dirk-thomas commented 5 years ago

I also can't use colcon_cd with ZSH

Fixes in c9c66e84c1502f3caa90eaf598a8581723c5650b.

cottsay commented 5 years ago

Requires colcon/colcon-core#240 and colcon/colcon-output#23.

Should this be reflected in setup.cfg? Or is a more of a soft requirement?

dirk-thomas commented 5 years ago

It doesn't strictly require those. If you have an older version it will just ignore the env var and generate a log directory`.

I will see if I add a version dependency anyway before releasing this... Thanks for the pointer.