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

Adds environment name to PROMPT on Cmd, Cmder,ConEmu #2167

Merged
merged 3 commits into from May 15, 2018

Conversation

Projects
3 participants
@gtalarico
Contributor

gtalarico commented May 9, 2018

This PR makes cmd.exe and cmder/conemu show the virtual environment's name
in those shell's prompt.

This edit could also be in fork_cmder() , but that would not fix cmd.exe.
I understand the entire section might need some re-work and clean up but until then this should be an easy and contained fix that will make a lot of windows users happy.

image

PS: this is my first PR for a large project

@gtalarico

This comment has been minimized.

Contributor

gtalarico commented May 9, 2018

Fixes #2116

@techalchemy

This comment has been minimized.

Member

techalchemy commented May 9, 2018

Is this really all it takes...

@techalchemy

This comment has been minimized.

Member

techalchemy commented May 9, 2018

BTW we should upstream these to pew as well

@techalchemy

This comment has been minimized.

Member

techalchemy commented May 9, 2018

@uranusjr This change is pretty small, while the 11.11 changes are somewhat large in scope. I really like the idea of getting these improvements and quality of life fixes done, if you think these are ok (and we get an upstream pr to pew opened) we can consider point releasing for these

@uranusjr

This comment has been minimized.

Member

uranusjr commented May 9, 2018

In general it is (%PROMPT% is essentially PS1 for cmd), but I wonder what consequences would it have on custom init scripts (clink.lua for example). Will it result in duplicate venv names, or put it in the wrong place? We’ll need to put this in the wild and see what happens. Yeah I’d say let’s do this.

@techalchemy

This comment has been minimized.

Member

techalchemy commented May 9, 2018

Seems like a good reason to get this out before a minor release, we can work through that stuff.

@techalchemy techalchemy merged commit da5a8b3 into pypa:master May 15, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment