Repository: python-cmd2/cmd2

Issue:
set command should support tab-completion of setting names
It would be helpful if the **set** command support tab-completion of setting names.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8d49d34e..6bf3c961 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,8 @@
         * Enables applications to return a non-zero exit code when exiting from ``cmdloop``
     * ``ACHelpFormatter`` now inherits from ``argparse.RawTextHelpFormatter`` to make it easier
     for formatting help/description text
+    * Aliases are now sorted alphabetically
+    * The **set** command now tab-completes settable parameter names
     
 ## 0.9.4 (August 21, 2018)
 * Bug Fixes
diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index 49ac91a5..8b1cabc9 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -45,7 +45,7 @@ from typing import Any, Callable, Dict, List, Mapping, Optional, Tuple, Type, Un
 from . import constants
 from . import utils
 from . import plugin
-from .argparse_completer import AutoCompleter, ACArgumentParser
+from .argparse_completer import AutoCompleter, ACArgumentParser, ACTION_ARG_CHOICES
 from .clipboard import can_clip, get_paste_buffer, write_to_paste_buffer
 from .parsing import StatementParser, Statement
 
@@ -2284,6 +2284,9 @@ Usage:  Usage: alias [name] | [<name> <value>]
                 # Set the alias
                 self.aliases[name] = value
                 self.poutput("Alias {!r} created".format(name))
+
+                # Keep aliases in alphabetically sorted order
+                self.aliases = collections.OrderedDict(sorted(self.aliases.items()))
             else:
                 errmsg = "Aliases can not contain: {}".format(invalidchars)
                 self.perror(errmsg, traceback_war=False)
@@ -2541,22 +2544,21 @@ Usage:  Usage: unalias [-a] name [name ...]
         Output redirection and pipes allowed: {}"""
         return read_only_settings.format(str(self.terminators), self.allow_cli_args, self.allow_redirection)
 
-    def show(self, args: argparse.Namespace, parameter: str) -> None:
+    def show(self, args: argparse.Namespace, parameter: str='') -> None:
         """Shows current settings of parameters.
 
         :param args: argparse parsed arguments from the set command
-        :param parameter:
-        :return:
+        :param parameter: optional search parameter
         """
-        param = ''
-        if parameter:
-            param = parameter.strip().lower()
+        param = parameter.strip().lower()
         result = {}
         maxlen = 0
+
         for p in self.settable:
             if (not param) or p.startswith(param):
-                result[p] = '%s: %s' % (p, str(getattr(self, p)))
+                result[p] = '{}: {}'.format(p, str(getattr(self, p)))
                 maxlen = max(maxlen, len(result[p]))
+
         if result:
             for p in sorted(result):
                 if args.long:
@@ -2568,7 +2570,7 @@ Usage:  Usage: unalias [-a] name [name ...]
             if args.all:
                 self.poutput('\nRead only settings:{}'.format(self.cmdenvironment()))
         else:
-            raise LookupError("Parameter '%s' not supported (type 'set' for list of parameters)." % param)
+            raise LookupError("Parameter '{}' not supported (type 'set' for list of parameters).".format(param))
 
     set_description = "Sets a settable parameter or shows current settings of parameters.\n"
     set_description += "\n"
@@ -2578,39 +2580,44 @@ Usage:  Usage: unalias [-a] name [name ...]
     set_parser = ACArgumentParser(description=set_description)
     set_parser.add_argument('-a', '--all', action='store_true', help='display read-only settings as well')
     set_parser.add_argument('-l', '--long', action='store_true', help='describe function of parameter')
-    set_parser.add_argument('settable', nargs=(0, 2), help='[param_name] [value]')
+    setattr(set_parser.add_argument('param', nargs='?', help='parameter to set or view'),
+            ACTION_ARG_CHOICES, settable)
+    set_parser.add_argument('value', nargs='?', help='the new value for settable')
 
     @with_argparser(set_parser)
     def do_set(self, args: argparse.Namespace) -> None:
         """Sets a settable parameter or shows current settings of parameters"""
-        try:
-            param_name, val = args.settable
-            val = val.strip()
-            param_name = param_name.strip().lower()
-            if param_name not in self.settable:
-                hits = [p for p in self.settable if p.startswith(param_name)]
-                if len(hits) == 1:
-                    param_name = hits[0]
-                else:
-                    return self.show(args, param_name)
-            current_val = getattr(self, param_name)
-            if (val[0] == val[-1]) and val[0] in ("'", '"'):
-                val = val[1:-1]
+
+        # Check if param was passed in
+        if not args.param:
+            return self.show(args)
+        param = args.param.strip().lower()
+
+        # Check if value was passed in
+        if not args.value:
+            return self.show(args, param)
+        value = args.value
+
+        # Check if param points to just one settable
+        if param not in self.settable:
+            hits = [p for p in self.settable if p.startswith(param)]
+            if len(hits) == 1:
+                param = hits[0]
             else:
-                val = utils.cast(current_val, val)
-            setattr(self, param_name, val)
-            self.poutput('%s - was: %s\nnow: %s\n' % (param_name, current_val, val))
-            if current_val != val:
-                try:
-                    onchange_hook = getattr(self, '_onchange_%s' % param_name)
-                    onchange_hook(old=current_val, new=val)
-                except AttributeError:
-                    pass
-        except (ValueError, AttributeError):
-            param = ''
-            if args.settable:
-                param = args.settable[0]
-            self.show(args, param)
+                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)
+
+        self.poutput('{} - was: {}\nnow: {}\n'.format(param, current_value, value))
+
+        # See if we need to call a change hook for this settable
+        if current_value != value:
+            onchange_hook = getattr(self, '_onchange_{}'.format(param), None)
+            if onchange_hook is not None:
+                onchange_hook(old=current_value, new=value)
 
     def do_shell(self, statement: Statement) -> None:
         """Execute a command as if at the OS prompt
diff --git a/cmd2/utils.py b/cmd2/utils.py
index 64401895..9d71d061 100644
--- a/cmd2/utils.py
+++ b/cmd2/utils.py
@@ -20,6 +20,28 @@ def strip_ansi(text: str) -> str:
     return constants.ANSI_ESCAPE_RE.sub('', text)
 
 
+def is_quoted(arg: str) -> bool:
+    """
+    Checks if a string is quoted
+    :param arg: the string being checked for quotes
+    :return: True if a string is quoted
+    """
+    return len(arg) > 1 and arg[0] == arg[-1] and arg[0] in constants.QUOTES
+
+
+def quote_string_if_needed(arg: str) -> str:
+    """ Quotes a string if it contains spaces and isn't already quoted """
+    if is_quoted(arg) or ' ' not in arg:
+        return arg
+
+    if '"' in arg:
+        quote = "'"
+    else:
+        quote = '"'
+
+    return quote + arg + quote
+
+
 def strip_quotes(arg: str) -> str:
     """ Strip outer quotes from a string.
 
@@ -28,7 +50,7 @@ def strip_quotes(arg: str) -> str:
     :param arg:  string to strip outer quotes from
     :return: same string with potentially outer quotes stripped
     """
-    if len(arg) > 1 and arg[0] == arg[-1] and arg[0] in constants.QUOTES:
+    if is_quoted(arg):
         arg = arg[1:-1]
     return arg
 
@@ -71,6 +93,8 @@ def cast(current: Any, new: str) -> Any:
     :return: new value with same type as current, or the current value if there was an error casting
     """
     typ = type(current)
+    orig_new = new
+
     if typ == bool:
         try:
             return bool(int(new))
@@ -78,18 +102,18 @@ def cast(current: Any, new: str) -> Any:
             pass
         try:
             new = new.lower()
+            if (new == 'on') or (new[0] in ('y', 't')):
+                return True
+            if (new == 'off') or (new[0] in ('n', 'f')):
+                return False
         except AttributeError:
             pass
-        if (new == 'on') or (new[0] in ('y', 't')):
-            return True
-        if (new == 'off') or (new[0] in ('n', 'f')):
-            return False
     else:
         try:
             return typ(new)
         except (ValueError, TypeError):
             pass
-    print("Problem setting parameter (now %s) to %s; incorrect type?" % (current, new))
+    print("Problem setting parameter (now {}) to {}; incorrect type?".format(current, orig_new))
     return current
 
 


Test patch:
diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py
index fdf0f661..1e7e2c3f 100644
--- a/tests/test_cmd2.py
+++ b/tests/test_cmd2.py
@@ -72,7 +72,7 @@ def test_base_invalid_option(base_app, capsys):
     out = normalize(out)
     err = normalize(err)
     assert 'Error: unrecognized arguments: -z' in err[0]
-    assert out[0] == 'Usage: set settable{0..2} [-h] [-a] [-l]'
+    assert out[0] == 'Usage: set [param] [value] [-h] [-a] [-l]'
 
 def test_base_shortcuts(base_app):
     out = run_cmd(base_app, 'shortcuts')
@@ -1804,6 +1804,24 @@ def test_create_invalid_alias(base_app, alias_name, capsys):
     out, err = capsys.readouterr()
     assert "can not contain" in err
 
+def test_complete_unalias(base_app):
+    text = 'f'
+    line = text
+    endidx = len(line)
+    begidx = endidx - len(text)
+
+    # Validate there are no completions when there are no aliases
+    assert base_app.complete_unalias(text, line, begidx, endidx) == []
+
+    # Create a few aliases - two the start with 'f' and one that doesn't
+    run_cmd(base_app, 'alias fall quit')
+    run_cmd(base_app, 'alias fake pyscript')
+    run_cmd(base_app, 'alias carapace shell')
+
+    # Validate that there are now completions
+    expected = ['fake', 'fall']
+    assert base_app.complete_unalias(text, line, begidx, endidx) == expected
+
 def test_ppaged(base_app):
     msg = 'testing...'
     end = '\n'
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 691abdf8..8c8daa39 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -76,3 +76,38 @@ def test_natural_sort():
     assert cu.natural_sort(my_list) == ['A', 'café', 'micro', 'unity', 'X0', 'x1', 'X2', 'X11', 'x22', 'µ']
     my_list = ['a3', 'a22', 'A2', 'A11', 'a1']
     assert cu.natural_sort(my_list) == ['a1', 'A2', 'a3', 'A11', 'a22']
+
+def test_is_quoted_short():
+    my_str = ''
+    assert not cu.is_quoted(my_str)
+    your_str = '"'
+    assert not cu.is_quoted(your_str)
+
+def test_is_quoted_yes():
+    my_str = '"This is a test"'
+    assert cu.is_quoted(my_str)
+    your_str = "'of the emergengy broadcast system'"
+    assert cu.is_quoted(your_str)
+
+def test_is_quoted_no():
+    my_str = '"This is a test'
+    assert not cu.is_quoted(my_str)
+    your_str = "of the emergengy broadcast system'"
+    assert not cu.is_quoted(your_str)
+    simple_str = "hello world"
+    assert not cu.is_quoted(simple_str)
+
+def test_quote_string_if_needed_yes():
+    my_str = "Hello World"
+    assert cu.quote_string_if_needed(my_str) == '"' + my_str + '"'
+    your_str = '"foo" bar'
+    assert cu.quote_string_if_needed(your_str) == "'" + your_str + "'"
+
+def test_quot_string_if_needed_no():
+    my_str = "HelloWorld"
+    assert cu.quote_string_if_needed(my_str) == my_str
+    your_str = "'Hello World'"
+    assert cu.quote_string_if_needed(your_str) == your_str
+
+
+