Skip to content

Leaver user’s PATH untouched.#17

Open
eigengrau wants to merge 1 commit intopindexis:masterfrom
eigengrau:install-location
Open

Leaver user’s PATH untouched.#17
eigengrau wants to merge 1 commit intopindexis:masterfrom
eigengrau:install-location

Conversation

@eigengrau
Copy link
Copy Markdown

Instead of adding the location of qfc to the user’s PATH, this
leaves PATH untouched and calls qfc at an arbitrary install
location instead.

In addition to not adding noise to the user’s path for just one
script, this also allows having qfc installed to a location other
than ~/.qfc.

@eigengrau
Copy link
Copy Markdown
Author

Seems lots of folks wanted the script path to not be hard-wired to ~/.qfc (cf. #16 & #12). This patch addresses these as well. However, it looks like you don’t actually need to use PATH at all.

@pindexis
Copy link
Copy Markdown
Owner

sorry for taking long time to respond, been too busy ):

I like the idea of not touching the PATH environment variable. and I don't think of any reason why someone would call the qfc python command-line directly without passing by the SHELL script.

There are few issues with the code you added though. $0 won't work in BASH when the script is sourced(which is the case here). take a look here:
http://stackoverflow.com/questions/59895/can-a-bash-script-tell-what-directory-its-stored-in

Instead of adding the location of qfc to the user’s PATH, this
leaves PATH untouched and calls qfc at an arbitrary install
location instead.

In addition to not adding noise to the user’s path for just one
script, this also allows having qfc installed to a location other
than ~/.qfc.
@eigengrau
Copy link
Copy Markdown
Author

Ah, interesting pitfall. I amended the PR using BASH_SOURCE when on Bash. Let me know if anything else might cause problems.

Thanks for making qfc!

@desyncr
Copy link
Copy Markdown

desyncr commented Oct 17, 2016

Ping @pindexis

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.

3 participants