-
Notifications
You must be signed in to change notification settings - Fork 4
Correct cd history when going to package not on current PWD #28
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
base: master
Are you sure you want to change the base?
Conversation
|
@cottsay, Friendly ping to prevent this PR from being open for many years. |
| echo "Could not find package '$1' from the current working" \ | ||
| "directory" 1>&2 | ||
| fi | ||
| OLDPWD="$_colcon_cd_old_pwd" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that this change is here so that if no package is found and thus no change in directory is finalized this restores the shell context back as if when running cd normally you tried to cd into a non-existent directory. It might be worth going through and annotating for future contributors when OLDPWD and the internal _colcon_cd_old_pwd variables are set, updated and unset to make the maintenance easier on future contributors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some comments. Are they clear enough?
Signed-off-by: SuperJappie08 <36795178+SuperJappie08@users.noreply.github.com>
|
@cottsay, The current test failure is in an unrelated part of the code. |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
I fixed it in |
Corrects the
cdjump back behavior to work as expected #27.