Merge pull request #882 from python-cmd2/misc

do_set fix

Author: kmvanbrunt

CHANGELOG.md

@@ -1,3 +1,8 @@
+## 0.10.1 (TBD)
+* Bug Fixes
+    * Corrected issue where the actual new value was not always being printed in do_set. This occurred in cases where
+      the typed value differed from what the setter had converted it to.
+
 ## 0.10.0 (February 7, 2020)
 * Enhancements
     * Changed the default help text to make `help -v` more discoverable

cmd2/cmd2.py

@@ -431,9 +431,8 @@ def allow_style(self, new_val: str) -> None:
         if new_val in [ansi.STYLE_TERMINAL, ansi.STYLE_ALWAYS, ansi.STYLE_NEVER]:
             ansi.allow_style = new_val
         else:
-            raise ValueError('Invalid value: {} (valid values: {}, {}, {})'.format(new_val, ansi.STYLE_TERMINAL,
-                                                                                   ansi.STYLE_ALWAYS,
-                                                                                   ansi.STYLE_NEVER))
+            raise ValueError("must be {}, {}, or {} (case-insensitive)".format(ansi.STYLE_TERMINAL, ansi.STYLE_ALWAYS,
+                                                                               ansi.STYLE_NEVER))
 
     def _completion_supported(self) -> bool:
         """Return whether tab completion is supported"""
@@ -2886,8 +2885,8 @@ def do_set(self, args: argparse.Namespace) -> None:
                 # Try to update the settable's value
                 try:
                     orig_value = getattr(self, args.param)
-                    new_value = settable.val_type(args.value)
-                    setattr(self, args.param, new_value)
+                    setattr(self, args.param, settable.val_type(args.value))
+                    new_value = getattr(self, args.param)
                 # noinspection PyBroadException
                 except Exception as e:
                     err_msg = "Error setting {}: {}".format(args.param, e)
@@ -3814,9 +3813,6 @@ def async_alert(self, alert_msg: str, new_prompt: Optional[str] = None) -> None:
         # Sanity check that can't fail if self.terminal_lock was acquired before calling this function
         if self.terminal_lock.acquire(blocking=False):
 
-            # Figure out what prompt is displaying
-            current_prompt = self.continuation_prompt if self._at_continuation_prompt else self.prompt
-
             # Only update terminal if there are changes
             update_terminal = False
 
@@ -3835,6 +3831,8 @@ def async_alert(self, alert_msg: str, new_prompt: Optional[str] = None) -> None:
 
             if update_terminal:
                 import shutil
+
+                current_prompt = self.continuation_prompt if self._at_continuation_prompt else self.prompt
                 terminal_str = ansi.async_alert_str(terminal_columns=shutil.get_terminal_size().columns,
                                                     prompt=current_prompt, line=readline.get_line_buffer(),
                                                     cursor_offset=rl_get_point(), alert_msg=alert_msg)
@@ -3867,9 +3865,9 @@ def async_update_prompt(self, new_prompt: str) -> None:  # pragma: no cover
                    a prompt is onscreen.  Therefore it is best to acquire the lock before calling this function
                    to guarantee the prompt changes.
 
-                   If a continuation prompt is currently being displayed while entering a multiline
-                   command, the onscreen prompt will not change. However self.prompt will still be updated
-                   and display immediately after the multiline line command completes.
+                   If user is at a continuation prompt while entering a multiline command, the onscreen prompt will
+                   not change. However self.prompt will still be updated and display immediately after the multiline
+                   line command completes.
 
         :param new_prompt: what to change the prompt to
         """

tests/test_cmd2.py

@@ -145,13 +145,13 @@ def test_set_no_settables(base_app):
 
 
 @pytest.mark.parametrize('new_val, is_valid, expected', [
-    (ansi.STYLE_NEVER, False, ansi.STYLE_NEVER),
-    ('neVeR', False, ansi.STYLE_NEVER),
-    (ansi.STYLE_TERMINAL, False, ansi.STYLE_TERMINAL),
-    ('TeRMInal', False, ansi.STYLE_TERMINAL),
-    (ansi.STYLE_ALWAYS, False, ansi.STYLE_ALWAYS),
-    ('AlWaYs', False, ansi.STYLE_ALWAYS),
-    ('invalid', True, ansi.STYLE_TERMINAL),
+    (ansi.STYLE_NEVER, True, ansi.STYLE_NEVER),
+    ('neVeR', True, ansi.STYLE_NEVER),
+    (ansi.STYLE_TERMINAL, True, ansi.STYLE_TERMINAL),
+    ('TeRMInal', True, ansi.STYLE_TERMINAL),
+    (ansi.STYLE_ALWAYS, True, ansi.STYLE_ALWAYS),
+    ('AlWaYs', True, ansi.STYLE_ALWAYS),
+    ('invalid', False, ansi.STYLE_TERMINAL),
 ])
 def test_set_allow_style(base_app, new_val, is_valid, expected):
     # Initialize allow_style for this test
@@ -161,14 +161,17 @@ def test_set_allow_style(base_app, new_val, is_valid, expected):
     out, err = run_cmd(base_app, 'set allow_style {}'.format(new_val))
 
     # Verify the results
-    assert bool(err) == is_valid
     assert ansi.allow_style == expected
+    if is_valid:
+        assert not err
+        assert "now: {!r}".format(new_val.capitalize()) in out[1]
 
     # Reload ansi module to reset allow_style 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):
         super().__init__(*args, **kwargs)