More unit tests for aliases and macros

3 files changed, 64 insertions, 67 deletions

diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index 4fceab87..84b01a06 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -2244,24 +2244,20 @@ class Cmd(cmd.Cmd):
 
     def alias_create(self, args: argparse.Namespace):
         """ Creates or overwrites an alias """
+
         # Validate the alias name
-        valid, errmsg = self.statement_parser.is_valid_command(args.name, allow_shortcut=False)
+        alias_name = utils.strip_quotes(args.name)
+        valid, errmsg = self.statement_parser.is_valid_command(alias_name)
         if not valid:
             errmsg = "Invalid alias name: {}".format(errmsg)
             self.perror(errmsg, traceback_war=False)
             return
 
-        if args.name in self.macros:
+        if alias_name in self.macros:
             errmsg = "Aliases cannot have the same name as a macro"
             self.perror(errmsg, traceback_war=False)
             return
 
-        valid, errmsg = self.statement_parser.is_valid_command(args.command, allow_shortcut=True)
-        if not valid:
-            errmsg = "Invalid alias target:  {}".format(errmsg)
-            self.perror(errmsg, traceback_war=False)
-            return
-
         utils.unquote_redirection_tokens(args.command_args)
 
         # Build the alias value string
@@ -2270,9 +2266,9 @@ class Cmd(cmd.Cmd):
             value += ' ' + cur_arg
 
         # Set the alias
-        result = "overwritten" if args.name in self.aliases else "created"
-        self.aliases[args.name] = value
-        self.poutput("Alias {!r} {}".format(args.name, result))
+        result = "overwritten" if alias_name in self.aliases else "created"
+        self.aliases[alias_name] = value
+        self.poutput("Alias {!r} {}".format(alias_name, result))
 
     def alias_delete(self, args: argparse.Namespace):
         """ Deletes aliases """
@@ -2285,22 +2281,24 @@ class Cmd(cmd.Cmd):
             # Get rid of duplicates
             aliases_to_delete = utils.remove_duplicates(args.name)
 
-            for cur_arg in aliases_to_delete:
-                if cur_arg in self.aliases:
-                    del self.aliases[cur_arg]
-                    self.poutput("Alias {!r} deleted".format(cur_arg))
+            for cur_name in aliases_to_delete:
+                stripped_name = utils.strip_quotes(cur_name)
+                if stripped_name in self.aliases:
+                    del self.aliases[stripped_name]
+                    self.poutput("Alias {!r} deleted".format(stripped_name))
                 else:
-                    self.perror("Alias {!r} does not exist".format(cur_arg), traceback_war=False)
+                    self.perror("Alias {!r} does not exist".format(stripped_name), traceback_war=False)
 
     def alias_list(self, args: argparse.Namespace):
         """ Lists some or all aliases """
         if args.name:
             names_to_view = utils.remove_duplicates(args.name)
             for cur_name in names_to_view:
-                if cur_name in self.aliases:
-                    self.poutput("alias create {} {}".format(cur_name, self.aliases[cur_name]))
+                stripped_name = utils.strip_quotes(cur_name)
+                if stripped_name in self.aliases:
+                    self.poutput("alias create {} {}".format(stripped_name, self.aliases[stripped_name]))
                 else:
-                    self.perror("Alias {!r} not found".format(cur_name), traceback_war=False)
+                    self.perror("Alias {!r} not found".format(stripped_name), traceback_war=False)
         else:
             sorted_aliases = utils.alphabetical_sort(self.aliases)
             for cur_alias in sorted_aliases:
@@ -2377,35 +2375,31 @@ class Cmd(cmd.Cmd):
             func(self, args)
         else:
             # No subcommand was provided, so call help
-            self.alias_parser.print_help()
+            self.do_help('alias')
 
     # -----  Macro subcommand functions -----
 
     def macro_create(self, args: argparse.Namespace):
         """ Creates or overwrites a macro """
+
         # Validate the macro name
-        valid, errmsg = self.statement_parser.is_valid_command(args.name, allow_shortcut=False)
+        macro_name = utils.strip_quotes(args.name)
+        valid, errmsg = self.statement_parser.is_valid_command(macro_name)
         if not valid:
             errmsg = "Invalid macro name: {}".format(errmsg)
             self.perror(errmsg, traceback_war=False)
             return
 
-        if args.name in self.get_all_commands():
+        if macro_name in self.get_all_commands():
             errmsg = "Macros cannot have the same name as a command"
             self.perror(errmsg, traceback_war=False)
             return
 
-        if args.name in self.aliases:
+        if macro_name in self.aliases:
             errmsg = "Macros cannot have the same name as an alias"
             self.perror(errmsg, traceback_war=False)
             return
 
-        valid, errmsg = self.statement_parser.is_valid_command(args.command, allow_shortcut=True)
-        if not valid:
-            errmsg = "Invalid macro target:  {}".format(errmsg)
-            self.perror(errmsg, traceback_war=False)
-            return
-
         utils.unquote_redirection_tokens(args.command_args)
 
         # Build the macro value string
@@ -2459,9 +2453,9 @@ class Cmd(cmd.Cmd):
                 break
 
         # Set the macro
-        result = "overwritten" if args.name in self.macros else "created"
-        self.macros[args.name] = Macro(name=args.name, value=value, required_arg_count=max_arg_num, arg_list=arg_list)
-        self.poutput("Macro {!r} {}".format(args.name, result))
+        result = "overwritten" if macro_name in self.macros else "created"
+        self.macros[macro_name] = Macro(name=macro_name, value=value, required_arg_count=max_arg_num, arg_list=arg_list)
+        self.poutput("Macro {!r} {}".format(macro_name, result))
 
     def macro_delete(self, args: argparse.Namespace):
         """ Deletes macros """
@@ -2474,22 +2468,24 @@ class Cmd(cmd.Cmd):
             # Get rid of duplicates
             macros_to_delete = utils.remove_duplicates(args.name)
 
-            for cur_arg in macros_to_delete:
-                if cur_arg in self.macros:
-                    del self.macros[cur_arg]
-                    self.poutput("Macro {!r} deleted".format(cur_arg))
+            for cur_name in macros_to_delete:
+                stripped_name = utils.strip_quotes(cur_name)
+                if stripped_name in self.macros:
+                    del self.macros[stripped_name]
+                    self.poutput("Macro {!r} deleted".format(stripped_name))
                 else:
-                    self.perror("Macro {!r} does not exist".format(cur_arg), traceback_war=False)
+                    self.perror("Macro {!r} does not exist".format(stripped_name), traceback_war=False)
 
     def macro_list(self, args: argparse.Namespace):
         """ Lists some or all macros """
         if args.name:
             names_to_view = utils.remove_duplicates(args.name)
             for cur_name in names_to_view:
-                if cur_name in self.macros:
-                    self.poutput("macro create {} {}".format(cur_name, self.macros[cur_name].value))
+                stripped_name = utils.strip_quotes(cur_name)
+                if stripped_name in self.macros:
+                    self.poutput("macro create {} {}".format(stripped_name, self.macros[stripped_name].value))
                 else:
-                    self.perror("Macro {!r} not found".format(cur_name), traceback_war=False)
+                    self.perror("Macro {!r} not found".format(stripped_name), traceback_war=False)
         else:
             sorted_macros = utils.alphabetical_sort(self.macros)
             for cur_macro in sorted_macros:
@@ -2589,7 +2585,7 @@ class Cmd(cmd.Cmd):
             func(self, args)
         else:
             # No subcommand was provided, so call help
-            self.macro_parser.print_help()
+            self.do_help('macro')
 
     @with_argument_list
     def do_help(self, arglist: List[str]) -> None:
diff --git a/cmd2/parsing.py b/cmd2/parsing.py
index 82e8ee39..498834cf 100644
--- a/cmd2/parsing.py
+++ b/cmd2/parsing.py
@@ -291,7 +291,7 @@ class StatementParser:
         expr = r'\A\s*(\S*?)({})'.format(second_group)
         self._command_pattern = re.compile(expr)
 
-    def is_valid_command(self, word: str, allow_shortcut: bool) -> Tuple[bool, str]:
+    def is_valid_command(self, word: str) -> Tuple[bool, str]:
         """Determine whether a word is a valid name for a command.
 
         Commands can not include redirection characters, whitespace,
@@ -311,12 +311,11 @@ class StatementParser:
         if not word:
             return False, 'cannot be an empty string'
 
-        if not allow_shortcut:
-            errmsg = 'cannot start with a shortcut: '
-            errmsg += ', '.join(shortcut for (shortcut, expansion) in self.shortcuts)
-            for (shortcut, expansion) in self.shortcuts:
-                if word.startswith(shortcut):
-                    return False, errmsg
+        errmsg = 'cannot start with a shortcut: '
+        errmsg += ', '.join(shortcut for (shortcut, expansion) in self.shortcuts)
+        for (shortcut, expansion) in self.shortcuts:
+            if word.startswith(shortcut):
+                return False, errmsg
 
         errmsg = 'cannot contain: whitespace, quotes, '
         errchars = []
diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py
index 8900a14f..0468dd36 100644
--- a/tests/test_cmd2.py
+++ b/tests/test_cmd2.py
@@ -1765,9 +1765,10 @@ def test_poutput_color_never(base_app):
     assert out == expected
 
 
-# These are invalid targets for aliases and macros
-invalid_alias_and_macro_targets = [
+# These are invalid names for aliases and macros
+invalid_command_name = [
     '""',  # Blank name
+    '!no_shortcut',
     '">"',
     '"no>pe"',
     '"no spaces"',
@@ -1776,9 +1777,6 @@ invalid_alias_and_macro_targets = [
     'noembedded"quotes',
 ]
 
-# These are invalid names for aliases and macros
-invalid_alias_and_macro_names = invalid_alias_and_macro_targets + ['!no_shortcut']
-
 def test_get_alias_names(base_app):
     assert len(base_app.aliases) == 0
     run_cmd(base_app, 'alias create fake pyscript')
@@ -1793,6 +1791,10 @@ def test_get_macro_names(base_app):
     assert len(base_app.macros) == 2
     assert sorted(base_app.get_macro_names()) == ['bar', 'foo']
 
+def test_alias_no_subcommand(base_app, capsys):
+    out = run_cmd(base_app, 'alias')
+    assert "Usage: alias [-h]" in out[0]
+
 def test_alias_create(base_app, capsys):
     # Create the alias
     out = run_cmd(base_app, 'alias create fake pyscript')
@@ -1820,7 +1822,7 @@ def test_alias_create_with_quotes(base_app, capsys):
     out = run_cmd(base_app, 'alias list fake')
     assert out == normalize('alias create fake help > "out file.txt"')
 
-@pytest.mark.parametrize('alias_name', invalid_alias_and_macro_names)
+@pytest.mark.parametrize('alias_name', invalid_command_name)
 def test_alias_create_invalid_name(base_app, alias_name, capsys):
     run_cmd(base_app, 'alias create {} help'.format(alias_name))
     out, err = capsys.readouterr()
@@ -1833,12 +1835,6 @@ def test_alias_create_with_macro_name(base_app, capsys):
     out, err = capsys.readouterr()
     assert "Aliases cannot have the same name as a macro" in err
 
-@pytest.mark.parametrize('alias_target', invalid_alias_and_macro_targets)
-def test_alias_create_with_invalid_target(base_app, alias_target, capsys):
-    run_cmd(base_app, 'alias create my_alias {}'.format(alias_target))
-    out, err = capsys.readouterr()
-    assert "Invalid alias target" in err
-
 def test_alias_list_invalid_alias(base_app, capsys):
     # Look up invalid alias
     out = run_cmd(base_app, 'alias list invalid')
@@ -1862,6 +1858,10 @@ def test_alias_delete_non_existing(base_app, capsys):
     out, err = capsys.readouterr()
     assert "Alias 'fake' does not exist" in err
 
+def test_alias_delete_no_name(base_app, capsys):
+    out = run_cmd(base_app, 'alias delete')
+    assert "Usage: alias delete" in out[0]
+
 def test_multiple_aliases(base_app):
     alias1 = 'h1'
     alias2 = 'h2'
@@ -1875,6 +1875,10 @@ def test_multiple_aliases(base_app):
     expected = normalize(BASE_HELP_VERBOSE)
     assert out == expected
 
+def test_macro_no_subcommand(base_app, capsys):
+    out = run_cmd(base_app, 'macro')
+    assert "Usage: macro [-h]" in out[0]
+
 def test_macro_create(base_app, capsys):
     # Create the macro
     out = run_cmd(base_app, 'macro create fake pyscript')
@@ -1902,7 +1906,7 @@ def test_macro_create_with_quotes(base_app, capsys):
     out = run_cmd(base_app, 'macro list fake')
     assert out == normalize('macro create fake help > "out file.txt"')
 
-@pytest.mark.parametrize('macro_name', invalid_alias_and_macro_names)
+@pytest.mark.parametrize('macro_name', invalid_command_name)
 def test_macro_create_invalid_name(base_app, macro_name, capsys):
     run_cmd(base_app, 'macro create {} help'.format(macro_name))
     out, err = capsys.readouterr()
@@ -1921,12 +1925,6 @@ def test_macro_create_with_command_name(base_app, capsys):
     out, err = capsys.readouterr()
     assert "Macros cannot have the same name as a command" in err
 
-@pytest.mark.parametrize('macro_target', invalid_alias_and_macro_targets)
-def test_macro_create_with_invalid_target(base_app, macro_target, capsys):
-    run_cmd(base_app, 'macro create my_macro {}'.format(macro_target))
-    out, err = capsys.readouterr()
-    assert "Invalid macro target" in err
-
 def test_macro_create_with_args(base_app, capsys):
     # Create the macro
     out = run_cmd(base_app, 'macro create fake {1} {2}')
@@ -1981,6 +1979,10 @@ def test_macro_delete_non_existing(base_app, capsys):
     out, err = capsys.readouterr()
     assert "Macro 'fake' does not exist" in err
 
+def test_macro_delete_no_name(base_app, capsys):
+    out = run_cmd(base_app, 'macro delete')
+    assert "Usage: macro delete" in out[0]
+
 def test_multiple_macros(base_app):
     macro1 = 'h1'
     macro2 = 'h2'