Added validation when setting allow_ansi

Author: kmvanbrunt

cmd2/cmd2.py

@@ -387,7 +387,10 @@ def __init__(self, completekey: str = 'tab', stdin=None, stdout=None, *,
         self.settable = \
             {
                 # allow_ansi is a special case in which it's an application-wide setting defined in ansi.py
-                'allow_ansi': 'Allow ANSI escape sequences in output (valid values: Terminal, Always, Never)',
+                'allow_ansi': ('Allow ANSI escape sequences in output '
+                               '(valid values: {}, {}, {})'.format(ansi.ANSI_TERMINAL,
+                                                                   ansi.ANSI_ALWAYS,
+                                                                   ansi.ANSI_NEVER)),
                 'continuation_prompt': 'On 2nd+ line of input',
                 'debug': 'Show full error stack on error',
                 'echo': 'Echo command issued into output',
@@ -561,7 +564,11 @@ def allow_ansi(self) -> str:
     @allow_ansi.setter
     def allow_ansi(self, new_val: str) -> None:
         """Read-only property needed to support do_set when it sets allow_ansi"""
-        ansi.allow_ansi = new_val
+        if new_val not in (ansi.ANSI_TERMINAL, ansi.ANSI_ALWAYS, ansi.ANSI_NEVER):
+            self.perror('Invalid value: {} (valid values: {}, {}, {})'.format(new_val, ansi.ANSI_TERMINAL,
+                                                                              ansi.ANSI_ALWAYS, ansi.ANSI_NEVER))
+        else:
+            ansi.allow_ansi = new_val
 
     @property
     def visible_prompt(self) -> str:
@@ -2953,17 +2960,20 @@ def do_set(self, args: argparse.Namespace) -> None:
                 return self._show(args, param)
 
         # Update the settable's value
-        current_value = getattr(self, param)
-        value = utils.cast(current_value, value)
-        setattr(self, param, value)
+        orig_value = getattr(self, param)
+        setattr(self, param, utils.cast(orig_value, value))
+
+        # In cases where a Python property is used to validate and update a settable's value, its value will not
+        # change if the passed in one is invalid. Therefore we should read its actual value back and not assume.
+        new_value = getattr(self, param)
 
-        self.poutput('{} - was: {}\nnow: {}'.format(param, current_value, value))
+        self.poutput('{} - was: {}\nnow: {}'.format(param, orig_value, new_value))
 
         # See if we need to call a change hook for this settable
-        if current_value != value:
+        if orig_value != new_value:
             onchange_hook = getattr(self, '_onchange_{}'.format(param), None)
             if onchange_hook is not None:
-                onchange_hook(old=current_value, new=value)
+                onchange_hook(old=orig_value, new=new_value)
 
     shell_parser = ACArgumentParser()
     setattr(shell_parser.add_argument('command', help='the command to run'),

tests/test_cmd2.py

@@ -181,6 +181,20 @@ def test_set_quiet(base_app):
     out, err = run_cmd(base_app, 'set quiet')
     assert out == ['quiet: True']
 
+@pytest.mark.parametrize('new_val, is_valid', [
+    (ansi.ANSI_NEVER, False),
+    (ansi.ANSI_TERMINAL, False),
+    (ansi.ANSI_ALWAYS, False),
+    ('invalid', True),
+])
+def test_set_allow_ansi(base_app, new_val, is_valid):
+    out, err = run_cmd(base_app, 'set allow_ansi {}'.format(new_val))
+    assert bool(err) == is_valid
+
+    # Reload ansi module to reset allow_ansi to its default since it's an
+    # application-wide setting that can affect other unit tests.
+    import importlib
+    importlib.reload(ansi)
 
 class OnChangeHookApp(cmd2.Cmd):
     def __init__(self, *args, **kwargs):