Repository: python-cmd2/cmd2

Issue:
Cmd2.redirector isn't honored any more, either make it work or deprecate it
With the merge of PR #370, `Cmd2.redirector` is no longer honored: the redirector symbol is currently hard-coded as '>'. The documentation still states that you can set `Cmd2.redirector` to something like '->' and have the parsing logic honor that change. We need to either fix the code to match the documentation, or deprecate the attribute and fix the documentation to say that you can't change the redirector.

Related to #392.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 503f15e0..f9627194 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -29,6 +29,7 @@
     * Deleted ``cmd_with_subs_completer``, ``get_subcommands``, and ``get_subcommand_completer``
         * Replaced by default AutoCompleter implementation for all commands using argparse
     * Deleted support for old method of calling application commands with ``cmd()`` and ``self``
+    * ``cmd2.redirector`` is no longer supported. Output redirection can only be done with '>' or '>>'
 * Python 2 no longer supported
     * ``cmd2`` now supports Python 3.4+
 
diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index 02ae96fe..43fd99ec 100755
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -338,7 +338,6 @@ class Cmd(cmd.Cmd):
     # Attributes used to configure the StatementParser, best not to change these at runtime
     blankLinesAllowed = False
     multiline_commands = []
-    redirector = '>'        # for sending output to file
     shortcuts = {'?': 'help', '!': 'shell', '@': 'load', '@@': '_relative_load'}
     aliases = dict()
     terminators = [';']
@@ -1149,29 +1148,26 @@ class Cmd(cmd.Cmd):
 
             if len(raw_tokens) > 1:
 
-                # Build a list of all redirection tokens
-                all_redirects = constants.REDIRECTION_CHARS + ['>>']
-
                 # Check if there are redirection strings prior to the token being completed
                 seen_pipe = False
                 has_redirection = False
 
                 for cur_token in raw_tokens[:-1]:
-                    if cur_token in all_redirects:
+                    if cur_token in constants.REDIRECTION_TOKENS:
                         has_redirection = True
 
-                        if cur_token == '|':
+                        if cur_token == constants.REDIRECTION_PIPE:
                             seen_pipe = True
 
                 # Get token prior to the one being completed
                 prior_token = raw_tokens[-2]
 
                 # If a pipe is right before the token being completed, complete a shell command as the piped process
-                if prior_token == '|':
+                if prior_token == constants.REDIRECTION_PIPE:
                     return self.shell_cmd_complete(text, line, begidx, endidx)
 
                 # Otherwise do path completion either as files to redirectors or arguments to the piped process
-                elif prior_token in all_redirects or seen_pipe:
+                elif prior_token in constants.REDIRECTION_TOKENS or seen_pipe:
                     return self.path_complete(text, line, begidx, endidx)
 
                 # If there were redirection strings anywhere on the command line, then we
@@ -1820,7 +1816,7 @@ class Cmd(cmd.Cmd):
 
             # We want Popen to raise an exception if it fails to open the process.  Thus we don't set shell to True.
             try:
-                self.pipe_proc = subprocess.Popen(shlex.split(statement.pipe_to), stdin=subproc_stdin)
+                self.pipe_proc = subprocess.Popen(statement.pipe_to, stdin=subproc_stdin)
             except Exception as ex:
                 # Restore stdout to what it was and close the pipe
                 self.stdout.close()
@@ -1834,24 +1830,30 @@ class Cmd(cmd.Cmd):
                 raise ex
         elif statement.output:
             if (not statement.output_to) and (not can_clip):
-                raise EnvironmentError('Cannot redirect to paste buffer; install ``xclip`` and re-run to enable')
+                raise EnvironmentError("Cannot redirect to paste buffer; install 'pyperclip' and re-run to enable")
             self.kept_state = Statekeeper(self, ('stdout',))
             self.kept_sys = Statekeeper(sys, ('stdout',))
             self.redirecting = True
             if statement.output_to:
+                # going to a file
                 mode = 'w'
-                if statement.output == 2 * self.redirector:
+                # statement.output can only contain
+                # REDIRECTION_APPEND or REDIRECTION_OUTPUT
+                if statement.output == constants.REDIRECTION_APPEND:
                     mode = 'a'
                 sys.stdout = self.stdout = open(os.path.expanduser(statement.output_to), mode)
             else:
+                # going to a paste buffer
                 sys.stdout = self.stdout = tempfile.TemporaryFile(mode="w+")
-                if statement.output == '>>':
+                if statement.output == constants.REDIRECTION_APPEND:
                     self.poutput(get_paste_buffer())
 
     def _restore_output(self, statement):
-        """Handles restoring state after output redirection as well as the actual pipe operation if present.
+        """Handles restoring state after output redirection as well as
+        the actual pipe operation if present.
 
-        :param statement: Statement object which contains the parsed input from the user
+        :param statement: Statement object which contains the parsed
+                          input from the user
         """
         # If we have redirected output to a file or the clipboard or piped it to a shell command, then restore state
         if self.kept_state is not None:
diff --git a/cmd2/constants.py b/cmd2/constants.py
index 838650e5..b829000f 100644
--- a/cmd2/constants.py
+++ b/cmd2/constants.py
@@ -4,9 +4,14 @@
 
 import re
 
-# Used for command parsing, tab completion and word breaks. Do not change.
+# Used for command parsing, output redirection, tab completion and word
+# breaks. Do not change.
 QUOTES = ['"', "'"]
-REDIRECTION_CHARS = ['|', '>']
+REDIRECTION_PIPE = '|'
+REDIRECTION_OUTPUT = '>'
+REDIRECTION_APPEND = '>>'
+REDIRECTION_CHARS = [REDIRECTION_PIPE, REDIRECTION_OUTPUT]
+REDIRECTION_TOKENS = [REDIRECTION_PIPE, REDIRECTION_OUTPUT, REDIRECTION_APPEND]
 
 # Regular expression to match ANSI escape codes
 ANSI_ESCAPE_RE = re.compile(r'\x1b[^m]*m')
diff --git a/cmd2/parsing.py b/cmd2/parsing.py
index 3a9b390b..ce15bd38 100644
--- a/cmd2/parsing.py
+++ b/cmd2/parsing.py
@@ -45,7 +45,8 @@ class Statement(str):
                             redirection, if any
     :type suffix:           str or None
     :var pipe_to:           if output was piped to a shell command, the shell command
-    :type pipe_to:          str or None
+                            as a list of tokens
+    :type pipe_to:          list
     :var output:            if output was redirected, the redirection token, i.e. '>>'
     :type output:           str or None
     :var output_to:         if output was redirected, the destination, usually a filename
@@ -283,12 +284,27 @@ class StatementParser:
                 argv = tokens
                 tokens = []
 
+        # check for a pipe to a shell process
+        # if there is a pipe, everything after the pipe needs to be passed
+        # to the shell, even redirected output
+        # this allows '(Cmd) say hello | wc > countit.txt'
+        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:]
+            # remove all the tokens after the pipe
+            tokens = tokens[:pipe_pos]
+        except ValueError:
+            # no pipe in the tokens
+            pipe_to = None
+
         # check for output redirect
         output = None
         output_to = None
         try:
-            output_pos = tokens.index('>')
-            output = '>'
+            output_pos = tokens.index(constants.REDIRECTION_OUTPUT)
+            output = constants.REDIRECTION_OUTPUT
             output_to = ' '.join(tokens[output_pos+1:])
             # remove all the tokens after the output redirect
             tokens = tokens[:output_pos]
@@ -296,26 +312,14 @@ class StatementParser:
             pass
 
         try:
-            output_pos = tokens.index('>>')
-            output = '>>'
+            output_pos = tokens.index(constants.REDIRECTION_APPEND)
+            output = constants.REDIRECTION_APPEND
             output_to = ' '.join(tokens[output_pos+1:])
             # remove all tokens after the output redirect
             tokens = tokens[:output_pos]
         except ValueError:
             pass
 
-        # check for pipes
-        try:
-            # find the first pipe if it exists
-            pipe_pos = tokens.index('|')
-            # save everything after the first pipe
-            pipe_to = ' '.join(tokens[pipe_pos+1:])
-            # remove all the tokens after the pipe
-            tokens = tokens[:pipe_pos]
-        except ValueError:
-            # no pipe in the tokens
-            pipe_to = None
-
         if terminator:
             # whatever is left is the suffix
             suffix = ' '.join(tokens)
diff --git a/docs/freefeatures.rst b/docs/freefeatures.rst
index 95ae127c..a03a1d08 100644
--- a/docs/freefeatures.rst
+++ b/docs/freefeatures.rst
@@ -100,26 +100,8 @@ As in a Unix shell, output of a command can be redirected:
   - appended to a file with ``>>``, as in ``mycommand args >> filename.txt``
   - piped (``|``) as input to operating-system commands, as in
     ``mycommand args | wc``
-  - sent to the paste buffer, ready for the next Copy operation, by
-    ending with a bare ``>``, as in ``mycommand args >``..  Redirecting
-    to paste buffer requires software to be installed on the operating
-    system, pywin32_ on Windows or xclip_ on \*nix.
+  - sent to the operating system paste buffer, by ending with a bare ``>``, as in ``mycommand args >``. You can even append output to the current contents of the paste buffer by ending your command with ``>>``.
 
-If your application depends on mathematical syntax, ``>`` may be a bad
-choice for redirecting output - it will prevent you from using the
-greater-than sign in your actual user commands.  You can override your
-app's value of ``self.redirector`` to use a different string for output redirection::
-
-    class MyApp(cmd2.Cmd):
-        redirector = '->'
-
-::
-
-    (Cmd) say line1 -> out.txt
-    (Cmd) say line2 ->-> out.txt
-    (Cmd) !cat out.txt
-    line1
-    line2
 
 .. note::
 
@@ -136,8 +118,8 @@ app's value of ``self.redirector`` to use a different string for output redirect
    arguments after them from the command line arguments accordingly.  But output from a command will not be redirected
    to a file or piped to a shell command.
 
-.. _pywin32: http://sourceforge.net/projects/pywin32/
-.. _xclip: http://www.cyberciti.biz/faq/xclip-linux-insert-files-command-output-intoclipboard/
+If you need to include any of these redirection characters in your command,
+you can enclose them in quotation marks, ``mycommand 'with > in the argument'``.
 
 Python
 ======
diff --git a/docs/unfreefeatures.rst b/docs/unfreefeatures.rst
index a4776a53..41144c8f 100644
--- a/docs/unfreefeatures.rst
+++ b/docs/unfreefeatures.rst
@@ -10,13 +10,17 @@ commands whose names are listed in the
 parameter ``app.multiline_commands``.  These
 commands will be executed only
 after the user has entered a *terminator*.
-By default, the command terminators is
+By default, the command terminator is
 ``;``; replacing or appending to the list
 ``app.terminators`` allows different
 terminators.  A blank line
 is *always* considered a command terminator
 (cannot be overridden).
 
+In multiline commands, output redirection characters
+like ``>`` and ``|`` are part of the command
+arguments unless they appear after the terminator.
+
 
 Parsed statements
 =================


Test patch:
diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py
index bc76505f..6e4a5a3e 100644
--- a/tests/test_cmd2.py
+++ b/tests/test_cmd2.py
@@ -1430,7 +1430,7 @@ def test_clipboard_failure(capsys):
     # Make sure we got the error output
     out, err = capsys.readouterr()
     assert out == ''
-    assert 'Cannot redirect to paste buffer; install ``xclip`` and re-run to enable' in err
+    assert "Cannot redirect to paste buffer; install 'pyperclip' and re-run to enable" in err
 
 
 class CmdResultApp(cmd2.Cmd):
diff --git a/tests/test_parsing.py b/tests/test_parsing.py
index bfb55b23..41966c71 100644
--- a/tests/test_parsing.py
+++ b/tests/test_parsing.py
@@ -159,7 +159,7 @@ def test_parse_simple_pipe(parser, line):
     assert statement.command == 'simple'
     assert not statement.args
     assert statement.argv == ['simple']
-    assert statement.pipe_to == 'piped'
+    assert statement.pipe_to == ['piped']
 
 def test_parse_double_pipe_is_not_a_pipe(parser):
     line = 'double-pipe || is not a pipe'
@@ -177,7 +177,7 @@ def test_parse_complex_pipe(parser):
     assert statement.argv == ['command', 'with', 'args,', 'terminator']
     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', '>'),
@@ -227,9 +227,9 @@ def test_parse_pipe_and_redirect(parser):
     assert statement.argv == ['output', 'into']
     assert statement.terminator == ';'
     assert statement.suffix == 'sufx'
-    assert statement.pipe_to == 'pipethrume plz'
-    assert statement.output == '>'
-    assert statement.output_to == 'afile.txt'
+    assert statement.pipe_to == ['pipethrume', 'plz', '>', 'afile.txt']
+    assert not statement.output
+    assert not statement.output_to
 
 def test_parse_output_to_paste_buffer(parser):
     line = 'output to paste buffer >> '
@@ -240,8 +240,9 @@ def test_parse_output_to_paste_buffer(parser):
     assert statement.output == '>>'
 
 def test_parse_redirect_inside_terminator(parser):
-    """The terminator designates the end of the commmand/arguments portion.  If a redirector
-    occurs before a terminator, then it will be treated as part of the arguments and not as a redirector."""
+    """The terminator designates the end of the commmand/arguments portion.
+    If a redirector occurs before a terminator, then it will be treated as
+    part of the arguments and not as a redirector."""
     line = 'has > inside;'
     statement = parser.parse(line)
     assert statement.command == 'has'
@@ -385,7 +386,7 @@ def test_parse_alias_pipe(parser, line):
     statement = parser.parse(line)
     assert statement.command == 'help'
     assert not statement.args
-    assert statement.pipe_to == 'less'
+    assert statement.pipe_to == ['less']
 
 def test_parse_alias_terminator_no_whitespace(parser):
     line = 'helpalias;'