?? prompt should be wrapped in paranthesis ??

[tusharsadhwani]

Thanks for contributing, make sure you address all the checklists (for details on how see

development documentation)!

• ran the linter to address style issues (tox -e fix_lint)
• wrote descriptive pull request text
• ensured there are test(s) validating the fix
• added news fragment in docs/changelog folder
• updated/extended the documentation

---

This adds brackets and a space around the prompt text, in the case of --prompt=.

Previous behaviour:

$ pwd
/code/myproject

$ virtualenv venv --prompt .
[...]
$ . venv/bin/activate
myproject$ 

New behaviour:

$ virtualenv venv --prompt .
[...]
$ . venv/bin/activate
(myproject) $ 

---

This is a super trivial change, should I make a news fragment for this?

[gaborbernat]

Also this is not a fix. it's a feature as it changes current behaviour.

[gaborbernat]

The logic looks alright, can you amend the existing tests to check for it?

[tusharsadhwani]

I can't find where/how I'd add a test for it :')
I can't see anywhere the actual PS1 is being tested

[gaborbernat]

I can't find where/how I'd add a test for it :')
I can't see anywhere the actual PS1 is being tested

Probably here https://github.com/pypa/virtualenv/blob/main/tests/unit/activation/conftest.py#L125 you'd need to extend it 🤔 and then assert here https://github.com/pypa/virtualenv/blob/main/tests/unit/activation/conftest.py#L139

[tusharsadhwani]

Okay, adding platform-specific prompt checks will take a while.
PS1 only exists on bash it seems.
For powershell I'd have to run the prompt function somehow? Not sure about the rest at all

[gaborbernat]

Thank you! No worries. We are not in a rush 😊

[tusharsadhwani]

Sorry for testing everything on CI, but it all works now.

Weirdly though, the test_fish[no_prompt] test is failing on CI while it works fine locally. For some reason the order of %s in the printf command gets swapped on CI for the fish activation script. (Edit: Removing the set_color calls does work around that.)

As for powershell, the non-ASCII characters are messing up.
Expected: '(e-$ è-j) PS '
Actual prompt: '(e-$ è-j) PS '

Expected: 'èрт🚒♞中片-j\r\n PS '
Actual: '(e-$ èрт🚒♞ä¸\xad片-j) PS '
To fix this, I had to make it so that windows only gets an ascii venv name.

[gaborbernat]

Sorry for testing everything on CI, but it all works now.

Weirdly though, the test_fish[no_prompt] test is failing on CI while it works fine locally.

The CI is the true judge of everything works fine 😃 so you'll need to fix the CI for this to be ready to review 🙏

[tusharsadhwani]

@gaborbernat All done, PTAL :)

[gaborbernat]

Looks good the only thing remaining is a changelog entry, as a feature, announcing the change in behaviour.

[tusharsadhwani]

It's crazy that some of these checks take 60-90 mins to run. There must be a better way 🤔

But anyway, doc entry has also been added, hopefully I did it right.

[gaborbernat]

It's crazy that some of these checks take 60-90 mins to run. There must be a better way

Not that I'm aware so if you have suggestions don't hold back.