classification
Title: activate.fish sets VENV prompt incorrectly
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: Dan McCombs, brett.cannon, python-dev, vinay.sajip, xiang.zhang
Priority: normal Keywords: patch

Created on 2016-02-12 13:08 by Dan McCombs, last changed 2016-07-01 04:37 by vinay.sajip. This issue is now closed.

Files
File name Uploaded Description Edit
activate-fish.patch Dan McCombs, 2016-02-12 13:08 review
Messages (9)
msg260175 - (view) Author: Dan McCombs (Dan McCombs) * Date: 2016-02-12 13:08
Currently, the activate.fish VENV script sets fish's prompt to be prepended with a literal "__VENV_PROMPT__" rather than the contents of the $__VENV_PROMPT__ variable as intended.

The attached patch simply adds the missing "$" to the variable in the conditional test and it's usage, so it's only being set if the variable is non-zero, rather than testing if the string "__VENV_PROMPT__" is non-zero like it is doing right now.

The results in the prompt being correctly prepended by "(my_actual_venv_name)".
msg260243 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-02-13 16:27
New changeset cfc66e37eb8e by Vinay Sajip in branch '3.5':
Fixes #26348: Corrected typos in activate.fish script.
https://hg.python.org/cpython/rev/cfc66e37eb8e

New changeset 0f1ac94d2f98 by Vinay Sajip in branch 'default':
Fixes #26348: Merged fix from 3.5.
https://hg.python.org/cpython/rev/0f1ac94d2f98
msg262764 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2016-04-01 19:49
Implementing this patch has led to another issue being raised: #26664. Dan - would you care to take a look and comment? Thanks.
msg264385 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-04-27 17:53
The original behaviour is right. __VENV_PROMPT__ is meant to be a placeholder, not a variable interpreted by shell. I think this patch should be reverted. Related issue is #26664.
msg264395 - (view) Author: Dan McCombs (Dan McCombs) * Date: 2016-04-27 19:42
I somehow missed the comment on 2016-04-01, I apologize for being late to look at this.

Xiang is right, this patch should be reverted.

It looks like the issue reported was originally brought on by an activate.fish being used from a Python after the patch in commit 7f913c6ada03 was added, but the running version of Python did not include the patch, so that the placeholder was not being swapped out. In that case, updating the script to include $ and use the variable fixed the problem, but when the patch in 7f913c6ada03 is also included it causes the placeholder to be swapped to a name wrapped in parenthesis preceded by $ at the time the venv is created causing a syntax error in fish.

Since the patch in commit 7f913c6ada03 swaps that place holder and handles doing so in the other activate scripts as well, the patch that was part of this issue (cfc66e37eb8e) is not needed and should be reverted.

Here's the commit adding that placeholder for context:

https://hg.python.org/cpython/rev/7f913c6ada03
msg264396 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-04-27 19:50
So just to be clear, cfc66e37eb8e and 0f1ac94d2f98 should be reverted? No other changes?

And will this fix issue #26664 or affect it in any way?
msg264397 - (view) Author: Dan McCombs (Dan McCombs) * Date: 2016-04-27 19:59
Yes, reverting cfc66e37eb8e and 0f1ac94d2f98 should resolve #26664.
msg264398 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-04-27 20:01
I'll revert the two commits then and close both this and #26664 when I have a chance.
msg264399 - (view) Author: Dan McCombs (Dan McCombs) * Date: 2016-04-27 20:03
Thanks Brett.
History
Date User Action Args
2016-07-01 04:37:39vinay.sajipsetstatus: open -> closed
2016-04-27 20:03:01Dan McCombssetmessages: + msg264399
2016-04-27 20:01:45brett.cannonsetmessages: + msg264398
2016-04-27 19:59:06Dan McCombssetmessages: + msg264397
2016-04-27 19:50:36brett.cannonsetstatus: closed -> open
assignee: brett.cannon
messages: + msg264396
2016-04-27 19:42:15Dan McCombssetmessages: + msg264395
2016-04-27 17:53:07xiang.zhangsetnosy: + xiang.zhang
messages: + msg264385
2016-04-01 19:49:23vinay.sajipsetmessages: + msg262764
2016-02-13 16:30:42vinay.sajipsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2016-02-13 16:27:41python-devsetnosy: + python-dev
messages: + msg260243
2016-02-13 04:03:38brett.cannonsetnosy: + brett.cannon
2016-02-12 23:38:26terry.reedysetnosy: + vinay.sajip
stage: patch review

versions: - Python 3.3, Python 3.4
2016-02-12 13:08:59Dan McCombscreate