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

bpo-35328: Always set VIRTUAL_ENV_PROMPT at venv activation #22324

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mafredri
Copy link

@mafredri mafredri commented Sep 19, 2020

This is a follow-up to GH-21587 and modifies the behavior so that VIRTUAL_ENV_PROMPT is always set.

The change is needed because VIRTUAL_ENV_PROMPT is being conditionally set only if VIRTUAL_ENV_DISABLE_PROMPT is unset. Shell prompts must set VIRTUAL_ENV_DISABLE_PROMPT to non-empty to prevent venv from hijacking PS1. I'm one of the maintainers of the Pure prompt for zsh, and for us it would be great if VIRTUAL_ENV_PROMPT could always be set, just as VIRTUAL_ENV is too.

No news added since this PR is true to the news written in GH-21587.

https://bugs.python.org/issue35328

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@mafredri

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

Copy link
Contributor

@MaxwellDupre MaxwellDupre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried this with 3.11.0.a4 with this branch and I cant get it to work with either
virtualenv venv --prompt '(wat)'
or
python -m venv --prompt '(wat)' ./venv

I notice that there are files for activate csh and fish, hence why don't you make this work with a activate.zsh. Also, there are other scripting solutions on stackExchange:
https://unix.stackexchange.com/questions/87062/how-to-display-the-name-of-the-current-virtualenv

However, if your solution does work please show results in Issue35328

@mafredri
Copy link
Author

I have tried this with 3.11.0.a4 with this branch and I cant get it to work with either virtualenv venv --prompt '(wat)' or python -m venv --prompt '(wat)' ./venv

What doesn't work?

I notice that there are files for activate csh and fish, hence why don't you make this work with a activate.zsh. Also, there are other scripting solutions on stackExchange: https://unix.stackexchange.com/questions/87062/how-to-display-the-name-of-the-current-virtualenv

The Stack Exchange post does not address the issue that this PR is trying to amend, which is to make the custom prompt available via environment variable even if disable prompt is enabled.

@MaxwellDupre
Copy link
Contributor

I have recently updated my clone and checked out your PR:
Screenshot from 2022-02-01 18-12-43
Then running a zsh with python -m venv .venv --prompt env

I get
zsh

I see % prompt but expected (wat) %

Also, I note that virtualenv is not core cpython but a pypi package..

@mafredri
Copy link
Author

mafredri commented Feb 1, 2022

@MaxwellDupre are you saying the behavior is different without this patch? This PR does not touch anything that affects displaying of the virtualenv name in the prompt.

Copy link
Contributor

@MaxwellDupre MaxwellDupre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, ok. Then looks ok to me.

@sguth-affirm
Copy link

Bump, ran into an issue today where I needed this env. variable set despite the presence of VIRTUAL_ENV_DISABLE_PROMPT. Any chance it can get merged?

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

Successfully merging this pull request may close these issues.

8 participants