install: use .bashrc and notify user
#2039
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gaelicWizard
force-pushed
the
lib/install
branch
from
e72e72e
to
e2f1ce3
Compare
NoahGorny
reviewed
Jan 24, 2022
gaelicWizard
force-pushed
the
lib/install
branch
5 times, most recently
from
1212ec9
to
a45c914
Compare
gaelicWizard
force-pushed
the
lib/install
branch
2 times, most recently
from
c742a5d
to
d718669
Compare
tbhaxor
suggested changes
Feb 8, 2022
gaelicWizard
force-pushed
the
lib/install
branch
3 times, most recently
from
18196a6
to
1bcc3f4
Compare
tbhaxor
approved these changes
Feb 9, 2022
gaelicWizard
force-pushed
the
lib/install
branch
4 times, most recently
from
3efc183
to
7fd8deb
Compare
NoahGorny
requested changes
Feb 13, 2022
gaelicWizard
force-pushed
the
lib/install
branch
8 times, most recently
from
90fcb4d
to
eb14b34
Compare
|
Anyway, I've just learned that |
gaelicWizard
force-pushed
the
lib/install
branch
3 times, most recently
from
0f14c28
to
118861f
Compare
gaelicWizard
force-pushed
the
lib/install
branch
4 times, most recently
from
69dd2fe
to
2f7da32
Compare
|
Ok, there should now be tests covering both cases of existing |
The logic to guess whether to use `.bash_profile` or `.bashrc` was buggy and wrong. Just use `.bashrc` and either automatically fill in a `.bash_profile`, or notify the user that they need to edit their `.bash_profile`.
docs/installation: add to note about interactive/login shells
- Alsö, add implementation note at top.
uninstall: TIL that `fgrep` is deprecated...
- and generally comment out useless varbls
gaelicWizard
force-pushed
the
lib/install
branch
from
2f7da32
to
affbce6
Compare
NoahGorny
approved these changes
Dec 5, 2022
There was a problem hiding this comment.
Hi @gaelicWizard
sorry for the long time for the review, but I think it looks good.
@davidpfarrell - wanna take a look?
|
Is this a dead end or does anyone want to ressurect this PR andstill merge it? |
seefood
added
seems abandoned
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The logic to guess whether to use
.bash_profileor.bashrcwas buggy and wrong. Just use.bashrcand either automatically fill in a.bash_profile, or notify the user that they need to edit their.bash_profile.This closes #1455, fixes #2084, and resolves #2022.
Motivation and Context
I honestly don't think that we should ever modify a user's
~/.bashrcor~/.bash_profile, but if we do then we should be better about it.TODO: add an optional pass to
_bash-it-migratewhich moves~/.bash_profileto~/.bashrcif the latter doesn't exist.How Has This Been Tested?
Types of changes
Checklist:
clean_files.txtand formatted it usinglint_clean_files.sh.