-
Notifications
You must be signed in to change notification settings - Fork 11
fix bash startup file updating #20
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
* case for mac users: not all have `.bashrc`, I personally have `.profile` instead, added redundancy to that step by checking for startup files in order by which bash searches for them * fixed `-ne 0` to `-eq 0` to avoid adding source multiple times
just a ping |
Apologies @76creates, I've only just seen this old PR! I know @HighHarmonics2 is working on this and might have a thought about whether this can be integrated with their work. |
Yes, I'm working on a full refactor and will add a branch with that work soon. The changes proposed by @76creates look good and shouldn't impact what I'm doing. I will give a closer look and figure out how to proceed. |
I looked into this and I don't think we can merge this code as is. The intention is right - to allow for all possible shell profile variants: .bashrc, .bash_profile, .bash_login, .profile. The problem comes if you have multiple shell profiles. This can come from using different shells, or other factors. I have 3: .bashrc, .bash_profile, and .zshrc, which is for the new default shell = zsh. With this change, the conditional will keep testing for the different profile files and only update the last one it finds in the list. Also, .zshrc is missing, but that would be easy to add one more. It would be possible to refactor this so all profiles discovered would get the .ghcup/env commands. |
@76creates I see you are following this thread! Would you be willing to refactor your PR to add .zshrc and to add the .ghcup/env commands to all shell profile files found? Since this PR is still open, I think you could just add an additional commit. |
@HighHarmonics2 for sure, lemme see what I can brew and I will post it over the weekend ⚡ |
@76creates OK the refactor branch has been merged. So master now has the current version with some changes to the relevant section you are contributing. I'm not sure if that means you can modify this PR with new commits, or if you would need to start over. |
Would be great to resolve this issue. I was confused why Otherwise the script worked flawlessly! |
Ye sorry, this one got off my radar completely. Will try to fix something up in few days time. |
Small lexical fix
@HighHarmonics2 finally updated the script, give it a look ⚡ |
Thanks @76creates. I'll look into this. It may take a few days to get familiar with the code again. It's been awhile since I worked on this. |
.bashrc
, I personally have.profile
instead, added redundancy to that step by checking for startup files in order by which bash searches for them-ne 0
to-eq 0
to avoid adding source multiple times