Add support for more kinds of redirect operators in Eshell

* lisp/eshell/esh-arg.el: Require cl-lib.
(eshell-finish-arg): Allow passing multiple ARGUMENTS.
(eshell-quote-argument): Handle the case when 'eshell-finish-arg' was
passed multiple arguments.

* lisp/eshell/esh-cmd.el (eshell-do-pipelines)
(eshell-do-pipelines-synchronously): Only set stdout output handle.

* lisp/eshell/esh-io.el (eshell-redirection-operators-alist): New
constant.
(eshell-io-initialize): Prefer sharp quotes for functions.
(eshell-parse-redirection, eshell-strip-redirections): Add support for
more redirection forms.
(eshell-copy-output-handle, eshell-set-all-output-handles): New
functions.

* test/lisp/eshell/esh-io-tests.el
(esh-io-test/redirect-all/overwrite, esh-io-test/redirect-all/append)
(esh-io-test/redirect-all/insert, esh-io-test/redirect-copy)
(esh-io-test/redirect-copy-first, esh-io-test/redirect-pipe): New
tests.

* doc/misc/eshell.texi (Redirection): Document new redirection syntax.
(Pipelines): Document '|&' syntax.
(Bugs and ideas): Update item about redirection syntax.

* etc/NEWS: Announce this change.

6 files changed, 251 insertions, 47 deletions

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 0c98d2860e9..bc3b21d019e 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1659,6 +1659,40 @@ Redirect output to @var{dest}, inserting it at the current mark if
 @var{dest} is a buffer, at the beginning of the file if @var{dest} is
 a file, or otherwise behaving the same as @code{>>}.
 
+@item &> @var{file}
+@itemx >& @var{file}
+Redirect both standard output and standard error to @var{dest},
+overwriting its contents with the new output.
+
+@item &>> @var{file}
+@itemx >>& @var{file}
+Redirect both standard output and standard error to @var{dest},
+appending it to the existing contents of @var{dest}.
+
+@item &>>> @var{file}
+@itemx >>>& @var{file}
+Redirect both standard output and standard error to @var{dest},
+inserting it like with @code{>>> @var{file}}.
+
+@item >&@var{other-fd}
+@itemx @var{fd}>&@var{other-fd}
+Duplicate the file descriptor @var{other-fd} to @var{fd} (or 1 if
+unspecified).  The order in which this is used is signficant, so
+
+@example
+@var{command} > @var{file} 2>&1
+@end example
+
+redirects both standard output and standard error to @var{file},
+whereas
+
+@example
+@var{command} 2>&1 > @var{file}
+@end example
+
+only redirects standard output to @var{file} (and sends standard error
+to the display via standard output's original handle).
+
 @end table
 
 Eshell supports redirecting output to several different types of
@@ -1721,14 +1755,18 @@ The output function is called once on each line of output until
 @node Pipelines
 @section Pipelines
 As with most other shells, Eshell supports pipelines to pass the
-output of one command the input of the next command.  You can pipe
-commands to each other using the @code{|} operator.  For example,
+output of one command the input of the next command.  You can send the
+standard output of one command to the standard input of another using
+the @code{|} operator.  For example,
 
 @example
 ~ $ echo hello | rev
 olleh
 @end example
 
+To send both the standard output and standard error of a command to
+another command's input, you can use the @code{|&} operator.
+
 @subsection Running Shell Pipelines Natively
 When constructing shell pipelines that will move a lot of data, it is
 a good idea to bypass Eshell's own pipelining support and use the
@@ -2217,10 +2255,9 @@ current being used.
 
 @item How can Eshell learn if a background process has requested input?
 
-@item Support @samp{2>&1} and @samp{>&} and @samp{2>} and @samp{|&}
+@item Make a customizable syntax table for redirects
 
-The syntax table for parsing these should be customizable, such that the
-user could change it to use rc syntax: @samp{>[2=1]}.
+This way, the user could change it to use rc syntax: @samp{>[2=1]}.
 
 @item Allow @samp{$_[-1]}, which would indicate the last element of the array
 
diff --git a/etc/NEWS b/etc/NEWS
index 77ac0f5e6c1..476cd7ba6c1 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -321,6 +321,10 @@ been restricted to "...", '...', /.../, |...|, (...), [...], <...>,
 and {...}.  See the "(eshell) Argument Predication and Modification"
 node in the Eshell manual for more details.
 
++++
+*** Eshell pipelines now only pipe stdout by default.
+To pipe both stdout and stderr, use the '|&' operator instead of '|'.
+
 ---
 ** The 'delete-forward-char' command now deletes by grapheme clusters.
 This command is by default bound to the <Delete> function key
@@ -2238,6 +2242,13 @@ commands are Lisp function or external when supplying absolute file
 name arguments.  See "Electric forward slash" in the Eshell manual.
 
 +++
+*** Improved support for redirection operators in Eshell.
+Eshell now supports a wider variety of redirection operators.  For
+example, you can now redirect both stdout and stderr via '&>' or
+duplicate one output handle to another via 'NEW-FD>&OLD-FD'.  For more
+information, see "Redirections" in the Eshell manual.
+
++++
 *** Double-quoting an Eshell expansion now treats the result as a single string.
 If an Eshell expansion like '$FOO' is surrounded by double quotes, the
 result will always be a single string, no matter the type that would
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index 50fb7f5fdc6..576d32b8c5d 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -29,6 +29,9 @@
 
 (require 'esh-util)
 
+(eval-when-compile
+  (require 'cl-lib))
+
 (defgroup eshell-arg nil
   "Argument parsing involves transforming the arguments passed on the
 command line into equivalent Lisp forms that, when evaluated, will
@@ -248,10 +251,16 @@ convert the result to a number as well."
 	    eshell-current-modifiers (cdr eshell-current-modifiers))))
   (setq eshell-current-modifiers nil))
 
-(defun eshell-finish-arg (&optional argument)
-  "Finish the current ARGUMENT being processed."
-  (if argument
-      (setq eshell-current-argument argument))
+(defun eshell-finish-arg (&rest arguments)
+  "Finish the current argument being processed.
+If any ARGUMENTS are specified, they will be added to the final
+argument list in place of the value of the current argument."
+  (when arguments
+    (if (= (length arguments) 1)
+        (setq eshell-current-argument (car arguments))
+      (cl-assert (and (not eshell-arg-listified)
+                      (not eshell-current-modifiers)))
+      (setq eshell-current-argument (cons 'eshell-flatten-args arguments))))
   (throw 'eshell-arg-done t))
 
 (defun eshell-quote-argument (string)
@@ -291,7 +300,11 @@ Point is left at the end of the arguments."
                      (if (= (point) here)
                          (error "Failed to parse argument `%s'"
                                 (buffer-substring here (point-max))))
-                     (and arg (nconc args (list arg)))))))
+                     (when arg
+                       (nconc args
+                              (if (eq (car-safe arg) 'eshell-flatten-args)
+                                  (cdr arg)
+                                (list arg))))))))
               (throw 'eshell-incomplete (if (listp delim)
                                             delim
                                           (list delim (point) (cdr args)))))
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index a43ad77213d..413336e3eb5 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -810,8 +810,6 @@ This macro calls itself recursively, with NOTFIRST non-nil."
 	   `(let ((nextproc
 		   (eshell-do-pipelines (quote ,(cdr pipeline)) t)))
               (eshell-set-output-handle ,eshell-output-handle
-                                        'append nextproc)
-              (eshell-set-output-handle ,eshell-error-handle
                                         'append nextproc)))
 	,(let ((head (car pipeline)))
 	   (if (memq (car head) '(let progn))
@@ -842,8 +840,6 @@ This is used on systems where async subprocesses are not supported."
        ,(when (cdr pipeline)
           `(let ((output-marker ,(point-marker)))
              (eshell-set-output-handle ,eshell-output-handle
-                                       'append output-marker)
-             (eshell-set-output-handle ,eshell-error-handle
                                        'append output-marker)))
        ,(let ((head (car pipeline)))
           (if (memq (car head) '(let progn))
diff --git a/lisp/eshell/esh-io.el b/lisp/eshell/esh-io.el
index 01e8aceeabd..4620565f857 100644
--- a/lisp/eshell/esh-io.el
+++ b/lisp/eshell/esh-io.el
@@ -154,6 +154,14 @@ not be added to this variable."
 
 ;;; Internal Variables:
 
+(defconst eshell-redirection-operators-alist
+  '(("<"   . input)                     ; FIXME: Not supported yet.
+    (">"   . overwrite)
+    (">>"  . append)
+    (">>>" . insert))
+  "An association list of redirection operators to symbols
+describing the mode, e.g. for using with `eshell-get-target'.")
+
 (defvar eshell-current-handles nil)
 
 (defvar eshell-last-command-status 0
@@ -173,53 +181,104 @@ not be added to this variable."
 (defun eshell-io-initialize ()      ;Called from `eshell-mode' via intern-soft!
   "Initialize the I/O subsystem code."
   (add-hook 'eshell-parse-argument-hook
-	    'eshell-parse-redirection nil t)
+            #'eshell-parse-redirection nil t)
   (make-local-variable 'eshell-current-redirections)
   (add-hook 'eshell-pre-rewrite-command-hook
-	    'eshell-strip-redirections nil t)
+            #'eshell-strip-redirections nil t)
   (add-function :filter-return (local 'eshell-post-rewrite-command-function)
                 #'eshell--apply-redirections))
 
 (defun eshell-parse-redirection ()
-  "Parse an output redirection, such as `2>'."
-  (if (and (not eshell-current-quoted)
-	   (looking-at "\\([0-9]\\)?\\(<\\|>+\\)&?\\([0-9]\\)?\\s-*"))
+  "Parse an output redirection, such as `2>' or `>&'."
+  (when (not eshell-current-quoted)
+    (cond
+     ;; Copying a handle (e.g. `2>&1').
+     ((looking-at (rx (? (group digit))
+                      (group (or "<" ">"))
+                      "&" (group digit)
+                      (* (syntax whitespace))))
+      (let ((source (string-to-number (or (match-string 1) "1")))
+            (mode (cdr (assoc (match-string 2)
+                              eshell-redirection-operators-alist)))
+            (target (string-to-number (match-string 3))))
+        (when (eq mode 'input)
+          (error "Eshell does not support input redirection"))
+        (goto-char (match-end 0))
+        (eshell-finish-arg (list 'eshell-copy-output-handle
+                                 source target))))
+     ;; Shorthand for redirecting both stdout and stderr (e.g. `&>').
+     ((looking-at (rx (or (seq (group (** 1 3 ">")) "&")
+                          (seq "&" (group-n 1 (** 1 3 ">"))))
+                      (* (syntax whitespace))))
+      (if eshell-current-argument
+          (eshell-finish-arg)
+        (goto-char (match-end 0))
+        (eshell-finish-arg
+         (let ((mode (cdr (assoc (match-string 1)
+                                 eshell-redirection-operators-alist))))
+           (list 'eshell-set-all-output-handles
+                 (list 'quote mode))))))
+     ;; Shorthand for piping both stdout and stderr (i.e. `|&').
+     ((looking-at (rx "|&" (* (syntax whitespace))))
+      (if eshell-current-argument
+          (eshell-finish-arg)
+        (goto-char (match-end 0))
+        (eshell-finish-arg
+         '(eshell-copy-output-handle eshell-error-handle
+                                     eshell-output-handle)
+         '(eshell-operator "|"))))
+     ;; Regular redirecting (e.g. `2>').
+     ((looking-at (rx (? (group digit))
+                      (group (or "<" (** 1 3 ">")))
+                      (* (syntax whitespace))))
       (if eshell-current-argument
-	  (eshell-finish-arg)
-	(let ((sh (match-string 1))
-	      (oper (match-string 2))
-;	      (th (match-string 3))
-	      )
-	  (if (string= oper "<")
-	      (error "Eshell does not support input redirection"))
-	  (eshell-finish-arg
-	   (prog1
-	       (list 'eshell-set-output-handle
-		     (or (and sh (string-to-number sh)) 1)
-		     (list 'quote
-			   (aref [overwrite append insert]
-				 (1- (length oper)))))
-	     (goto-char (match-end 0))))))))
+          (eshell-finish-arg)
+        (let ((source (if (match-string 1)
+                          (string-to-number (match-string 1))
+                        eshell-output-handle))
+              (mode (cdr (assoc (match-string 2)
+                                eshell-redirection-operators-alist))))
+          (when (eq mode 'input)
+            (error "Eshell does not support input redirection"))
+          (goto-char (match-end 0))
+          (eshell-finish-arg
+           ;; Note: the target will be set later by
+           ;; `eshell-strip-redirections'.
+           (list 'eshell-set-output-handle
+                 source (list 'quote mode)))))))))
 
 (defun eshell-strip-redirections (terms)
   "Rewrite any output redirections in TERMS."
   (setq eshell-current-redirections (list t))
   (let ((tl terms)
-	(tt (cdr terms)))
+        (tt (cdr terms)))
     (while tt
-      (if (not (and (consp (car tt))
-		    (eq (caar tt) 'eshell-set-output-handle)))
-	  (setq tt (cdr tt)
-		tl (cdr tl))
-	(unless (cdr tt)
-	  (error "Missing redirection target"))
-	(nconc eshell-current-redirections
-	       (list (list 'ignore
-			   (append (car tt) (list (cadr tt))))))
-	(setcdr tl (cddr tt))
-	(setq tt (cddr tt))))
+      (cond
+       ;; Strip `eshell-copy-output-handle'.
+       ((and (consp (car tt))
+             (eq (caar tt) 'eshell-copy-output-handle))
+        (nconc eshell-current-redirections
+               (list (car tt)))
+        (setcdr tl (cddr tt))
+        (setq tt (cdr tt)))
+       ;; Strip `eshell-set-output-handle' or
+       ;; `eshell-set-all-output-handles' and the term immediately
+       ;; after (the redirection target).
+       ((and (consp (car tt))
+             (memq (caar tt) '(eshell-set-output-handle
+                               eshell-set-all-output-handles)))
+        (unless (cdr tt)
+          (error "Missing redirection target"))
+        (nconc eshell-current-redirections
+               (list (list 'ignore
+                           (append (car tt) (list (cadr tt))))))
+        (setcdr tl (cddr tt))
+        (setq tt (cddr tt)))
+       (t
+        (setq tt (cdr tt)
+              tl (cdr tl)))))
     (setq eshell-current-redirections
-	  (cdr eshell-current-redirections))))
+          (cdr eshell-current-redirections))))
 
 (defun eshell--apply-redirections (cmd)
   "Apply any redirection which were specified for COMMAND."
@@ -295,6 +354,22 @@ If HANDLES is nil, use `eshell-current-handles'."
               (aset handles index (cons nil 1)))
           (setcar (aref handles index) current))))))
 
+(defun eshell-copy-output-handle (index index-to-copy &optional handles)
+  "Copy the handle INDEX-TO-COPY to INDEX for the current HANDLES.
+If HANDLES is nil, use `eshell-current-handles'."
+  (let* ((handles (or handles eshell-current-handles))
+         (handle-to-copy (car (aref handles index-to-copy))))
+    (setcar (aref handles index)
+            (if (listp handle-to-copy)
+                (copy-sequence handle-to-copy)
+              handle-to-copy))))
+
+(defun eshell-set-all-output-handles (mode &optional target handles)
+  "Set output and error HANDLES to point to TARGET using MODE.
+If HANDLES is nil, use `eshell-current-handles'."
+  (eshell-set-output-handle eshell-output-handle mode target handles)
+  (eshell-copy-output-handle eshell-error-handle eshell-output-handle handles))
+
 (defun eshell-close-target (target status)
   "Close an output TARGET, passing STATUS as the result.
 STATUS should be non-nil on successful termination of the output."
diff --git a/test/lisp/eshell/esh-io-tests.el b/test/lisp/eshell/esh-io-tests.el
index 6cd2dff1c13..37b234eaf06 100644
--- a/test/lisp/eshell/esh-io-tests.el
+++ b/test/lisp/eshell/esh-io-tests.el
@@ -199,6 +199,78 @@
       (should (equal (buffer-string) "stderr\n")))
     (should (equal (buffer-string) "stdout\n"))))
 
+(ert-deftest esh-io-test/redirect-all/overwrite ()
+  "Check that redirecting to stdout and stderr via shorthand works."
+  (eshell-with-temp-buffer bufname "old"
+    (with-temp-eshell
+     (eshell-match-command-output (format "test-output &> #<%s>" bufname)
+                                  "\\`\\'"))
+    (should (equal (buffer-string) "stdout\nstderr\n")))
+  ;; Also check the alternate (and less-preferred in Bash) `>&' syntax.
+  (eshell-with-temp-buffer bufname "old"
+    (with-temp-eshell
+     (eshell-match-command-output (format "test-output >& #<%s>" bufname)
+                                  "\\`\\'"))
+    (should (equal (buffer-string) "stdout\nstderr\n"))))
+
+(ert-deftest esh-io-test/redirect-all/append ()
+  "Check that redirecting to stdout and stderr via shorthand works."
+  (eshell-with-temp-buffer bufname "old"
+    (with-temp-eshell
+     (eshell-match-command-output (format "test-output &>> #<%s>" bufname)
+                                  "\\`\\'"))
+    (should (equal (buffer-string) "oldstdout\nstderr\n")))
+  ;; Also check the alternate (and less-preferred in Bash) `>>&' syntax.
+  (eshell-with-temp-buffer bufname "old"
+    (with-temp-eshell
+     (eshell-match-command-output (format "test-output >>& #<%s>" bufname)
+                                  "\\`\\'"))
+    (should (equal (buffer-string) "oldstdout\nstderr\n"))))
+
+(ert-deftest esh-io-test/redirect-all/insert ()
+  "Check that redirecting to stdout and stderr via shorthand works."
+  (eshell-with-temp-buffer bufname "old"
+    (goto-char (point-min))
+    (with-temp-eshell
+     (eshell-match-command-output (format "test-output &>>> #<%s>" bufname)
+                                  "\\`\\'"))
+    (should (equal (buffer-string) "stdout\nstderr\nold")))
+  ;; Also check the alternate `>>>&' syntax.
+  (eshell-with-temp-buffer bufname "old"
+    (goto-char (point-min))
+    (with-temp-eshell
+     (eshell-match-command-output (format "test-output >>>& #<%s>" bufname)
+                                  "\\`\\'"))
+    (should (equal (buffer-string) "stdout\nstderr\nold"))))
+
+(ert-deftest esh-io-test/redirect-copy ()
+  "Check that redirecting stdout and then copying stdout to stderr works.
+This should redirect both stdout and stderr to the same place."
+  (eshell-with-temp-buffer bufname "old"
+    (with-temp-eshell
+     (eshell-match-command-output (format "test-output > #<%s> 2>&1" bufname)
+                                  "\\`\\'"))
+    (should (equal (buffer-string) "stdout\nstderr\n"))))
+
+(ert-deftest esh-io-test/redirect-copy-first ()
+  "Check that copying stdout to stderr and then redirecting stdout works.
+This should redirect stdout to a buffer, and stderr to where
+stdout originally pointed (the terminal)."
+  (eshell-with-temp-buffer bufname "old"
+    (with-temp-eshell
+     (eshell-match-command-output (format "test-output 2>&1 > #<%s>" bufname)
+                                  "stderr\n"))
+    (should (equal (buffer-string) "stdout\n"))))
+
+(ert-deftest esh-io-test/redirect-pipe ()
+  "Check that \"redirecting\" to a pipe works."
+  ;; `|' should only redirect stdout.
+  (eshell-command-result-equal "test-output | rev"
+                               "stderr\ntuodts\n")
+  ;; `|&' should redirect stdout and stderr.
+  (eshell-command-result-equal "test-output |& rev"
+                               "tuodts\nrredts\n"))
+
 
 ;; Virtual targets