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

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

Merged
merged 3 commits into from
May 15, 2018
Merged

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

merged 3 commits into from
May 15, 2018

Conversation

gtalarico
Copy link
Contributor

@gtalarico 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
Copy link
Contributor Author

gtalarico commented May 9, 2018

Fixes #2116

@techalchemy
Copy link
Member

Is this really all it takes...

@techalchemy
Copy link
Member

BTW we should upstream these to pew as well

@techalchemy techalchemy added OS: Windows This issue affects the Windows Operating System. Type: Vendored Dependencies This issue affects vendored dependencies within pipenv. labels May 9, 2018
@techalchemy
Copy link
Member

@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
Copy link
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
Copy link
Member

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS: Windows This issue affects the Windows Operating System. Type: Vendored Dependencies This issue affects vendored dependencies within pipenv.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants