Repository: python-cmd2/cmd2

Issue:
Postcommand Hooks and argparse
Hi

I am using argparse for autocompletion and with sub-commands. 
I always annotate my command methods with `@with_argparser(parser)` which i think is the right way to use the argparser with cmd2. 
I did experiment with postcommand hooks using the `register_postcmd_hook` method. 
The documentation states, that the registered postcommand is only executed when the command method was called (in case of a raised exception the postcommand it is not executed). 

Now, if argparse can't parse the submitted args (for example when there are too few), the command method is not executed, but the postcommand hooks will be executed anyway. 

I understand, that in this context, the `@with_argparser(parser)` belongs to the method and thus one could argument, the method was called and didn't raise an exception, but on the other side, in most cases (i think), it makes not a lot of sense to execute postcommands if argparse can't parse the arguments of the command method. 
Is this behaviour intended? And if yes, is there a possibility to change it? I know i could ditch the annotations, use the argparser inside the command method and raise an exception if argparse fails, but this seems to be a lot of overhead (and i would lose the autocomplete stuff).

Could i possibly "monkey patch" the `with_argparser` method so a exception will be raised? (Also not a very elegant solution i think)

Thanks for any help.



Discussion:

Patch:
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)
 


Test patch:
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