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

IPython/lib/editorhooks.py: wait for process even if wait=False #10239

Merged
merged 1 commit into from Feb 3, 2017

Conversation

@segevfiner
Copy link
Contributor

segevfiner commented Feb 3, 2017

The wait parameter is meant to add a prompt before returning from
the hook for editors that exit immediatly (fork/CreateProcess) but
it accidently prevented waiting at all for the process when it was
False. I think it was meant to be not wait and proc.wait() but we
might as well wait for the process in the wait=True case anyhow. It's
less confusing.

It would be nice if this is backported to 5.x.

The wait parameter is meant to add a prompt before returning from
the hook for editors that exit immediatly (fork/CreateProcess) but
it accidently prevented waiting at all for the process when it was
False. I think it was meant to be `not wait and proc.wait()` but we
might as well wait for the process in the `wait=True` case anyhow. It's
less confusing.
@segevfiner segevfiner force-pushed the segevfiner:editorhooks-wait-fix branch from a6e592a to 7e3493b Feb 3, 2017
@takluyver takluyver added this to the 5.3 milestone Feb 3, 2017
@takluyver takluyver merged commit e311b1d into ipython:master Feb 3, 2017
4 checks passed
4 checks passed
codecov/patch 100% of diff hit (target 0%)
Details
codecov/project 66.28% (+<.01%) compared to e834116
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@takluyver
Copy link
Member

takluyver commented Feb 3, 2017

Thanks, this seems reasonable.

@segevfiner segevfiner deleted the segevfiner:editorhooks-wait-fix branch Feb 3, 2017
@takluyver
Copy link
Member

takluyver commented Feb 3, 2017

@meeseeksdev backport to 5.x

meeseeksdev bot pushed a commit that referenced this pull request Feb 3, 2017
… if wait=False

The wait parameter is meant to add a prompt before returning from
the hook for editors that exit immediatly (fork/CreateProcess) but
it accidently prevented waiting at all for the process when it was
False. I think it was meant to be `not wait and proc.wait()` but we
might as well wait for the process in the `wait=True` case anyhow. It's
less confusing.

It would be nice if this is backported to 5.x.
takluyver added a commit that referenced this pull request Feb 3, 2017
Backport PR #10239 on branch 5.x
@Carreau Carreau added the backported label Feb 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.