Merge pull request #670 from python-cmd2/macro_refactor

Macro refactor

Author: kmvanbrunt

CHANGELOG.md

@@ -1,6 +1,10 @@
 ## 0.9.13 (TBD, 2019)
 * Bug Fixes
     * Fixed issue where the wrong terminator was being appended by `Statement.expanded_command_line()`
+    * Fixed issue where aliases and macros could not contain terminator characters in their values
+    * History now shows what was typed for macros and not the resolved value by default. This is consistent with
+    the behavior of aliases. Use the `expanded` or `verbose` arguments to `history` to see the resolved value for
+    the macro.
 * Enhancements
     * `pyscript` limits a command's stdout capture to the same period that redirection does.
       Therefore output from a command's postparsing and finalization hooks isn't saved in the StdSim object.
@@ -11,10 +15,13 @@
      scroll the actual error message off the screen.
     * Exceptions occurring in tab completion functions are now printed to stderr before returning control back to
     readline. This makes debugging a lot easier since readline suppresses these exceptions.
+* Potentially breaking changes
+    * Replaced `unquote_redirection_tokens()` with `unquote_specific_tokens()`. This was to support the fix
+      that allows terminators in alias and macro values.
 * **Python 3.4 EOL notice**
     * Python 3.4 reached its [end of life](https://www.python.org/dev/peps/pep-0429/) on March 18, 2019
     * This is the last release of `cmd2` which will support Python 3.4
-    
+
 ## 0.9.12 (April 22, 2019)
 * Bug Fixes
     * Fixed a bug in how redirection and piping worked inside ``py`` or ``pyscript`` commands

cmd2/cmd2.py

@@ -1703,7 +1703,7 @@ def onecmd_plus_hooks(self, line: str, pyscript_bridge_call: bool = False) -> bo
 
         stop = False
         try:
-            statement = self._complete_statement(line)
+            statement = self._input_line_to_statement(line)
         except EmptyStatement:
             return self._run_cmdfinalization_hooks(stop, None)
         except ValueError as ex:
@@ -1867,6 +1867,9 @@ def _complete_statement(self, line: str) -> Statement:
         self.pseudo_raw_input(). It returns a literal 'eof' if the input
         pipe runs out. We can't refactor it because we need to retain
         backwards compatibility with the standard library version of cmd.
+
+        :param line: the line being parsed
+        :return: the completed Statement
         """
         while True:
             try:
@@ -1914,6 +1917,91 @@ def _complete_statement(self, line: str) -> Statement:
             raise EmptyStatement()
         return statement
 
+    def _input_line_to_statement(self, line: str) -> Statement:
+        """
+        Parse the user's input line and convert it to a Statement, ensuring that all macros are also resolved
+
+        :param line: the line being parsed
+        :return: parsed command line as a Statement
+        """
+        used_macros = []
+        orig_line = line
+
+        # Continue until all macros are resolved
+        while True:
+            # Make sure all input has been read and convert it to a Statement
+            statement = self._complete_statement(line)
+
+            # Check if this command matches a macro and wasn't already processed to avoid an infinite loop
+            if statement.command in self.macros.keys() and statement.command not in used_macros:
+                used_macros.append(statement.command)
+                line = self._resolve_macro(statement)
+                if line is None:
+                    raise EmptyStatement()
+            else:
+                break
+
+        # This will be true when a macro was used
+        if orig_line != statement.raw:
+            # Build a Statement that contains the resolved macro line
+            # but the originally typed line for its raw member.
+            statement = Statement(statement.args,
+                                  raw=orig_line,
+                                  command=statement.command,
+                                  arg_list=statement.arg_list,
+                                  multiline_command=statement.multiline_command,
+                                  terminator=statement.terminator,
+                                  suffix=statement.suffix,
+                                  pipe_to=statement.pipe_to,
+                                  output=statement.output,
+                                  output_to=statement.output_to,
+                                  )
+        return statement
+
+    def _resolve_macro(self, statement: Statement) -> Optional[str]:
+        """
+        Resolve a macro and return the resulting string
+
+        :param statement: the parsed statement from the command line
+        :return: the resolved macro or None on error
+        """
+        from itertools import islice
+
+        if statement.command not in self.macros.keys():
+            raise KeyError('{} is not a macro'.format(statement.command))
+
+        macro = self.macros[statement.command]
+
+        # Make sure enough arguments were passed in
+        if len(statement.arg_list) < macro.minimum_arg_count:
+            self.perror("The macro '{}' expects at least {} argument(s)".format(statement.command,
+                                                                                macro.minimum_arg_count),
+                        traceback_war=False)
+            return None
+
+        # Resolve the arguments in reverse and read their values from statement.argv since those
+        # are unquoted. Macro args should have been quoted when the macro was created.
+        resolved = macro.value
+        reverse_arg_list = sorted(macro.arg_list, key=lambda ma: ma.start_index, reverse=True)
+
+        for arg in reverse_arg_list:
+            if arg.is_escaped:
+                to_replace = '{{' + arg.number_str + '}}'
+                replacement = '{' + arg.number_str + '}'
+            else:
+                to_replace = '{' + arg.number_str + '}'
+                replacement = statement.argv[int(arg.number_str)]
+
+            parts = resolved.rsplit(to_replace, maxsplit=1)
+            resolved = parts[0] + replacement + parts[1]
+
+        # Append extra arguments and use statement.arg_list since these arguments need their quotes preserved
+        for arg in islice(statement.arg_list, macro.minimum_arg_count, None):
+            resolved += ' ' + arg
+
+        # Restore any terminator, suffix, redirection, etc.
+        return resolved + statement.post_command
+
     def _redirect_output(self, statement: Statement) -> Tuple[bool, utils.RedirectionSavedState]:
         """Handles output redirection for >, >>, and |.
 
@@ -2060,73 +2148,25 @@ def onecmd(self, statement: Union[Statement, str]) -> bool:
         """
         # For backwards compatibility with cmd, allow a str to be passed in
         if not isinstance(statement, Statement):
-            statement = self._complete_statement(statement)
+            statement = self._input_line_to_statement(statement)
 
-        # Check if this is a macro
-        if statement.command in self.macros:
-            stop = self._run_macro(statement)
-        else:
-            func = self.cmd_func(statement.command)
-            if func:
-                # Check to see if this command should be stored in history
-                if statement.command not in self.exclude_from_history \
-                        and statement.command not in self.disabled_commands:
-                    self.history.append(statement)
+        func = self.cmd_func(statement.command)
+        if func:
+            # Check to see if this command should be stored in history
+            if statement.command not in self.exclude_from_history \
+                    and statement.command not in self.disabled_commands:
+                self.history.append(statement)
 
-                stop = func(statement)
+            stop = func(statement)
 
-            else:
-                stop = self.default(statement)
+        else:
+            stop = self.default(statement)
 
         if stop is None:
             stop = False
 
         return stop
 
-    def _run_macro(self, statement: Statement) -> bool:
-        """
-        Resolve a macro and run the resulting string
-
-        :param statement: the parsed statement from the command line
-        :return: a flag indicating whether the interpretation of commands should stop
-        """
-        from itertools import islice
-
-        if statement.command not in self.macros.keys():
-            raise KeyError('{} is not a macro'.format(statement.command))
-
-        macro = self.macros[statement.command]
-
-        # Make sure enough arguments were passed in
-        if len(statement.arg_list) < macro.minimum_arg_count:
-            self.perror("The macro '{}' expects at least {} argument(s)".format(statement.command,
-                                                                                macro.minimum_arg_count),
-                        traceback_war=False)
-            return False
-
-        # Resolve the arguments in reverse and read their values from statement.argv since those
-        # are unquoted. Macro args should have been quoted when the macro was created.
-        resolved = macro.value
-        reverse_arg_list = sorted(macro.arg_list, key=lambda ma: ma.start_index, reverse=True)
-
-        for arg in reverse_arg_list:
-            if arg.is_escaped:
-                to_replace = '{{' + arg.number_str + '}}'
-                replacement = '{' + arg.number_str + '}'
-            else:
-                to_replace = '{' + arg.number_str + '}'
-                replacement = statement.argv[int(arg.number_str)]
-
-            parts = resolved.rsplit(to_replace, maxsplit=1)
-            resolved = parts[0] + replacement + parts[1]
-
-        # Append extra arguments and use statement.arg_list since these arguments need their quotes preserved
-        for arg in islice(statement.arg_list, macro.minimum_arg_count, None):
-            resolved += ' ' + arg
-
-        # Run the resolved command
-        return self.onecmd_plus_hooks(resolved)
-
     def default(self, statement: Statement) -> Optional[bool]:
         """Executed when the command given isn't a recognized command implemented by a do_* method.
 
@@ -2286,7 +2326,10 @@ def alias_create(self, args: argparse.Namespace) -> None:
             self.perror("Alias cannot have the same name as a macro", traceback_war=False)
             return
 
-        utils.unquote_redirection_tokens(args.command_args)
+        # Unquote redirection and terminator tokens
+        tokens_to_unquote = constants.REDIRECTION_TOKENS
+        tokens_to_unquote.extend(self.statement_parser.terminators)
+        utils.unquote_specific_tokens(args.command_args, tokens_to_unquote)
 
         # Build the alias value string
         value = args.command
@@ -2342,8 +2385,8 @@ def alias_list(self, args: argparse.Namespace) -> None:
     alias_create_description = "Create or overwrite an alias"
 
     alias_create_epilog = ("Notes:\n"
-                           "  If you want to use redirection or pipes in the alias, then quote them to\n"
-                           "  prevent the 'alias create' command from being redirected.\n"
+                           "  If you want to use redirection, pipes, or terminators like ';' in the value\n"
+                           "  of the alias, then quote them.\n"
                            "\n"
                            "  Since aliases are resolved during parsing, tab completion will function as it\n"
                            "  would for the actual command the alias resolves to.\n"
@@ -2418,7 +2461,10 @@ def macro_create(self, args: argparse.Namespace) -> None:
             self.perror("Macro cannot have the same name as an alias", traceback_war=False)
             return
 
-        utils.unquote_redirection_tokens(args.command_args)
+        # Unquote redirection and terminator tokens
+        tokens_to_unquote = constants.REDIRECTION_TOKENS
+        tokens_to_unquote.extend(self.statement_parser.terminators)
+        utils.unquote_specific_tokens(args.command_args, tokens_to_unquote)
 
         # Build the macro value string
         value = args.command
@@ -2546,16 +2592,13 @@ def macro_list(self, args: argparse.Namespace) -> None:
                            "\n"
                            "    macro create backup !cp \"{1}\" \"{1}.orig\"\n"
                            "\n"
-                           "  Be careful! Since macros can resolve into commands, aliases, and macros,\n"
-                           "  it is possible to create a macro that results in infinite recursion.\n"
-                           "\n"
-                           "  If you want to use redirection or pipes in the macro, then quote them as in\n"
-                           "  this example to prevent the 'macro create' command from being redirected.\n"
+                           "  If you want to use redirection, pipes, or terminators like ';' in the value\n"
+                           "  of the macro, then quote them.\n"
                            "\n"
                            "    macro create show_results print_results -type {1} \"|\" less\n"
                            "\n"
-                           "  Because macros do not resolve until after parsing (hitting Enter), tab\n"
-                           "  completion will only complete paths.")
+                           "  Because macros do not resolve until after hitting Enter, tab completion\n"
+                           "  will only complete paths while entering a macro.")
 
     macro_create_parser = macro_subparsers.add_parser('create', help=macro_create_help,
                                                       description=macro_create_description,

cmd2/parsing.py

@@ -198,9 +198,9 @@ def command_and_args(self) -> str:
         return rtn
 
     @property
-    def expanded_command_line(self) -> str:
-        """Contains command_and_args plus any ending terminator, suffix, and redirection chars"""
-        rtn = self.command_and_args
+    def post_command(self) -> str:
+        """A string containing any ending terminator, suffix, and redirection chars"""
+        rtn = ''
         if self.terminator:
             rtn += self.terminator
 
@@ -217,6 +217,11 @@ def expanded_command_line(self) -> str:
 
         return rtn
 
+    @property
+    def expanded_command_line(self) -> str:
+        """Combines command_and_args and post_command"""
+        return self.command_and_args + self.post_command
+
     @property
     def argv(self) -> List[str]:
         """a list of arguments a la sys.argv.
@@ -618,26 +623,27 @@ def get_command_arg_list(self, command_name: str, to_parse: Union[Statement, str
             return to_parse, to_parse.argv[1:]
 
     def _expand(self, line: str) -> str:
-        """Expand shortcuts and aliases"""
+        """Expand aliases and shortcuts"""
+
+        # Make a copy of aliases so we can keep track of what aliases have been resolved to avoid an infinite loop
+        remaining_aliases = list(self.aliases.keys())
+        keep_expanding = bool(remaining_aliases)
 
-        # expand aliases
-        # make a copy of aliases so we can edit it
-        tmp_aliases = list(self.aliases.keys())
-        keep_expanding = bool(tmp_aliases)
         while keep_expanding:
-            for cur_alias in tmp_aliases:
-                keep_expanding = False
-                # apply our regex to line
-                match = self._command_pattern.search(line)
-                if match:
-                    # we got a match, extract the command
-                    command = match.group(1)
-                    if command and command == cur_alias:
-                        # rebuild line with the expanded alias
-                        line = self.aliases[cur_alias] + match.group(2) + line[match.end(2):]
-                        tmp_aliases.remove(cur_alias)
-                        keep_expanding = bool(tmp_aliases)
-                        break
+            keep_expanding = False
+
+            # apply our regex to line
+            match = self._command_pattern.search(line)
+            if match:
+                # we got a match, extract the command
+                command = match.group(1)
+
+                # Check if this command matches an alias that wasn't already processed
+                if command in remaining_aliases:
+                    # rebuild line with the expanded alias
+                    line = self.aliases[command] + match.group(2) + line[match.end(2):]
+                    remaining_aliases.remove(command)
+                    keep_expanding = bool(remaining_aliases)
 
         # expand shortcuts
         for (shortcut, expansion) in self.shortcuts:

cmd2/utils.py

@@ -262,15 +262,16 @@ def natural_sort(list_to_sort: Iterable[str]) -> List[str]:
     return sorted(list_to_sort, key=natural_keys)
 
 
-def unquote_redirection_tokens(args: List[str]) -> None:
+def unquote_specific_tokens(args: List[str], tokens_to_unquote: List[str]) -> None:
     """
-    Unquote redirection tokens in a list of command-line arguments
-    This is used when redirection tokens have to be passed to another command
+    Unquote a specific tokens in a list of command-line arguments
+    This is used when certain tokens have to be passed to another command
     :param args: the command line args
+    :param tokens_to_unquote: the tokens, which if present in args, to unquote
     """
     for i, arg in enumerate(args):
         unquoted_arg = strip_quotes(arg)
-        if unquoted_arg in constants.REDIRECTION_TOKENS:
+        if unquoted_arg in tokens_to_unquote:
             args[i] = unquoted_arg