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

smartcd fixes #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

smartcd fixes #5

wants to merge 2 commits into from

Conversation

garethstockwell
Copy link

  • Temporarily disable noclobber for bash
  • Fix script mapping by always using physical paths

Gareth Stockwell added 2 commits May 3, 2012 13:50
bash doesn't have zsh's localoptions, so we store the old value of
the clobber option in a global variable, and then explicitly
restore it once the file write is complete.
cd /tmp
mkdir foo
ln -s foo foo_link
cd /tmp/foo
smartcd edit enter
...
cd /tmp/foo_link
smartcd edit enter
...

Using the above sequence, the two "smartcd edit" commands should edit the same
script.  However, because smartcd uses the output of "pwd" to generate the path
to the script directory, two different scripts are created.

This patch fixes the bug by always using physical paths ("pwd -P").

It also fixes some unit test failures which occur if the directory from which
the tests are run was entered via a symbolic link:

cd /tmp
git clone https://github.com/cxreg/smartcd.git smartcd
ln -s smartcd smartcd_link
cd /tmp/smartcd && ./t/harness.sh # passes
cd /tmp/smartcd_link && ./t/harness.sh # smartcd.t steps 7, 10, 11 fail
@cxreg
Copy link
Owner

cxreg commented May 18, 2012

I like the noclobber fix. I think I will generalize the bash option-setting code to handle other options, just in case.

The "cd -P vs pwd -P" situation is one I'm not sure about. I may want to make that configurable, since some users may like the current behavior.

Thanks for your patches!

@garethstockwell
Copy link
Author

You're welcome. I have some more patches which I will submit once I've tested them a bit more.

smartcd is a great tool which I was happy to discover recently. I had previously written my own very basic cd hook which allowed me to set environment variables (but without autostashing nor recursion) and execute shell commands. But since smartcd is so much more powerful and generic, I switched to it immediately.

Nice work!

Gareth

Sent from my Nokia N9

On 18/05/2012 18:25 Dave Olszewski wrote:

I like the noclobber fix. I think I will generalize the bash option-setting code to handle other options, just in case.

The "cd -P vs pwd -P" situation is one I'm not sure about. I may want to make that configurable, since some users may like the current behavior.

Thanks for your patches!


Reply to this email directly or view it on GitHub:
#5 (comment)


Subject to local law, communications with Accenture and its affiliates including telephone calls and emails (including content), may be monitored by our systems for the purposes of security and the assessment of internal compliance with Accenture policy.


www.accenture.com

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.

2 participants