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

Add .zshrc to install script, since it was missing. #63

Merged
merged 1 commit into from Apr 8, 2023

Conversation

emirkmo
Copy link
Contributor

@emirkmo emirkmo commented Mar 27, 2023

Add check for .zshrc after .zprofile to enable installing on a wider linux platform base. Also Make it so that $PATH is not modified if usr/local/bin already exists on path in .zshrc. (Added a new PR to make this default on other install methods as well).

Problem:

The install script fails if there is only .zshrc but no .zprofile. This is a common setup as .zshrc is actually the place to declare packages etc. for use in any interactive shell and .zprofile is supposed to be only for login shells: See unix.stackexchange discussion.

Add check for `.zshrc` after `.zprofile` to enable installing on a wider linux platform base. Also Make it so that `$PATH` is not modified if `usr/local/bin`lready exists on path in `.zshrc`.

The install script fails if there is only .zshrc but no .zprofile. This is a common setup as .zshrc is actually the place to declare packages etc. for use in any interactive shell and .zprofile is supposed to be only for login shells: [See unix.stackexchange discussion](https://unix.stackexchange.com/questions/71253/what-should-shouldnt-go-in-zshenv-zshrc-zlogin-zprofile-zlogout).
@0xacx
Copy link
Owner

0xacx commented Apr 8, 2023

Thank you for your PR and concern on interactive login shells.
I understand the differences between zprofile and zshrc in theory but in practice, since zprofile is sourced before zshrc, any variables should already be accessible.
I wouldn't like to add another file for zsh, as that would increase complexity unnecessarily but after reading the issue where you said that zprofile does not exist in debian, I decided to merge this PR.

@0xacx 0xacx merged commit f49ec79 into 0xacx:main Apr 8, 2023
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.

None yet

2 participants