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

Edit .bash_profile if it exists #1090

Closed
wants to merge 2 commits into from
Closed

Conversation

awelkie
Copy link
Contributor

@awelkie awelkie commented May 3, 2017

Fixes #883

@Diggsey
Copy link
Contributor

Diggsey commented May 5, 2017

Hi @awelkie, this looks great.

I would like to slightly alter the detection logic here, specifically:

  • Modify .zprofile if either it already exists, or the shell is zsh
  • Modify .bash_profile only if it exists, regardless of the current shell

IMO, this would provide the best result given each shell's profile logic.

However, I'd be happy to accept this PR as-is if you won't be able to make these changes, since it's a clear improvement over the status quo, and shouldn't cause any problems to tweak later.

@brson
Copy link
Contributor

brson commented May 5, 2017

@Diggsey's suggestion sounds reasonable to me. Thanks @awelkie!

@awelkie
Copy link
Contributor Author

awelkie commented May 6, 2017

I'll take a crack at making these changes. I would also like to add support for fish (#478). Should the logic there be to add add a file to ~/.config/fish/conf.d only if ~/.config/fish/ exists, regardless of the current shell?

@Diggsey
Copy link
Contributor

Diggsey commented May 6, 2017

@awelkie Thanks!

For all shells except bash, we should set their configuration if we detect their presence at all on the system. For the moment, the two detection methods we use are $SHELL and whether their configuration files are detected.

Bash is different, because it will fall back to .profile if .bash_profile does not exist, so even if $SHELL == "bash", we can't create .bash_profile if it doesn't already exist, as then bash would stop looking in .profile, which would break things for the user.

So for fish, we want to add a file to ~/.config/fish/conf.d if either the fish config directory already exists, or if fish is the current shell.

@awelkie
Copy link
Contributor Author

awelkie commented May 12, 2017

I don't think I'll be able to get to the making the improvements for another few days, so feel free to merge this in if you want.

@brson
Copy link
Contributor

brson commented Jun 9, 2017

For this PR I agree that at least it would be better to remove reading "SHELL" and just look for the presense of .bash_profile. zsh changes can be left for some other PR.

This should really also have a test case. Crib off of "install_adds_path_to_profile".

@brson brson mentioned this pull request Jun 24, 2017
bors added a commit that referenced this pull request Jun 24, 2017
Bash profile

Continuation of @awelkie's previous PR: #1090
@brson
Copy link
Contributor

brson commented Jun 24, 2017

I've updated this in #1179.

@brson brson closed this Jun 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants