diff --git a/cliff/command.py b/cliff/command.py index 1c6ac48f..0d4fc732 100644 --- a/cliff/command.py +++ b/cliff/command.py @@ -133,9 +133,9 @@ class Command(object): Return the value returned by :meth:`take_action` or 0. """ - self._run_before_hooks(parsed_args) + parsed_args = self._run_before_hooks(parsed_args) return_code = self.take_action(parsed_args) or 0 - self._run_after_hooks(parsed_args, return_code) + return_code = self._run_after_hooks(parsed_args, return_code) return return_code def _run_before_hooks(self, parsed_args): @@ -149,7 +149,12 @@ class Command(object): hook processing behavior. """ for hook in self._hooks: - hook.obj.before(parsed_args) + ret = hook.obj.before(parsed_args) + # If the return is None do not change parsed_args, otherwise + # set up to pass it to the next hook + if ret is not None: + parsed_args = ret + return parsed_args def _run_after_hooks(self, parsed_args, return_code): """Calls after() method of the hooks. @@ -162,7 +167,12 @@ class Command(object): hook processing behavior. """ for hook in self._hooks: - hook.obj.after(parsed_args, return_code) + ret = hook.obj.after(parsed_args, return_code) + # If the return is None do not change return_code, otherwise + # set up to pass it to the next hook + if ret is not None: + return_code = ret + return return_code class _SmartHelpFormatter(_argparse.HelpFormatter): diff --git a/cliff/display.py b/cliff/display.py index 84c23ee6..82b71d61 100644 --- a/cliff/display.py +++ b/cliff/display.py @@ -110,10 +110,11 @@ class DisplayCommandBase(command.Command): return columns_to_include, selector def run(self, parsed_args): - self._run_before_hooks(parsed_args) + parsed_args = self._run_before_hooks(parsed_args) self.formatter = self._formatter_plugins[parsed_args.formatter].obj column_names, data = self.take_action(parsed_args) - self._run_after_hooks(parsed_args, (column_names, data)) + column_names, data = self._run_after_hooks(parsed_args, + (column_names, data)) self.produce_output(parsed_args, column_names, data) return 0 diff --git a/cliff/hooks.py b/cliff/hooks.py index 9af627c0..eee99a17 100644 --- a/cliff/hooks.py +++ b/cliff/hooks.py @@ -51,6 +51,7 @@ class CommandHook(object): :param parsed_args: The arguments to the command. :paramtype parsed_args: argparse.Namespace """ + return parsed_args @abc.abstractmethod def after(self, parsed_args, return_code): @@ -63,3 +64,4 @@ class CommandHook(object): :param return_code: The value returned from take_action(). :paramtype return_code: int """ + return return_code diff --git a/cliff/tests/test_command_hooks.py b/cliff/tests/test_command_hooks.py index 589f067c..b8c7c4d4 100644 --- a/cliff/tests/test_command_hooks.py +++ b/cliff/tests/test_command_hooks.py @@ -97,6 +97,75 @@ class TestHook(hooks.CommandHook): self._after_called = True +class TestChangeHook(hooks.CommandHook): + + _before_called = False + _after_called = False + + def get_parser(self, parser): + parser.add_argument('--added-by-hook') + return parser + + def get_epilog(self): + return 'hook epilog' + + def before(self, parsed_args): + self._before_called = True + parsed_args.added_by_hook = 'othervalue' + parsed_args.added_by_before = True + return parsed_args + + def after(self, parsed_args, return_code): + self._after_called = True + return 24 + + +class TestDisplayChangeHook(hooks.CommandHook): + + _before_called = False + _after_called = False + + def get_parser(self, parser): + parser.add_argument('--added-by-hook') + return parser + + def get_epilog(self): + return 'hook epilog' + + def before(self, parsed_args): + self._before_called = True + parsed_args.added_by_hook = 'othervalue' + parsed_args.added_by_before = True + return parsed_args + + def after(self, parsed_args, return_code): + self._after_called = True + return (('Name',), ('othervalue',)) + + +class TestListerChangeHook(hooks.CommandHook): + + _before_called = False + _after_called = False + + def get_parser(self, parser): + parser.add_argument('--added-by-hook') + return parser + + def get_epilog(self): + return 'hook epilog' + + def before(self, parsed_args): + self._before_called = True + parsed_args.added_by_hook = 'othervalue' + parsed_args.added_by_before = True + return parsed_args + + def after(self, parsed_args, return_code): + self._after_called = True + return (('Name',), [('othervalue',)]) + + class TestCommandLoadHooks(base.TestBase): def test_no_app_or_name(self): @@ -147,8 +216,54 @@ class TestHooks(base.TestBase): def test_after(self): self.assertFalse(self.hook._after_called) - self.cmd.run(None) + result = self.cmd.run(None) self.assertTrue(self.hook._after_called) + self.assertEqual(result, 42) + + +class TestChangeHooks(base.TestBase): + + def setUp(self): + super(TestChangeHooks, self).setUp() + self.app = make_app() + self.cmd = TestCommand(self.app, None, cmd_name='test') + self.hook = TestChangeHook(self.cmd) + self.mgr = extension.ExtensionManager.make_test_instance( + [extension.Extension( + 'parser-hook', + None, + None, + self.hook)], + ) + # Replace the auto-loaded hooks with our explicitly created + # manager. + self.cmd._hooks = self.mgr + + def test_get_parser(self): + parser = self.cmd.get_parser('test') + results = parser.parse_args(['--added-by-hook', 'value']) + self.assertEqual(results.added_by_hook, 'value') + + def test_get_epilog(self): + results = self.cmd.get_epilog() + self.assertIn('hook epilog', results) + + def test_before(self): + self.assertFalse(self.hook._before_called) + parser = self.cmd.get_parser('test') + results = parser.parse_args(['--added-by-hook', 'value']) + self.cmd.run(results) + self.assertTrue(self.hook._before_called) + self.assertEqual(results.added_by_hook, 'othervalue') + self.assertTrue(results.added_by_before) + + def test_after(self): + self.assertFalse(self.hook._after_called) + parser = self.cmd.get_parser('test') + results = parser.parse_args(['--added-by-hook', 'value']) + result = self.cmd.run(results) + self.assertTrue(self.hook._after_called) + self.assertEqual(result, 24) class TestShowOneHooks(base.TestBase): @@ -193,6 +308,51 @@ class TestShowOneHooks(base.TestBase): self.assertTrue(self.hook._after_called) +class TestShowOneChangeHooks(base.TestBase): + + def setUp(self): + super(TestShowOneChangeHooks, self).setUp() + self.app = make_app() + self.cmd = TestShowCommand(self.app, None, cmd_name='test') + self.hook = TestDisplayChangeHook(self.cmd) + self.mgr = extension.ExtensionManager.make_test_instance( + [extension.Extension( + 'parser-hook', + None, + None, + self.hook)], + ) + # Replace the auto-loaded hooks with our explicitly created + # manager. + self.cmd._hooks = self.mgr + + def test_get_parser(self): + parser = self.cmd.get_parser('test') + results = parser.parse_args(['--added-by-hook', 'value']) + self.assertEqual(results.added_by_hook, 'value') + + def test_get_epilog(self): + results = self.cmd.get_epilog() + self.assertIn('hook epilog', results) + + def test_before(self): + self.assertFalse(self.hook._before_called) + parser = self.cmd.get_parser('test') + results = parser.parse_args(['--added-by-hook', 'value']) + self.cmd.run(results) + self.assertTrue(self.hook._before_called) + self.assertEqual(results.added_by_hook, 'othervalue') + self.assertTrue(results.added_by_before) + + def test_after(self): + self.assertFalse(self.hook._after_called) + parser = self.cmd.get_parser('test') + results = parser.parse_args(['--added-by-hook', 'value']) + result = self.cmd.run(results) + self.assertTrue(self.hook._after_called) + self.assertEqual(result, 0) + + class TestListerHooks(base.TestBase): def setUp(self): @@ -233,3 +393,48 @@ class TestListerHooks(base.TestBase): results = parser.parse_args(['--added-by-hook', 'value']) self.cmd.run(results) self.assertTrue(self.hook._after_called) + + +class TestListerChangeHooks(base.TestBase): + + def setUp(self): + super(TestListerChangeHooks, self).setUp() + self.app = make_app() + self.cmd = TestListerCommand(self.app, None, cmd_name='test') + self.hook = TestListerChangeHook(self.cmd) + self.mgr = extension.ExtensionManager.make_test_instance( + [extension.Extension( + 'parser-hook', + None, + None, + self.hook)], + ) + # Replace the auto-loaded hooks with our explicitly created + # manager. + self.cmd._hooks = self.mgr + + def test_get_parser(self): + parser = self.cmd.get_parser('test') + results = parser.parse_args(['--added-by-hook', 'value']) + self.assertEqual(results.added_by_hook, 'value') + + def test_get_epilog(self): + results = self.cmd.get_epilog() + self.assertIn('hook epilog', results) + + def test_before(self): + self.assertFalse(self.hook._before_called) + parser = self.cmd.get_parser('test') + results = parser.parse_args(['--added-by-hook', 'value']) + self.cmd.run(results) + self.assertTrue(self.hook._before_called) + self.assertEqual(results.added_by_hook, 'othervalue') + self.assertTrue(results.added_by_before) + + def test_after(self): + self.assertFalse(self.hook._after_called) + parser = self.cmd.get_parser('test') + results = parser.parse_args(['--added-by-hook', 'value']) + result = self.cmd.run(results) + self.assertTrue(self.hook._after_called) + self.assertEqual(result, 0)