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

Escpace spaces on cmder shell path - Fixes #2115 #2168

Merged
merged 6 commits into from May 23, 2018
Merged

Escpace spaces on cmder shell path - Fixes #2115 #2168

merged 6 commits into from May 23, 2018

Conversation

@gtalarico
Copy link
Contributor

@gtalarico gtalarico commented May 9, 2018

This PR fixes #2115 by escaping space characters windows-style %CMDER_ROOT%

The issue, which is detailed on #2115, happens when Cmder is running from a location which contains space characters.

@uranusjr
Copy link
Member

@uranusjr uranusjr commented May 9, 2018

This looks good, but would cause conflicts with #2161 (because I rewrote too many things). I’d want to push this in a 11.10.x bugfix release (if there is), and remember to apply this in 11.11.

@gtalarico gtalarico changed the title Espace spaces on cmder shell path - Fixes #2115 Escpace spaces on cmder shell path - Fixes #2115 May 9, 2018
@techalchemy
Copy link
Member

@techalchemy techalchemy commented May 9, 2018

I think I'd rather rework this for 11.11 if it still applies

@techalchemy
Copy link
Member

@techalchemy techalchemy commented May 9, 2018

As an additional note for this pr and for #2167 you will need to add a patch to tasks/vendoring/patches/patched/pew-some-description.patch using git diff -p pipenv/patched/pew/ > tasks/vendoring/patches/patched/pew-some-description.patch where you fill n the description -- this is for our automated vendoring process.

@gtalarico
Copy link
Contributor Author

@gtalarico gtalarico commented May 9, 2018

I think I'd rather rework this for 11.11 if it still applies

👍 yes I am not proud of this one :)

you will need to add a patch to

done

@techalchemy
Copy link
Member

@techalchemy techalchemy commented May 9, 2018

@gtalarico the rework comment is more with regard to the changes in #2161

@kennethreitz kennethreitz merged commit 220d0c9 into pypa:master May 23, 2018
0 of 2 checks passed
0 of 2 checks passed
buildkite/pipenv Build #174 scheduled
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
@techalchemy techalchemy moved this from Done to Needs Changelog in 2018.06.x Release Jun 16, 2018
@techalchemy techalchemy moved this from Needs Changelog to Done in 2018.06.x Release Jun 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Linked issues

Successfully merging this pull request may close these issues.

4 participants