diff options
| -rw-r--r-- | cmd2/cmd2.py | 74 | ||||
| -rw-r--r-- | cmd2/parsing.py | 22 | ||||
| -rw-r--r-- | cmd2/utils.py | 21 | ||||
| -rw-r--r-- | tests/test_cmd2.py | 2 | ||||
| -rw-r--r-- | tests/test_parsing.py | 33 |
5 files changed, 84 insertions, 68 deletions
diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index 431c51ae..aa4c7a49 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -2027,35 +2027,44 @@ class Cmd(cmd.Cmd): 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 @@ class Cmd(cmd.Cmd): # 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 diff --git a/cmd2/parsing.py b/cmd2/parsing.py index 934f1d26..306d3bdd 100644 --- a/cmd2/parsing.py +++ b/cmd2/parsing.py @@ -160,8 +160,8 @@ class Statement(str): # 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 @@ class Statement(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 @@ class StatementParser: 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 = '' diff --git a/cmd2/utils.py b/cmd2/utils.py index e8e8a611..45e55c2b 100644 --- a/cmd2/utils.py +++ b/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') diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py index 5f6af8c5..85d97da3 100644 --- a/tests/test_cmd2.py +++ b/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") diff --git a/tests/test_parsing.py b/tests/test_parsing.py index 09215804..bd9648dc 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -11,7 +11,6 @@ import pytest 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 == '' |