From c8381601e4e3b31969ea6108617356572a7d1ca3 Mon Sep 17 00:00:00 2001 From: Todd Leonhardt Date: Sat, 23 Jun 2018 19:43:42 -0400 Subject: Deprecated CmdResult helper class and promoted CommandResult These classes are subtly different, particularly in terms of their truthiness. CmdResult - attributes: out, err, war - truthy: if err is falsy CommandResult - attributes: stdout, stderr, data - truthy: if err is falsy AND data is not None So CmdResult was oriented to provide essentially info, error, and warning messages to the user (typically as stirngs), whereas CommandResult is geared towards providing info and error messages to the user as strings in addition to data to the user in a command-specific format which is arbitrary other than it should never be None if the command succeeds. --- CHANGELOG.md | 5 +++++ cmd2/__init__.py | 3 ++- cmd2/cmd2.py | 2 +- cmd2/pyscript_bridge.py | 2 +- docs/argument_processing.rst | 2 +- examples/python_scripting.py | 12 +++++++----- examples/scripts/conditional.py | 1 + tests/test_cmd2.py | 25 +++++++++++++------------ 8 files changed, 31 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 078af8d8..0acaf6df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ * Added ``chop`` argument to ``cmd2.Cmd.ppaged()`` method for displaying output using a pager * If ``chop`` is ``False``, then ``self.pager`` is used as the pager * Otherwise ``self.pager_chop`` is used as the pager +* Deprecations + * The ``CmdResult`` helper class is *deprecated* and replaced by the improved ``CommandResult`` class + * ``CommandResult`` has the following attributes: **stdout**, **stderr**, and **data** + * ``CmdResult`` had attributes of: **out**, **err**, **war** + * ``CmdResult`` will be deleted in the next release ## 0.8.8 (TBD, 2018) * Bug Fixes diff --git a/cmd2/__init__.py b/cmd2/__init__.py index e9a82acb..f61b7165 100644 --- a/cmd2/__init__.py +++ b/cmd2/__init__.py @@ -1,5 +1,6 @@ # # -*- coding: utf-8 -*- """This simply imports certain things for backwards compatibility.""" -from .cmd2 import __version__, Cmd, CmdResult, Statement, EmptyStatement, categorize +from .cmd2 import __version__, Cmd, Statement, EmptyStatement, categorize from .cmd2 import with_argument_list, with_argparser, with_argparser_and_unknown_args, with_category +from .pyscript_bridge import CommandResult \ No newline at end of file diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index e2fd25fa..3fb5fbda 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -3219,7 +3219,7 @@ class Statekeeper(object): class CmdResult(utils.namedtuple_with_two_defaults('CmdResult', ['out', 'err', 'war'])): - """Derive a class to store results from a named tuple so we can tweak dunder methods for convenience. + """DEPRECATED: Derive a class to store results from a named tuple so we can tweak dunder methods for convenience. This is provided as a convenience and an example for one possible way for end users to store results in the self._last_result attribute of cmd2.Cmd class instances. See the "python_scripting.py" example for how it can diff --git a/cmd2/pyscript_bridge.py b/cmd2/pyscript_bridge.py index 9353e611..3f58ab84 100644 --- a/cmd2/pyscript_bridge.py +++ b/cmd2/pyscript_bridge.py @@ -22,7 +22,7 @@ from .argparse_completer import _RangeAction from .utils import namedtuple_with_defaults -class CommandResult(namedtuple_with_defaults('CmdResult', ['stdout', 'stderr', 'data'])): +class CommandResult(namedtuple_with_defaults('CommandResult', ['stdout', 'stderr', 'data'])): """Encapsulates the results from a command. Named tuple attributes diff --git a/docs/argument_processing.rst b/docs/argument_processing.rst index ecf59504..5aef3720 100644 --- a/docs/argument_processing.rst +++ b/docs/argument_processing.rst @@ -333,7 +333,7 @@ Here's what it looks like:: if unknown: self.perror("dir does not take any positional arguments:", traceback_war=False) self.do_help('dir') - self._last_result = CmdResult('', 'Bad arguments') + self._last_result = CommandResult('', 'Bad arguments') return # Get the contents as a list diff --git a/examples/python_scripting.py b/examples/python_scripting.py index fd2d7e8f..069bcff5 100755 --- a/examples/python_scripting.py +++ b/examples/python_scripting.py @@ -56,7 +56,7 @@ class CmdLineApp(cmd2.Cmd): if not arglist or len(arglist) != 1: self.perror("cd requires exactly 1 argument:", traceback_war=False) self.do_help('cd') - self._last_result = cmd2.CmdResult('', 'Bad arguments') + self._last_result = cmd2.CommandResult('', 'Bad arguments') return # Convert relative paths to absolute paths @@ -64,7 +64,8 @@ class CmdLineApp(cmd2.Cmd): # Make sure the directory exists, is a directory, and we have read access out = '' - err = '' + err = None + data = None if not os.path.isdir(path): err = '{!r} is not a directory'.format(path) elif not os.access(path, os.R_OK): @@ -77,10 +78,11 @@ class CmdLineApp(cmd2.Cmd): else: out = 'Successfully changed directory to {!r}\n'.format(path) self.stdout.write(out) + data = path if err: self.perror(err, traceback_war=False) - self._last_result = cmd2.CmdResult(out, err) + self._last_result = cmd2.CommandResult(out, err, data) # Enable tab completion for cd command def complete_cd(self, text, line, begidx, endidx): @@ -96,7 +98,7 @@ class CmdLineApp(cmd2.Cmd): if unknown: self.perror("dir does not take any positional arguments:", traceback_war=False) self.do_help('dir') - self._last_result = cmd2.CmdResult('', 'Bad arguments') + self._last_result = cmd2.CommandResult('', 'Bad arguments') return # Get the contents as a list @@ -109,7 +111,7 @@ class CmdLineApp(cmd2.Cmd): self.stdout.write(fmt.format(f)) self.stdout.write('\n') - self._last_result = cmd2.CmdResult(contents) + self._last_result = cmd2.CommandResult(data=contents) if __name__ == '__main__': diff --git a/examples/scripts/conditional.py b/examples/scripts/conditional.py index 87cd10ac..d7ee5ea2 100644 --- a/examples/scripts/conditional.py +++ b/examples/scripts/conditional.py @@ -30,6 +30,7 @@ app('cd {}'.format(directory)) if self._last_result: print('\nContents of directory {!r}:'.format(directory)) app('dir -l') + print('{}\n'.format(self._last_result.data)) # Change back to where we were print('Changing back to original directory: {!r}'.format(original_dir)) diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py index 26df4807..94cd42f9 100644 --- a/tests/test_cmd2.py +++ b/tests/test_cmd2.py @@ -1468,32 +1468,33 @@ def test_clipboard_failure(base_app, capsys): assert "Cannot redirect to paste buffer; install 'pyperclip' and re-run to enable" in err -class CmdResultApp(cmd2.Cmd): +class CommandResultApp(cmd2.Cmd): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) def do_affirmative(self, arg): - self._last_result = cmd2.CmdResult(arg) + self._last_result = cmd2.CommandResult(arg, data=True) def do_negative(self, arg): - self._last_result = cmd2.CmdResult('', arg) + self._last_result = cmd2.CommandResult(arg) @pytest.fixture -def cmdresult_app(): - app = CmdResultApp() +def commandresult_app(): + app = CommandResultApp() app.stdout = StdOut() return app -def test_cmdresult(cmdresult_app): +def test_commandresult_truthy(commandresult_app): arg = 'foo' - run_cmd(cmdresult_app, 'affirmative {}'.format(arg)) - assert cmdresult_app._last_result - assert cmdresult_app._last_result == cmd2.CmdResult(arg) + run_cmd(commandresult_app, 'affirmative {}'.format(arg)) + assert commandresult_app._last_result + assert commandresult_app._last_result == cmd2.CommandResult(arg, data=True) +def test_commandresult_falsy(commandresult_app): arg = 'bar' - run_cmd(cmdresult_app, 'negative {}'.format(arg)) - assert not cmdresult_app._last_result - assert cmdresult_app._last_result == cmd2.CmdResult('', arg) + run_cmd(commandresult_app, 'negative {}'.format(arg)) + assert not commandresult_app._last_result + assert commandresult_app._last_result == cmd2.CommandResult(arg) def test_is_text_file_bad_input(base_app): -- cgit v1.2.1