Added capability to redirect pipe commands and chain them together

Author: kmvanbrunt

cmd2/cmd2.py

@@ -2027,35 +2027,44 @@ def _redirect_output(self, statement: Statement) -> Tuple[bool, utils.Redirectio
             subproc_stdin = io.open(read_fd, 'r')
             new_stdout = io.open(write_fd, 'w')
 
-            # We want Popen to raise an exception if it fails to open the process.  Thus we don't set shell to True.
+            # Set options to not forward signals to the pipe process. If a Ctrl-C event occurs,
+            # our sigint handler will forward it only to the most recent pipe process. This makes
+            # sure pipe processes close in the right order (most recent first).
+            if sys.platform == 'win32':
+                creationflags = subprocess.CREATE_NEW_PROCESS_GROUP
+                start_new_session = False
+            else:
+                creationflags = 0
+                start_new_session = True
+
+            # For any stream that is a StdSim, we will use a pipe so we can capture its output
+            proc = subprocess.Popen(statement.pipe_to,
+                                    stdin=subproc_stdin,
+                                    stdout=subprocess.PIPE if isinstance(self.stdout, utils.StdSim) else self.stdout,
+                                    stderr=subprocess.PIPE if isinstance(sys.stderr, utils.StdSim) else sys.stderr,
+                                    creationflags=creationflags,
+                                    start_new_session=start_new_session,
+                                    shell=True)
+
+            # Popen was called with shell=True so the user can do stuff like redirect the output of the pipe
+            # process (ex: !ls | grep foo > out.txt). But this makes it difficult to know if the pipe process
+            # started OK, since the shell itself always starts. Therefore, we will wait a short time and check
+            # if the pipe process is still running.
             try:
-                # Set options to not forward signals to the pipe process. If a Ctrl-C event occurs,
-                # our sigint handler will forward it only to the most recent pipe process. This makes
-                # sure pipe processes close in the right order (most recent first).
-                if sys.platform == 'win32':
-                    creationflags = subprocess.CREATE_NEW_PROCESS_GROUP
-                    start_new_session = False
-                else:
-                    creationflags = 0
-                    start_new_session = True
-
-                # For any stream that is a StdSim, we will use a pipe so we can capture its output
-                proc = \
-                    subprocess.Popen(statement.pipe_to,
-                                     stdin=subproc_stdin,
-                                     stdout=subprocess.PIPE if isinstance(self.stdout, utils.StdSim) else self.stdout,
-                                     stderr=subprocess.PIPE if isinstance(sys.stderr, utils.StdSim) else sys.stderr,
-                                     creationflags=creationflags,
-                                     start_new_session=start_new_session)
+                proc.wait(0.2)
+            except subprocess.TimeoutExpired:
+                pass
 
-                saved_state.redirecting = True
-                saved_state.pipe_proc_reader = utils.ProcReader(proc, self.stdout, sys.stderr)
-                sys.stdout = self.stdout = new_stdout
-            except Exception as ex:
-                self.perror('Failed to open pipe because - {}'.format(ex), traceback_war=False)
+            # Check if the pipe process already exited
+            if proc.returncode is not None:
+                self.perror('Pipe process exited with code {} before command could run'.format(proc.returncode))
                 subproc_stdin.close()
                 new_stdout.close()
                 redir_error = True
+            else:
+                saved_state.redirecting = True
+                saved_state.pipe_proc_reader = utils.ProcReader(proc, self.stdout, sys.stderr)
+                sys.stdout = self.stdout = new_stdout
 
         elif statement.output:
             import tempfile
@@ -3021,21 +3030,8 @@ def do_shell(self, args: argparse.Namespace) -> None:
         # Create a list of arguments to shell
         tokens = [args.command] + args.command_args
 
-        # Support expanding ~ in quoted paths
-        for index, _ in enumerate(tokens):
-            if tokens[index]:
-                # Check if the token is quoted. Since parsing already passed, there isn't
-                # an unclosed quote. So we only need to check the first character.
-                first_char = tokens[index][0]
-                if first_char in constants.QUOTES:
-                    tokens[index] = utils.strip_quotes(tokens[index])
-
-                tokens[index] = os.path.expanduser(tokens[index])
-
-                # Restore the quotes
-                if first_char in constants.QUOTES:
-                    tokens[index] = first_char + tokens[index] + first_char
-
+        # Expand ~ where needed
+        utils.expand_user_in_tokens(tokens)
         expanded_command = ' '.join(tokens)
 
         # Prevent KeyboardInterrupts while in the shell process. The shell process will

cmd2/parsing.py

@@ -160,8 +160,8 @@ def do_mycommand(stmt):
     # characters appearing after the terminator but before output redirection, if any
     suffix = attr.ib(default='', validator=attr.validators.instance_of(str))
 
-    # if output was piped to a shell command, the shell command as a list of tokens
-    pipe_to = attr.ib(default=attr.Factory(list), validator=attr.validators.instance_of(list))
+    # if output was piped to a shell command, the shell command as a string
+    pipe_to = attr.ib(default='', validator=attr.validators.instance_of(str))
 
     # if output was redirected, the redirection token, i.e. '>>'
     output = attr.ib(default='', validator=attr.validators.instance_of(str))
@@ -208,12 +208,12 @@ def post_command(self) -> str:
             rtn += ' ' + self.suffix
 
         if self.pipe_to:
-            rtn += ' | ' + ' '.join(self.pipe_to)
+            rtn += ' | ' + self.pipe_to
 
         if self.output:
             rtn += ' ' + self.output
             if self.output_to:
-                rtn += ' ' + self.output_to
+                rtn += ' ' + utils.quote_string_if_needed(self.output_to)
 
         return rtn
 
@@ -460,18 +460,18 @@ def parse(self, line: str, expand: bool = True) -> Statement:
         try:
             # find the first pipe if it exists
             pipe_pos = tokens.index(constants.REDIRECTION_PIPE)
-            # save everything after the first pipe as tokens
-            pipe_to = tokens[pipe_pos + 1:]
 
-            for pos, cur_token in enumerate(pipe_to):
-                unquoted_token = utils.strip_quotes(cur_token)
-                pipe_to[pos] = os.path.expanduser(unquoted_token)
+            # Get the tokens for the pipe command and expand ~ where needed
+            pipe_to_tokens = tokens[pipe_pos + 1:]
+            utils.expand_user_in_tokens(pipe_to_tokens)
+
+            # Build the pipe command line string
+            pipe_to = ' '.join(pipe_to_tokens)
 
             # remove all the tokens after the pipe
             tokens = tokens[:pipe_pos]
         except ValueError:
-            # no pipe in the tokens
-            pipe_to = []
+            pipe_to = ''
 
         # check for output redirect
         output = ''

cmd2/utils.py

@@ -275,6 +275,27 @@ def unquote_specific_tokens(args: List[str], tokens_to_unquote: List[str]) -> No
             args[i] = unquoted_arg
 
 
+def expand_user_in_tokens(tokens: List[str]) -> None:
+    """
+    Call os.path.expanduser() on all tokens in an already parsed list of command-line arguments.
+    This also supports expanding user in quoted tokens.
+    :param tokens: tokens to expand
+    """
+    for index, _ in enumerate(tokens):
+        if tokens[index]:
+            # Check if the token is quoted. Since parsing already passed, there isn't
+            # an unclosed quote. So we only need to check the first character.
+            first_char = tokens[index][0]
+            if first_char in constants.QUOTES:
+                tokens[index] = strip_quotes(tokens[index])
+
+            tokens[index] = os.path.expanduser(tokens[index])
+
+            # Restore the quotes
+            if first_char in constants.QUOTES:
+                tokens[index] = first_char + tokens[index] + first_char
+
+
 def find_editor() -> str:
     """Find a reasonable editor to use by default for the system that the cmd2 application is running on."""
     editor = os.environ.get('EDITOR')

tests/test_cmd2.py

@@ -580,7 +580,7 @@ def test_pipe_to_shell_error(base_app):
     # Try to pipe command output to a shell command that doesn't exist in order to produce an error
     out, err = run_cmd(base_app, 'help | foobarbaz.this_does_not_exist')
     assert not out
-    assert "Failed to open pipe because" in err[0]
+    assert "Pipe process exited with code" in err[0]
 
 @pytest.mark.skipif(not clipboard.can_clip,
                     reason="Pyperclip could not find a copy/paste mechanism for your system")

tests/test_parsing.py

@@ -11,7 +11,6 @@
 
 import cmd2
 from cmd2 import constants, utils
-from cmd2.constants import MULTILINE_TERMINATOR
 from cmd2.parsing import StatementParser, shlex_split
 
 @pytest.fixture
@@ -46,7 +45,7 @@ def test_parse_empty_string(parser):
     assert statement.multiline_command == ''
     assert statement.terminator == ''
     assert statement.suffix == ''
-    assert statement.pipe_to == []
+    assert statement.pipe_to == ''
     assert statement.output == ''
     assert statement.output_to == ''
     assert statement.command_and_args == line
@@ -63,7 +62,7 @@ def test_parse_empty_string_default(default_parser):
     assert statement.multiline_command == ''
     assert statement.terminator == ''
     assert statement.suffix == ''
-    assert statement.pipe_to == []
+    assert statement.pipe_to == ''
     assert statement.output == ''
     assert statement.output_to == ''
     assert statement.command_and_args == line
@@ -130,7 +129,7 @@ def test_parse_single_word(parser, line):
     assert statement.multiline_command == ''
     assert statement.terminator == ''
     assert statement.suffix == ''
-    assert statement.pipe_to == []
+    assert statement.pipe_to == ''
     assert statement.output == ''
     assert statement.output_to == ''
     assert statement.command_and_args == line
@@ -224,8 +223,8 @@ def test_parse_simple_pipe(parser, line):
     assert statement.args == statement
     assert statement.argv == ['simple']
     assert not statement.arg_list
-    assert statement.pipe_to == ['piped']
-    assert statement.expanded_command_line == statement.command + ' | ' + ' '.join(statement.pipe_to)
+    assert statement.pipe_to == 'piped'
+    assert statement.expanded_command_line == statement.command + ' | ' + statement.pipe_to
 
 def test_parse_double_pipe_is_not_a_pipe(parser):
     line = 'double-pipe || is not a pipe'
@@ -247,7 +246,7 @@ def test_parse_complex_pipe(parser):
     assert statement.arg_list == statement.argv[1:]
     assert statement.terminator == '&'
     assert statement.suffix == 'sufx'
-    assert statement.pipe_to == ['piped']
+    assert statement.pipe_to == 'piped'
 
 @pytest.mark.parametrize('line,output', [
     ('help > out.txt', '>'),
@@ -307,7 +306,7 @@ def test_parse_pipe_and_redirect(parser):
     assert statement.arg_list == statement.argv[1:]
     assert statement.terminator == ';'
     assert statement.suffix == 'sufx'
-    assert statement.pipe_to == ['pipethrume', 'plz', '>', 'afile.txt']
+    assert statement.pipe_to == 'pipethrume plz > afile.txt'
     assert statement.output == ''
     assert statement.output_to == ''
 
@@ -516,7 +515,7 @@ def test_parse_alias_pipe(parser, line):
     assert statement.command == 'help'
     assert statement == ''
     assert statement.args == statement
-    assert statement.pipe_to == ['less']
+    assert statement.pipe_to == 'less'
 
 @pytest.mark.parametrize('line', [
     'helpalias;',
@@ -545,7 +544,7 @@ def test_parse_command_only_command_and_args(parser):
     assert statement.raw == line
     assert statement.terminator == ''
     assert statement.suffix == ''
-    assert statement.pipe_to == []
+    assert statement.pipe_to == ''
     assert statement.output == ''
     assert statement.output_to == ''
 
@@ -561,7 +560,7 @@ def test_parse_command_only_strips_line(parser):
     assert statement.raw == line
     assert statement.terminator == ''
     assert statement.suffix == ''
-    assert statement.pipe_to == []
+    assert statement.pipe_to == ''
     assert statement.output == ''
     assert statement.output_to == ''
 
@@ -577,7 +576,7 @@ def test_parse_command_only_expands_alias(parser):
     assert statement.raw == line
     assert statement.terminator == ''
     assert statement.suffix == ''
-    assert statement.pipe_to == []
+    assert statement.pipe_to == ''
     assert statement.output == ''
     assert statement.output_to == ''
 
@@ -594,7 +593,7 @@ def test_parse_command_only_expands_shortcuts(parser):
     assert statement.multiline_command == ''
     assert statement.terminator == ''
     assert statement.suffix == ''
-    assert statement.pipe_to == []
+    assert statement.pipe_to == ''
     assert statement.output == ''
     assert statement.output_to == ''
 
@@ -611,7 +610,7 @@ def test_parse_command_only_quoted_args(parser):
     assert statement.multiline_command == ''
     assert statement.terminator == ''
     assert statement.suffix == ''
-    assert statement.pipe_to == []
+    assert statement.pipe_to == ''
     assert statement.output == ''
     assert statement.output_to == ''
 
@@ -635,7 +634,7 @@ def test_parse_command_only_specialchars(parser, line, args):
     assert statement.multiline_command == ''
     assert statement.terminator == ''
     assert statement.suffix == ''
-    assert statement.pipe_to == []
+    assert statement.pipe_to == ''
     assert statement.output == ''
     assert statement.output_to == ''
 
@@ -664,7 +663,7 @@ def test_parse_command_only_empty(parser, line):
     assert statement.multiline_command == ''
     assert statement.terminator == ''
     assert statement.suffix == ''
-    assert statement.pipe_to == []
+    assert statement.pipe_to == ''
     assert statement.output == ''
     assert statement.output_to == ''
 
@@ -692,7 +691,7 @@ def test_statement_initialization():
     assert statement.multiline_command == ''
     assert statement.terminator == ''
     assert statement.suffix == ''
-    assert isinstance(statement.pipe_to, list)
+    assert isinstance(statement.pipe_to, str)
     assert not statement.pipe_to
     assert statement.output == ''
     assert statement.output_to == ''