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

Fix #11334: Cannot make multi-line code blocks in ipython #11354

Merged
merged 6 commits into from Oct 8, 2018

Conversation

@bxsx
Copy link
Contributor

@bxsx bxsx commented Oct 1, 2018

When codeop.compile_command() returns None it actually says "at least
some part of the code was compiled successfully" which is not really important
for checking if it's complete or not.
Once we haven't got any errors during compilation process, we just want
to check if there will be another nested block of code or not by checking
a colon.

When codeop.compile_command() returns None it actually says "at least
some part of the code was compiled successfully" which is not really important
for checking if it's complete or not.
Once we haven't got any errors during compilation process, we just want
to check if there will be another nested block of code or not by checking
a colon.
@Carreau
Copy link
Member

@Carreau Carreau commented Oct 1, 2018

That seem incorrect, now a = 1 is considered as incomplete, while it's not.

@bxsx
Copy link
Contributor Author

@bxsx bxsx commented Oct 1, 2018

Yeah, I have already fixed it. Trying to run a test suit locally but I get a lot of errors and unfortunately can't understand why (is there any dev-guide how to develop ipython locally?).

Anyway, I'm pushing the fix, to see what CI will say...

@bxsx
Copy link
Contributor Author

@bxsx bxsx commented Oct 1, 2018

That seem incorrect, now a = 1 is considered as incomplete, while it's not.

Seems it's OK now

@Carreau
Copy link
Member

@Carreau Carreau commented Oct 3, 2018

There seem to be some more edge cases:

if True:
    1
<no spaces>

Appear as incomplete and adds a new line with 4 spaces.

if True:
    1
	<4 spaces>

Appear as complete an executes.

@Carreau Carreau requested a review from takluyver Oct 3, 2018
@bxsx
Copy link
Contributor Author

@bxsx bxsx commented Oct 3, 2018

I am very busy today but this looks like an another bug to me that we haven't noticed before because of this bug.

If there is at least one space in the last line (blank) then the line is complete.

if True:
    1
<at least 1 space>

this seem to work correctly.

The bug is in a code before check_complete() is being called as it's already missed in the cell argument passed to the function.

I can take a look in it closer but not before weekend.

@bxsx
Copy link
Contributor Author

@bxsx bxsx commented Oct 3, 2018

InteractiveShell.check_complete() invokes TransferManager.check_complete() and seems the passing code argument doesn't have a trailing '\n' in any situation. I believe this is for some reasons and is correct, @Carreau ?

If so, this helped me to focus back to the TransferManager.check_complete() and seems like removing the condition fixes the issue but provides a regression in another test. This actually surprised me honestly.

Going off for a day or two, leaving the comment for notes, but feel free to share some information if you have or finish the fix obviously ;-)

PS. @Carreau is there any guide how to develop and test locally IPython? When I want to use IPython installed globally (to meet all requirements etc.) I have no access to the IPython-dev directory as all imports are already done for starting IPython itself. I can figure it out how to do this but maybe there is a note about it somewhere?

@Carreau
Copy link
Member

@Carreau Carreau commented Oct 3, 2018

InteractiveShell.check_complete() invokes TransferManager.check_complete() and seems the passing code argument doesn't have a trailing '\n' in any situation. I believe this is for some reasons and is correct, @Carreau ?

@takluyver wrote most of this,

PS. @Carreau is there any guide how to develop and test locally IPython? When I want to use IPython installed globally (to meet all requirements etc.) I have no access to the IPython-dev directory as all imports are already done for starting IPython itself. I can figure it out how to do this but maybe there is a note about it somewhere?

Clone the repo and run pip install -e . that should make a dev install that reflect modification to the source code.

I use cd ; iptest IPython.core.tests.test_inputtransformer2:test_check_complete; cd - to run a single test, and be sure not to be in IPython source tree when running the test.

https://github.com/ipython/ipython#development-and-instant-running is basically links to longer versions than this.

If I have time outside of $DAYJOB I'll try have a look, but @takluyver is the expert, and @tonyfast sent a PR to fix another bug recently so may have some insights.

@Carreau Carreau merged commit 498f5f7 into ipython:master Oct 8, 2018
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@Carreau Carreau added this to the 7.1 milestone Oct 13, 2018
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

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