Skip to content

fix custom zsh logic for handling arrays#80

Merged
dirk-thomas merged 1 commit intomasterfrom
fix_custom_zsh_logic
Jul 19, 2018
Merged

fix custom zsh logic for handling arrays#80
dirk-thomas merged 1 commit intomasterfrom
fix_custom_zsh_logic

Conversation

@dirk-thomas
Copy link
Copy Markdown
Contributor

Fixes ros2/ros2#546.

Otherwise when sourcing a package-level script it doesn't perform the ament_zsh_to_array function and therefore doesn't source package environment hooks.

@dirk-thomas dirk-thomas added bug Something isn't working in review Waiting for review (Kanban column) labels Jul 19, 2018
@dirk-thomas dirk-thomas self-assigned this Jul 19, 2018
@dirk-thomas
Copy link
Copy Markdown
Contributor Author

In addition colcon/colcon-ros#29 is necessary to chain to the zsh specific package script.

@mikaelarguedas
Copy link
Copy Markdown
Contributor

while currently not necessary for it to work shouldn't the other package_level templates also set AMENT_SHELL for consistency?

Copy link
Copy Markdown
Contributor

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, assuming that this has been tested and confirmed to fix the issue

@dirk-thomas
Copy link
Copy Markdown
Contributor Author

while currently not necessary for it to work shouldn't the other package_level templates also set AMENT_SHELL for consistency?

They could for consistency but there is no logic which check for e.g. bash so it isn't necessary atm.

@mikaelarguedas
Copy link
Copy Markdown
Contributor

I tested this on zsh on Linux and it fixes the issue for me 👍

@dirk-thomas dirk-thomas merged commit fc2bfb4 into master Jul 19, 2018
@dirk-thomas dirk-thomas deleted the fix_custom_zsh_logic branch July 19, 2018 21:24
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Jul 19, 2018
@mikaelarguedas mikaelarguedas mentioned this pull request Jul 20, 2018
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants