From d7c7a8f553188b6b211d7da82a957e1d8a2bc151 2012-06-07 23:17:09 From: Thomas Kluyver Date: 2012-06-07 23:17:09 Subject: [PATCH] Merge pull request #1850 from bfroehle/_1736_try_next Remove args/kwargs handling in TryNext, fix %paste error messages. --- diff --git a/IPython/core/error.py b/IPython/core/error.py index cf7d546..25ff887 100644 --- a/IPython/core/error.py +++ b/IPython/core/error.py @@ -35,22 +35,9 @@ class TryNext(IPythonCoreError): """Try next hook exception. Raise this in your hook function to indicate that the next hook handler - should be used to handle the operation. If you pass arguments to the - constructor those arguments will be used by the next hook instead of the - original ones. - - A _msg argument will not be passed on, so it can be used as a displayable - error message. + should be used to handle the operation. """ - def __init__(self, _msg="", *args, **kwargs): - self.args = args - self.kwargs = kwargs - self.msg = _msg - - def __str__(self): - return str(self.msg) - class UsageError(IPythonCoreError): """Error in magic function arguments, etc. diff --git a/IPython/core/hooks.py b/IPython/core/hooks.py index 81340d2..742393f 100644 --- a/IPython/core/hooks.py +++ b/IPython/core/hooks.py @@ -131,17 +131,15 @@ class CommandChainDispatcher: This will call all funcs in chain with the same args as were given to this function, and return the result of first func that didn't raise TryNext""" - + last_exc = TryNext() for prio,cmd in self.chain: #print "prio",prio,"cmd",cmd #dbg try: return cmd(*args, **kw) - except TryNext, exc: - if exc.args or exc.kwargs: - args = exc.args - kw = exc.kwargs + except TryNext as exc: + last_exc = exc # if no function will accept it, raise TryNext up to the caller - raise TryNext(*args, **kw) + raise last_exc def __str__(self): return str(self.chain) diff --git a/IPython/core/tests/test_hooks.py b/IPython/core/tests/test_hooks.py new file mode 100644 index 0000000..ec32bf5 --- /dev/null +++ b/IPython/core/tests/test_hooks.py @@ -0,0 +1,75 @@ +# -*- coding: utf-8 -*- +"""Tests for CommandChainDispatcher.""" + +from __future__ import absolute_import + +#----------------------------------------------------------------------------- +# Imports +#----------------------------------------------------------------------------- + +import nose.tools as nt +from IPython.core.error import TryNext +from IPython.core.hooks import CommandChainDispatcher + +#----------------------------------------------------------------------------- +# Local utilities +#----------------------------------------------------------------------------- + +# Define two classes, one which succeeds and one which raises TryNext. Each +# sets the attribute `called` to True when it is called. +class Okay(object): + def __init__(self, message): + self.message = message + self.called = False + def __call__(self): + self.called = True + return self.message + +class Fail(object): + def __init__(self, message): + self.message = message + self.called = False + def __call__(self): + self.called = True + raise TryNext(self.message) + +#----------------------------------------------------------------------------- +# Test functions +#----------------------------------------------------------------------------- + +def test_command_chain_dispatcher_ff(): + """Test two failing hooks""" + fail1 = Fail(u'fail1') + fail2 = Fail(u'fail2') + dp = CommandChainDispatcher([(0, fail1), + (10, fail2)]) + + try: + dp() + except TryNext as e: + nt.assert_equal(str(e), u'fail2') + else: + assert False, "Expected exception was not raised." + + nt.assert_true(fail1.called) + nt.assert_true(fail2.called) + +def test_command_chain_dispatcher_fofo(): + """Test a mixture of failing and succeeding hooks.""" + fail1 = Fail(u'fail1') + fail2 = Fail(u'fail2') + okay1 = Okay(u'okay1') + okay2 = Okay(u'okay2') + + dp = CommandChainDispatcher([(0, fail1), + # (5, okay1), # add this later + (10, fail2), + (15, okay2)]) + dp.add(okay1, 5) + + nt.assert_equal(dp(), u'okay1') + + nt.assert_true(fail1.called) + nt.assert_true(okay1.called) + nt.assert_false(fail2.called) + nt.assert_false(okay2.called) diff --git a/IPython/lib/clipboard.py b/IPython/lib/clipboard.py index 30974b3..deef0da 100644 --- a/IPython/lib/clipboard.py +++ b/IPython/lib/clipboard.py @@ -15,9 +15,8 @@ def win32_clipboard_get(): try: import win32clipboard except ImportError: - message = ("Getting text from the clipboard requires the pywin32 " - "extensions: http://sourceforge.net/projects/pywin32/") - raise TryNext(_msg=message) + raise TryNext("Getting text from the clipboard requires the pywin32 " + "extensions: http://sourceforge.net/projects/pywin32/") win32clipboard.OpenClipboard() text = win32clipboard.GetClipboardData(win32clipboard.CF_TEXT) # FIXME: convert \r\n to \n? @@ -44,9 +43,8 @@ def tkinter_clipboard_get(): try: import Tkinter except ImportError: - message = ("Getting text from the clipboard on this platform " - "requires Tkinter.") - raise TryNext(_msg=message) + raise TryNext("Getting text from the clipboard on this platform " + "requires Tkinter.") root = Tkinter.Tk() root.withdraw() text = root.clipboard_get() diff --git a/docs/source/whatsnew/development.txt b/docs/source/whatsnew/development.txt index b8f65c6..dc1d31b 100644 --- a/docs/source/whatsnew/development.txt +++ b/docs/source/whatsnew/development.txt @@ -25,3 +25,11 @@ Other new features :meth:`~IPython.core.interactiveshell.InteractiveShell.run_cell` to :meth:`~IPython.core.interactiveshell.InteractiveShell.run_ast_nodes` is now configurable. + +Backwards incompatible changes +------------------------------ + +* The exception :exc:`IPython.core.error.TryNext` previously accepted + arguments and keyword arguments to be passed to the next implementation + of the hook. This feature was removed as it made error message propagation + difficult and violated the principle of loose coupling.