-
-
Notifications
You must be signed in to change notification settings - Fork 960
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
modify declaration of dir variable in sections/dir #421
modify declaration of dir variable in sections/dir #421
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unable to reproduce the issue.
Edit: There was a mistake from my side during earlier reproducing attempts. This is reproducible and affects every unassigned initialized variables, Which have global alias defined with user configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good to go 👌
@gurpreetatwal do you consider to address the similar variable assignments ? If not, We could merge this.
sections/dir.zsh
Outdated
@@ -21,7 +21,7 @@ SPACESHIP_DIR_COLOR="${SPACESHIP_DIR_COLOR="cyan"}" | |||
spaceship_dir() { | |||
[[ $SPACESHIP_DIR_SHOW == false ]] && return | |||
|
|||
local dir | |||
local 'dir' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please leave a comment about using quotes here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be default behavior for all unassigned local variables. Documenting it in doc
would be better than leaving a single comment.
- explicitly assigning the variable prevents it accidentally expanding the dir variable if there is a shell alias assigned to it
as suggested in #420 (comment) fix #420
@denysdovhan @salmanulfarzy rebased and cleaned up the PR
|
@@ -27,7 +27,7 @@ spaceship_venv() { | |||
# Check if the current directory running via Virtualenv | |||
[ -n "$VIRTUAL_ENV" ] || return | |||
|
|||
local venv | |||
local 'venv' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could see this as being troublesome as well
example:
alias -g venv='python3 -m venv'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
modify declaration of dir variable in sections/dir
Description
potential fix for #420, feel free to close this PR if a better solution can be found
Fixes the issue by explicitly assigning the variable to prevent it from accidentally expanding the dir variable if there is a shell alias assigned to it.
Fix #420