Don't show 'try %paste' message while using magics #2744

Merged
merged 6 commits into from Jan 5, 2013

Conversation

Projects
None yet
2 participants
Contributor

mrshu commented Jan 5, 2013

This pull request tries to fix the behavior described in #1661.

@@ -348,6 +350,8 @@ class TerminalInteractiveShell(InteractiveShell):
help="Enable auto setting the terminal title."
)
+ using_magics = CBool(False)
@takluyver

takluyver Jan 5, 2013

Owner

I think the name should be a bit more specific, perhaps using_paste_magics. Also, a note here saying what it's for would be good.

@@ -150,6 +150,8 @@ def cleanup_input(self, block):
def store_or_execute(self, block, name):
""" Execute a block, or store it in a variable, per the user's request.
"""
+ self.shell.using_magics = True
@takluyver

takluyver Jan 5, 2013

Owner

This function is store_or_execute, but we only need to set the flag in the execute branch, just before it calls run_cell(). Also, don't forget to set the flag back to False afterwards.

Owner

takluyver commented Jan 5, 2013

Yes, a test case would be good for this. It should go in IPython.frontend.terminal.tests.test_interactiveshell. I guess you'll want to create a new TerminalMagics instance (get the testing shell using get_ipython()), and call store_or_execute(). You can use IPython.testing.tools.AssertPrints and AssertNotPrints to check when the message about %paste appears.

(Don't be disheartened if it feels like I'm giving you a lot of criticism, by the way. We have pull requests so we can do code review and improve contributions, and even regular developers rarely get everything right on the first go)

Contributor

mrshu commented Jan 5, 2013

No hard feelings at all!

This is the first time I'm playing with the inside of IPython so I'm glad there is someone who can guide me.

I'll try to add the test shortly.

Contributor

mrshu commented Jan 5, 2013

Ok, I added a simple test but I couldn't find a way to test the situation when it should only rise the IndentationError. Cloud you please give some hints?

self.shell.run_cell(b)
+ self.shell.using_paste_magics = True
@takluyver

takluyver Jan 5, 2013

Owner

I think you mean False ;-).

I'd also be inclined to use a try/finally block, so that it gets set back even if run_cell throws an error. It should catch all errors, but better safe than sorry.

@mrshu

mrshu Jan 5, 2013

Contributor

Oh yes, I did =D. This is what code reviews are great for.

Owner

takluyver commented Jan 5, 2013

I haven't tried this, but something like this ought to do the trick:

tm = TerminalMagics(ip)
with tt.AssertNotPrints("If you want to paste code"):
    tm.store_or_execute(s, name=None)
Contributor

mrshu commented Jan 5, 2013

Well, it seems to work.
Thanks!

Owner

takluyver commented Jan 5, 2013

Thanks. That looks good to me, and I've given it a quick test manually.

takluyver added a commit that referenced this pull request Jan 5, 2013

Merge pull request #2744 from mrshu/magics_fix
Don't show 'try %paste' message while using magics

@takluyver takluyver merged commit fed8f7d into ipython:master Jan 5, 2013

1 check passed

default The Travis build passed
Details

@mrshu mrshu deleted the mrshu:magics_fix branch Jan 5, 2013

Owner

takluyver commented Jan 5, 2013

P.S. Well done on getting your contribution merged. Now you know the process, it will be that much easier next time you find something you want to fix. :-)

Contributor

mrshu commented Jan 5, 2013

Yeah, thanks a lot for all the help!

Now comes the difficult part, finding something to work on =)

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

Merge pull request #2744 from mrshu/magics_fix
Don't show 'try %paste' message while using magics
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment