Merge pull request #906 from python-cmd2/parsing_exception

Parsing exception

7 files changed, 81 insertions, 21 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c378f893..70b84bf8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 1.0.1 (TBD, 2020)
+* Bug Fixes
+    * Fixed issue where postcmd hooks were running after an `argparse` exception in a command.
+
 ## 1.0.0 (March 1, 2020)
 * Enhancements
     * The documentation at [cmd2.rftd.io](https://cmd2.readthedocs.io) received a major overhaul
diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index 4ab3df3f..44e02005 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -50,7 +50,7 @@ from . import utils
 from .argparse_custom import CompletionItem, DEFAULT_ARGUMENT_PARSER
 from .clipboard import can_clip, get_paste_buffer, write_to_paste_buffer
 from .decorators import with_argparser
-from .exceptions import EmbeddedConsoleExit, EmptyStatement
+from .exceptions import Cmd2ArgparseError, Cmd2ShlexError, EmbeddedConsoleExit, EmptyStatement
 from .history import History, HistoryItem
 from .parsing import StatementParser, Statement, Macro, MacroArg, shlex_split
 from .rl_utils import rl_type, RlType, rl_get_point, rl_set_prompt, vt100_support, rl_make_safe_prompt, rl_warning
@@ -1599,12 +1599,10 @@ class Cmd(cmd.Cmd):
         stop = False
         try:
             statement = self._input_line_to_statement(line)
-        except EmptyStatement:
+        except (EmptyStatement, Cmd2ShlexError) as ex:
+            if isinstance(ex, Cmd2ShlexError):
+                self.perror("Invalid syntax: {}".format(ex))
             return self._run_cmdfinalization_hooks(stop, None)
-        except ValueError as ex:
-            # If shlex.split failed on syntax, let user know what's going on
-            self.pexcept("Invalid syntax: {}".format(ex))
-            return stop
 
         # now that we have a statement, run it with all the hooks
         try:
@@ -1684,8 +1682,8 @@ class Cmd(cmd.Cmd):
                         # Stop saving command's stdout before command finalization hooks run
                         self.stdout.pause_storage = True
 
-        except EmptyStatement:
-            # don't do anything, but do allow command finalization hooks to run
+        except (Cmd2ArgparseError, EmptyStatement):
+            # Don't do anything, but do allow command finalization hooks to run
             pass
         except Exception as ex:
             self.pexcept(ex)
@@ -1744,6 +1742,8 @@ class Cmd(cmd.Cmd):
 
         :param line: the line being parsed
         :return: the completed Statement
+        :raises: Cmd2ShlexError if a shlex error occurs (e.g. No closing quotation)
+                 EmptyStatement when the resulting Statement is blank
         """
         while True:
             try:
@@ -1755,7 +1755,7 @@ class Cmd(cmd.Cmd):
                     # it's not a multiline command, but we parsed it ok
                     # so we are done
                     break
-            except ValueError:
+            except Cmd2ShlexError:
                 # we have unclosed quotation marks, lets parse only the command
                 # and see if it's a multiline
                 statement = self.statement_parser.parse_command_only(line)
@@ -1792,7 +1792,7 @@ class Cmd(cmd.Cmd):
                 self._at_continuation_prompt = False
 
         if not statement.command:
-            raise EmptyStatement()
+            raise EmptyStatement
         return statement
 
     def _input_line_to_statement(self, line: str) -> Statement:
@@ -1801,6 +1801,8 @@ class Cmd(cmd.Cmd):
 
         :param line: the line being parsed
         :return: parsed command line as a Statement
+        :raises: Cmd2ShlexError if a shlex error occurs (e.g. No closing quotation)
+                 EmptyStatement when the resulting Statement is blank
         """
         used_macros = []
         orig_line = None
@@ -1819,7 +1821,7 @@ class Cmd(cmd.Cmd):
                 used_macros.append(statement.command)
                 line = self._resolve_macro(statement)
                 if line is None:
-                    raise EmptyStatement()
+                    raise EmptyStatement
             else:
                 break
 
diff --git a/cmd2/decorators.py b/cmd2/decorators.py
index 2c78134c..deac4701 100644
--- a/cmd2/decorators.py
+++ b/cmd2/decorators.py
@@ -4,6 +4,7 @@ import argparse
 from typing import Callable, List, Optional, Union
 
 from . import constants
+from .exceptions import Cmd2ArgparseError
 from .parsing import Statement
 
 
@@ -144,7 +145,7 @@ def with_argparser_and_unknown_args(parser: argparse.ArgumentParser, *,
             try:
                 args, unknown = parser.parse_known_args(parsed_arglist, namespace)
             except SystemExit:
-                return
+                raise Cmd2ArgparseError
             else:
                 setattr(args, '__statement__', statement)
                 return func(cmd2_app, args, unknown)
@@ -216,7 +217,7 @@ def with_argparser(parser: argparse.ArgumentParser, *,
             try:
                 args = parser.parse_args(parsed_arglist, namespace)
             except SystemExit:
-                return
+                raise Cmd2ArgparseError
             else:
                 setattr(args, '__statement__', statement)
                 return func(cmd2_app, args)
diff --git a/cmd2/exceptions.py b/cmd2/exceptions.py
index 747e2368..15787177 100644
--- a/cmd2/exceptions.py
+++ b/cmd2/exceptions.py
@@ -2,6 +2,20 @@
 """Custom exceptions for cmd2.  These are NOT part of the public API and are intended for internal use only."""
 
 
+class Cmd2ArgparseError(Exception):
+    """
+    Custom exception class for when a command has an error parsing its arguments.
+    This can be raised by argparse decorators or the command functions themselves.
+    The main use of this exception is to tell cmd2 not to run Postcommand hooks.
+    """
+    pass
+
+
+class Cmd2ShlexError(Exception):
+    """Raised when shlex fails to parse a command line string in StatementParser"""
+    pass
+
+
 class EmbeddedConsoleExit(SystemExit):
     """Custom exception class for use with the py command."""
     pass
diff --git a/cmd2/parsing.py b/cmd2/parsing.py
index 078b1860..71582f1a 100755
--- a/cmd2/parsing.py
+++ b/cmd2/parsing.py
@@ -10,6 +10,7 @@ import attr
 
 from . import constants
 from . import utils
+from .exceptions import Cmd2ShlexError
 
 
 def shlex_split(str_to_split: str) -> List[str]:
@@ -330,7 +331,7 @@ class StatementParser:
 
         :param line: the command line being lexed
         :return: A list of tokens
-        :raises ValueError: if there are unclosed quotation marks
+        :raises: Cmd2ShlexError if a shlex error occurs (e.g. No closing quotation)
         """
 
         # expand shortcuts and aliases
@@ -341,7 +342,10 @@ class StatementParser:
             return []
 
         # split on whitespace
-        tokens = shlex_split(line)
+        try:
+            tokens = shlex_split(line)
+        except ValueError as ex:
+            raise Cmd2ShlexError(ex)
 
         # custom lexing
         tokens = self.split_on_punctuation(tokens)
@@ -355,7 +359,7 @@ class StatementParser:
 
         :param line: the command line being parsed
         :return: a new :class:`~cmd2.Statement` object
-        :raises ValueError: if there are unclosed quotation marks
+        :raises: Cmd2ShlexError if a shlex error occurs (e.g. No closing quotation)
         """
 
         # handle the special case/hardcoded terminator of a blank line
@@ -518,8 +522,6 @@ class StatementParser:
         :param rawinput: the command line as entered by the user
         :return: a new :class:`~cmd2.Statement` object
         """
-        line = rawinput
-
         # expand shortcuts and aliases
         line = self._expand(rawinput)
 
diff --git a/tests/test_parsing.py b/tests/test_parsing.py
index 435f22eb..5f363320 100755
--- a/tests/test_parsing.py
+++ b/tests/test_parsing.py
@@ -96,7 +96,7 @@ def test_tokenize(parser, line, tokens):
     assert tokens_to_test == tokens
 
 def test_tokenize_unclosed_quotes(parser):
-    with pytest.raises(ValueError):
+    with pytest.raises(exceptions.Cmd2ShlexError):
         _ = parser.tokenize('command with "unclosed quotes')
 
 @pytest.mark.parametrize('tokens,command,args', [
@@ -583,7 +583,7 @@ def test_parse_redirect_to_unicode_filename(parser):
     assert statement.output_to == 'café'
 
 def test_parse_unclosed_quotes(parser):
-    with pytest.raises(ValueError):
+    with pytest.raises(exceptions.Cmd2ShlexError):
         _ = parser.tokenize("command with 'unclosed quotes")
 
 def test_empty_statement_raises_exception():
diff --git a/tests/test_plugin.py b/tests/test_plugin.py
index c118b60d..bb7753f0 100644
--- a/tests/test_plugin.py
+++ b/tests/test_plugin.py
@@ -3,6 +3,7 @@
 """
 Test plugin infrastructure and hooks.
 """
+import argparse
 import sys
 
 import pytest
@@ -14,7 +15,7 @@ except ImportError:
     from unittest import mock
 
 import cmd2
-from cmd2 import exceptions, plugin
+from cmd2 import exceptions, plugin, Cmd2ArgumentParser, with_argparser
 
 
 class Plugin:
@@ -254,6 +255,14 @@ class PluggedApp(Plugin, cmd2.Cmd):
         """Repeat back the arguments"""
         self.poutput(statement)
 
+    parser = Cmd2ArgumentParser(description="Test parser")
+    parser.add_argument("my_arg", help="some help text")
+
+    @with_argparser(parser)
+    def do_argparse_cmd(self, namespace: argparse.Namespace):
+        """Repeat back the arguments"""
+        self.poutput(namespace.__statement__)
+
 ###
 #
 # test pre and postloop hooks
@@ -836,3 +845,31 @@ def test_cmdfinalization_hook_exception(capsys):
     assert out == 'hello\n'
     assert err
     assert app.called_cmdfinalization == 1
+
+
+def test_cmd2_argparse_exception(capsys):
+    """
+    Verify Cmd2ArgparseErrors raised after calling a command prevent postcmd events from
+    running but do not affect cmdfinalization events
+    """
+    app = PluggedApp()
+    app.register_postcmd_hook(app.postcmd_hook)
+    app.register_cmdfinalization_hook(app.cmdfinalization_hook)
+
+    # First generate no exception and make sure postcmd_hook, postcmd, and cmdfinalization_hook run
+    app.onecmd_plus_hooks('argparse_cmd arg_val')
+    out, err = capsys.readouterr()
+    assert out == 'arg_val\n'
+    assert not err
+    assert app.called_postcmd == 2
+    assert app.called_cmdfinalization == 1
+
+    app.reset_counters()
+
+    # Next cause an argparse exception and verify no postcmd stuff runs but cmdfinalization_hook still does
+    app.onecmd_plus_hooks('argparse_cmd')
+    out, err = capsys.readouterr()
+    assert not out
+    assert "Error: the following arguments are required: my_arg" in err
+    assert app.called_postcmd == 0
+    assert app.called_cmdfinalization == 1