Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for spaces in path #519

Closed
wants to merge 1 commit into from
Closed

Support for spaces in path #519

wants to merge 1 commit into from

Conversation

raphendyr
Copy link

There is very commonly spaces in ros workspace path...

EDIT: Fixed setup.bash and setup.zsh too.

@dirk-thomas
Copy link
Member

The usage of dirname only is not sufficient. I will commit a working patch soon.

@raphendyr
Copy link
Author

Thanks for the better implementation.

ps. what's the reason to clean up path using pwd?

@dirk-thomas
Copy link
Member

Try you code in e.g. /tmp/test.sh and source it from / using . tmp/test.sh. dirname will return nothing which is just wrong. The modified version always returns an absolute path.

@raphendyr
Copy link
Author

Hmmm... ineresting... I got these results:

/ $ sh tmp/foo\ bar/test.sh 
/tmp/foo bar
tmp/foo bar
tmp/foo bar
/ $ dash tmp/foo\ bar/test.sh 
/tmp/foo bar
tmp/foo bar
tmp/foo bar
/ $ bash tmp/foo\ bar/test.sh 
/tmp/foo bar
tmp/foo bar
tmp/foo bar
/ $ zsh tmp/foo\ bar/test.sh 
/tmp/foo bar
tmp/foo bar
tmp/foo bar
/ $ cat tmp/foo\ bar/test.sh
#!/bin/sh

echo $(cd "`dirname "$0"`" && pwd)
echo $(dirname "$0")
echo ${0%/*}

ps. The last line could also be used?

@dirk-thomas
Copy link
Member

You must source the shell script and not invoke it in order to see the difference. Run source tmp/foo\ bar/test.sh.

@raphendyr
Copy link
Author

Well. Can't reproduce, but never mind. Not my problem, it works.

cwecht pushed a commit to cwecht/catkin that referenced this pull request Mar 20, 2018
This changes the Ros api that multiple textures will be returned,
which are forwarded from the Ros service call.
Adapts all usages to follow this new api.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants