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

[Bugfix] Update VIRTUAL_ENV_DISABLE_PROMPT value #1079

Merged
merged 1 commit into from Nov 19, 2018

Conversation

Projects
None yet
4 participants
@josselinauguste
Contributor

josselinauguste commented Nov 17, 2018

Following a prezto update, the flag value of the VIRTUAL_ENV_DISABLE_PROMPT variable is now 12 (https://github.com/sorin-ionescu/prezto/blob/e9387a177e04b05dcf4f82e622615ddd32c558db/modules/python/init.zsh#L103).

@dritter

This comment has been minimized.

Collaborator

dritter commented Nov 18, 2018

Hi @josselinauguste , thanks for this PR. TBH, I don't know if this is right or not. The relevant commit in prezto is sorin-ionescu/prezto@ee885d4 and they reference this change in pure sindresorhus/pure@afa625b .

Why don't we just check if VIRTUAL_ENV_DISABLE_PROMPT is not empty? That would work with any value. On the downside this would disable the segment, even if the value would be set to false.. But to me it seems better than checking magic values..

@josselinauguste

This comment has been minimized.

Contributor

josselinauguste commented Nov 18, 2018

TBH, I don't know neither :-) The only thing I know is that the previous version didn't work, and this one does work.

I'm gonna try by checking if it's empty.

Update VIRTUAL_ENV_DISABLE_PROMPT value
Following prezto update

@josselinauguste josselinauguste force-pushed the josselinauguste:master branch from b4e89e7 to 5c412b4 Nov 18, 2018

@josselinauguste

This comment has been minimized.

Contributor

josselinauguste commented Nov 18, 2018

It seems to work, the PR is updated!

@bhilburn

This comment has been minimized.

Owner

bhilburn commented Nov 19, 2018

Great recommendation, @dritter!

@josselinauguste - Thanks for putting together this PR and iterating on it with @dritter! This is a great first contribution to P9k - welcome to the project! =)

@bhilburn bhilburn merged commit 2f4b150 into bhilburn:master Nov 19, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@shengje

This comment has been minimized.

shengje commented on 5c412b4 Dec 6, 2018

I try this theme for a while but can not print the prompt virtualenv correctly.
Finally I change the code like this
if [[ -n "$virtualenv_path" && -z "$VIRTUAL_ENV_DISABLE_PROMPT" ]]; then
need to change into
if [[ -n "$virtualenv_path" && -n "$VIRTUAL_ENV_DISABLE_PROMPT" ]]; then

I'm not sure is my own problem or bug, just let anyone who has same problem to know this.
(my system : ubuntu 18.04 LTS with shell OMZ )

This comment has been minimized.

Collaborator

dritter replied Dec 6, 2018

Hi @shengje ,

To be honest, your suggested change does not look right to me. And to be fair, @josselinauguste doesn't look right either. The Variable is named disable_prompt , so it expects a certain value like true or false. Checking the entire variable for (non) zero length does not work if the variable is set to true/false:
screenshot_20181206_181444

As a workaround, if you do unset VIRTUAL_ENV_DISABLE_PROMPT, it should work..

This comment has been minimized.

shengje replied Dec 7, 2018

Hi @dritter ,
Thanks for correcting my suggestion!
Now I knew this workflow of this script.
This correction references by the theme in the original OMZ package.
I found that there is no variable VIRTUAL_ENV_DISABLE_PROMPT setting true or false in this code by default.
Is it equal unset VIRTUAL_ENV_DISABLE_PROMPT ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment