Skip to content
This repository

Rewrite quit()/exit()/Quit()/Exit() calls as magic #353

Closed
wants to merge 2 commits into from

3 participants

David Warde-Farley Thomas Kluyver Fernando Perez
David Warde-Farley
dwf commented

This is a resubmit of #345, GitHub wasn't updating the range, perhaps because I pushed those changes while the pull request was closed. #352 was an alternate plan that was decided against.

This attempts to resolve the complaint in #142 about quit() and exit() and their capitalized counterparts, by explicitly checking for these forms (with a regexp, for speed) and rewriting them as their corresponding magics. Just checks for at least one pair of parens following the initial token, with anything in between; nothing fancy. See the commit message of d0e797b for details.

Thomas Kluyver
Collaborator

Rather than integrating into AutoMagicChecker, would it be tidier to add a QuitterChecker to complement the QuitterHandler you've added?

David Warde-Farley
dwf commented

I don't know. Checkers and handlers don't necessarily have a 1:1 mapping, since both MagicChecker and AutoMagicChecker yield MagicHandlers, for example.

On the other hand, maybe quit(), etc. should work even with automagic off.

Thomas Kluyver
Collaborator

Mhm, but it seems to me that you're largely running a separate execution path through the same code, using is_quitter, so I think it would be neater to separate that out.

David Warde-Farley
dwf commented

I've refactored in the manner you suggested. QuitterChecker is now it's own separate thing.

David Warde-Farley dwf Add quitter checker and handler for quit(), etc. calls. Closes gh-142.
After some discussion with Fernando it was decided that detecting calls
on all magics as in pull request #352 was probably overkill. Opted for
detecting with a regex possible calls to absent Quitter objects and
rewriting them (as before), with visual notification of this rewrite.
This is done with a QuitterChecker which has priority just below that
of AutoMagicChecker.

The only case where you will get a NameError now is if you input
newlines before the starting and closing paren, but that's sufficiently
crazy as to not be worth considering (it takes one, optional,
integer argument in the real Python shell, why you'd ever need
newlines...).

Also subclassed MagicHandler to avoid doing the regex check twice (once
in the Checker and once in the Handler) and factored out the creation of
the replacement command so as to avoid potential code rot.
d0e797b
Thomas Kluyver
Collaborator

OK, thanks, that's neater. I've done a brief test, and I think it's an improvement. One more quick thing: can you add an automated test, in core/tests/test_handlers? It should be as simple as a single tuple.

David Warde-Farley dwf Tests for the quitter handler.
Also amended the tester to run within the builtin trap context manager
so that the builtins don't interfere.
efbb04f
David Warde-Farley
dwf commented

Done. It was a little more complicated, some of the tests were failing and it was because the builtin_trap wasn't being used for that prefilter.

Thomas Kluyver
Collaborator

Thanks, and well done for sorting that little issue with the tests. @fperez: I think this is ready to merge.

Fernando Perez
Owner

Hey, let's hold off for a minute... Last night I was thinking about this, and I have an idea. Give me an hour to see if it works, and if it doesn't we'll merge David's solution.

Thomas Kluyver
Collaborator

David, I'm sorry, I've encouraged you to jump through all these hoops, and now talking on IRC, Fernando and I have devised a simpler way to achieve the same thing, which I'm about to implement. But it wouldn't have happened at all without you starting to work on it, so thank-you for tackling the issue, and please do keep contributing.

Fernando Perez
Owner
David Warde-Farley
dwf commented
Fernando Perez
Owner

Sure, just see #357 for the new version.

Fernando Perez
Owner

Closing this one, since we'll go for the cleaner solution in #357.

Fernando Perez fperez closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 2 unique commits by 1 author.

Apr 10, 2011
David Warde-Farley dwf Add quitter checker and handler for quit(), etc. calls. Closes gh-142.
After some discussion with Fernando it was decided that detecting calls
on all magics as in pull request #352 was probably overkill. Opted for
detecting with a regex possible calls to absent Quitter objects and
rewriting them (as before), with visual notification of this rewrite.
This is done with a QuitterChecker which has priority just below that
of AutoMagicChecker.

The only case where you will get a NameError now is if you input
newlines before the starting and closing paren, but that's sufficiently
crazy as to not be worth considering (it takes one, optional,
integer argument in the real Python shell, why you'd ever need
newlines...).

Also subclassed MagicHandler to avoid doing the regex check twice (once
in the Checker and once in the Handler) and factored out the creation of
the replacement command so as to avoid potential code rot.
d0e797b
David Warde-Farley dwf Tests for the quitter handler.
Also amended the tester to run within the builtin trap context manager
so that the builtins don't interfere.
efbb04f
This page is out of date. Refresh to see the latest.
43 IPython/core/prefilter.py
@@ -702,6 +702,26 @@ def check(self, line_info):
702 702 return self.prefilter_manager.get_handler_by_name('magic')
703 703
704 704
  705 +class QuitterChecker(PrefilterChecker):
  706 +
  707 + priority = Int(701, config=True)
  708 +
  709 + quitter_pattern = re.compile('([qQ]uit|[eE]xit)\([^\)]*\)')
  710 +
  711 + def check(self, line_info):
  712 + # Pattern match against a small set of builtins that IPython removes
  713 + # related to quitting, in case user is call()ing them.
  714 + ifun = line_info.ifun
  715 + the_rest = line_info.the_rest
  716 + is_quitter = self.quitter_pattern.match(ifun + the_rest) is not None
  717 + if not is_quitter:
  718 + return None
  719 + head = ifun.split('(', 1)[0]
  720 + if is_shadowed(head, self.shell):
  721 + return None
  722 + else:
  723 + return self.prefilter_manager.get_handler_by_name('quitter')
  724 +
705 725 class AliasChecker(PrefilterChecker):
706 726
707 727 priority = Int(800, config=True)
@@ -867,13 +887,30 @@ class MagicHandler(PrefilterHandler):
867 887
868 888 def handle(self, line_info):
869 889 """Execute magic functions."""
870   - ifun = line_info.ifun
  890 + ifun = line_info.ifun
871 891 the_rest = line_info.the_rest
  892 + return self.magic_command(line_info, ifun + " " + the_rest)
  893 +
  894 + def magic_command(self, line_info, magic_str):
  895 + # So that we don't duplicate code in derived class, just in case
  896 + # this ever changes.
872 897 cmd = '%sget_ipython().magic(%s)' % (line_info.pre_whitespace,
873   - make_quoted_expr(ifun + " " + the_rest))
  898 + make_quoted_expr(magic_str))
874 899 return cmd
875 900
876 901
  902 +class QuitterHandler(MagicHandler):
  903 +
  904 + handler_name = Str('quitter')
  905 + esc_strings = List([])
  906 +
  907 + def handle(self, line_info):
  908 + """Execute magic function to quit."""
  909 + ifun = line_info.ifun.split('(', 1)[0]
  910 + self.shell.auto_rewrite_input(ESC_MAGIC + ifun)
  911 + return self.magic_command(line_info, ifun)
  912 +
  913 +
877 914 class AutoHandler(PrefilterHandler):
878 915
879 916 handler_name = Str('auto')
@@ -1008,6 +1045,7 @@ def handle(self, line_info):
1008 1045 EscCharsChecker,
1009 1046 AssignmentChecker,
1010 1047 AutoMagicChecker,
  1048 + QuitterChecker,
1011 1049 AliasChecker,
1012 1050 PythonOpsChecker,
1013 1051 AutocallChecker
@@ -1019,6 +1057,7 @@ def handle(self, line_info):
1019 1057 ShellEscapeHandler,
1020 1058 MacroHandler,
1021 1059 MagicHandler,
  1060 + QuitterHandler,
1022 1061 AutoHandler,
1023 1062 HelpHandler,
1024 1063 EmacsHandler
11 IPython/core/tests/test_handlers.py
@@ -42,8 +42,9 @@ def run(tests):
42 42 transformed (i.e. ipython's notion of _i)"""
43 43 for pre, post in tests:
44 44 global num_tests
45   - num_tests += 1
46   - actual = ip.prefilter_manager.prefilter_lines(pre)
  45 + num_tests += 1
  46 + with ip.shell.builtin_trap:
  47 + actual = ip.prefilter_manager.prefilter_lines(pre)
47 48 if actual != None:
48 49 actual = actual.rstrip('\n')
49 50 if actual != post:
@@ -169,4 +170,10 @@ def test_handlers():
169 170 ])
170 171 ip.magic('autocall 1')
171 172
  173 + for quit_str in ['quit', 'Quit', 'exit', 'Exit']:
  174 + run([
  175 + ("%s()" % quit_str, 'get_ipython().magic(u"%s")' % quit_str),
  176 + ("%s( )" % quit_str, 'get_ipython().magic(u"%s")' % quit_str),
  177 + ("%s(0)" % quit_str, 'get_ipython().magic(u"%s")' % quit_str),
  178 + ])
172 179 nt.assert_equals(failures, [])

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.