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

Respect VIRTUAL_ENV_DISABLE_PROMPT in nushell activation script #2458

Merged
merged 3 commits into from
Dec 5, 2022
Merged

Respect VIRTUAL_ENV_DISABLE_PROMPT in nushell activation script #2458

merged 3 commits into from
Dec 5, 2022

Conversation

m-lima
Copy link
Contributor

@m-lima m-lima commented Dec 4, 2022

The activate.nu script was not respecting the VIRTUAL_ENV_DISABLE_PROMPT like other scripts. This PR addresses that.

The changes can be summarized as:

  • Only add the prompt-related env vars if $env.VIRTUAL_ENV_DISABLE_PROMPT evaluates to false
  • Like with other activation scripts, it will evaluate to false if the env var is missing, it is empty, it is '0'. Also, because of nushell type system and the use of the into bool built-in, anything that nushell can covert into a false will also work, e.g. the strings false and 0000 or 0.0.

There are no changes to the documentation since this is already an expected behavior.
Also, I did not add an entry to the changelog as the developer documentation states that "non trivial" changes should be added. I considered this change trivial, but please let me know if an entry is required.

Fixes: #2461

Copy link
Contributor

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

This change is 52 new lines, and 28 removals, this is not trivial so let's add the changelog.

@m-lima
Copy link
Contributor Author

m-lima commented Dec 5, 2022

This change is 52 new lines, and 28 removals, this is not trivial so let's add the changelog.

Sure, no problem. I'll create an issue then, so that I can reference the changelog (and fix the test)

By the way, that amount of changes is the indentation.. In reality is was more like 20 additions, that's why I made my initial judgement of triviality

@m-lima
Copy link
Contributor Author

m-lima commented Dec 5, 2022

Interesting error. Looks like libc was not found to execute nushell? Or maybe nushell requires a specific version of glibc that is not present in the ubuntu vms running the test?
Or am I reading this wrong?

@gaborbernat gaborbernat merged commit d3215e4 into pypa:main Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

activate.nu is not respecting VIRTUAL_ENV_DISABLE_PROMPT
2 participants