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

replaced \t with tab character #27

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
3 participants
@muratsevim
Copy link

commented Aug 2, 2013

The sed installed on OSX Lion treats \t as a regular 't' char which raises issues if there is a 't' char in home path.

replaced \t with tab character
The sed installed on OSX Lion treats \t as a regular 't' char which raises issues if there is a 't' char in home path.
@dirk-thomas

This comment has been minimized.

Copy link
Member

commented Aug 2, 2013

@wjwwood Can you confirm the issue before the patch and that it works with the patch applied?

@wjwwood

This comment has been minimized.

Copy link
Member

commented Aug 2, 2013

Yes, I'll test that now.

@wjwwood

This comment has been minimized.

Copy link
Member

commented Aug 2, 2013

@muratsevim what do you mean by home path? A package of that name in the ROS_PACKAGE_PATH?

@wjwwood

This comment has been minimized.

Copy link
Member

commented Aug 2, 2013

Nvm, I see from the patch, if the home directory has a t in it, does it need to be at the front or anywhere in it?

@wjwwood

This comment has been minimized.

Copy link
Member

commented Aug 2, 2013

I can reproduce the problem and the fix:

$ export HOME=/tmp/rosbash_ws/thome

$ homedir=`echo $HOME | sed -e "s/\//\t\//g" -e "s/\t/\\\\\/g"`

$ echo $homedir
\/\mp\/rosbash_ws\/\home

$ export HOME=/tmp/rosbash_ws/thome

$ TAB=$'\t'

$ homedir=`echo $HOME | sed -e "s/\//${TAB}\//g" -e "s/${TAB}/\\\\\/g"`

$ echo $homedir
\/tmp\/rosbash_ws\/thome
@dirk-thomas

This comment has been minimized.

Copy link
Member

commented Aug 2, 2013

The goal here is to replace ~ with $HOME. Using / as the character to separate the reg exp parts is probably the worst decision in the rosbash code. By using | as the delimiter no escaping of $HOME would be necessary in the first place.

@dirk-thomas

This comment has been minimized.

Copy link
Member

commented Aug 2, 2013

I have created a modified pull request #28. @muratsevim Can you please verify that this also fixes your issue?

@muratsevim

This comment has been minimized.

Copy link
Author

commented Aug 2, 2013

@dirk-thomas I have tested #28 and can confirm it fixes my issue.

For your information, I don't actually use ~ in any of my ros paths but this was causing errors for me because the last t char on my home path was escaping the next / on the sed expression.

@dirk-thomas

This comment has been minimized.

Copy link
Member

commented Aug 2, 2013

Thanks for checking and providing the patch.

@dirk-thomas dirk-thomas closed this Aug 2, 2013

dirk-thomas added a commit that referenced this pull request Aug 2, 2013

Merge pull request #28 from ros/rosbash_homedir
simplify home expansion in completion, fix issue on osx (fix #27)

@muratsevim muratsevim deleted the muratsevim:patch-1 branch Aug 3, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.