Fix handling of output handles in nested Eshell forms

Previously, the output handles in nested forms would be reset to the
default, leading to wrong behavior for commands like

  {echo a; echo b} > file

"b" would be written to "file" as expected, but "a" would go to
standard output (bug#59545).

* lisp/eshell/esh-cmd.el (eshell-parse-command): Use
'eshell-with-copied-handles' for each statement within the whole
Eshell command.

* test/lisp/eshell/esh-io-tests.el (esh-io-test/redirect-subcommands)
(esh-io-test/redirect-subcommands/override)
(esh-io-test/redirect-subcommands/interpolated): New tests.

* test/lisp/eshell/em-script-tests.el
(em-script-test/source-script/redirect)
(em-script-test/source-script/redirect/dev-null): New tests.
(em-script-test/source-script, em-script-test/source-script/arg-vars)
(em-script-test/source-script/all-args-var): Tweak names/docstrings.

* test/lisp/eshell/em-extpipe-tests.el (em-extpipe-tests--deftest):
Skip over the newly-added 'eshell-with-copied-handles' form when
checking the parse results.

* test/lisp/eshell/em-tramp-tests.el (em-tramp-test/su-default)
(em-tramp-test/su-user, em-tramp-test/su-login)
(em-tramp-test/sudo-shell, em-tramp-test/sudo-user-shell)
(em-tramp-test/doas-shell, em-tramp-test/doas-user-shell): Update
expected command forms.

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index f9796d69a9ac8d43193b13d6edfdea3c949bc1f1..118ee80acb9203e26ececbb96f8b16fb1e52c0f3 100644 (file)
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -2162,11 +2162,6 @@ So that @kbd{M-@key{DEL}} acts in a predictable manner, etc.
 
 @item Allow all Eshell buffers to share the same history and list-dir
 
-@item There is a problem with script commands that output to @file{/dev/null}
-
-If a script file, somewhere in the middle, uses @samp{> /dev/null},
-output from all subsequent commands is swallowed.
-
 @item Split up parsing of text after @samp{$} in @file{esh-var.el}
 
 Make it similar to the way that @file{esh-arg.el} is structured.

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 03388236b06855f85d60f1a04a9f4cbb98a21687..79957aeb4162415cf9d269d6729db5f15ef4e4af 100644 (file)
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -418,8 +418,12 @@ hooks should be run before and after the command."
           (eshell-separate-commands terms "[&;]" nil 'eshell--sep-terms))))
     (let ((cmd commands))
       (while cmd
-       (if (cdr cmd)
-           (setcar cmd `(eshell-commands ,(car cmd))))
+        ;; Copy I/O handles so each full statement can manipulate them
+        ;; if they like.  As a small optimization, skip this for the
+        ;; last top-level one; we won't use these handles again
+        ;; anyway.
+        (when (or (not toplevel) (cdr cmd))
+         (setcar cmd `(eshell-with-copied-handles ,(car cmd))))
        (setq cmd (cdr cmd))))
     (if toplevel
        `(eshell-commands (progn

diff --git a/test/lisp/eshell/em-extpipe-tests.el b/test/lisp/eshell/em-extpipe-tests.el
index 04e7827942772d63c3a1db6986b2d8cbf5bcaed6..a2646a0296b455e2c0aac31f7d7f7672c6cbd9df 100644 (file)
--- a/test/lisp/eshell/em-extpipe-tests.el
+++ b/test/lisp/eshell/em-extpipe-tests.el
@@ -42,7 +42,7 @@
                    (shell-command-switch "-c"))
                ;; Strip `eshell-trap-errors'.
                (should (equal ,expected
-                              (cadr (eshell-parse-command input))))))
+                              (cadadr (eshell-parse-command input))))))
           (with-substitute-for-temp (&rest body)
             ;; Substitute name of an actual temporary file and/or
             ;; buffer into `input'.  The substitution logic is

diff --git a/test/lisp/eshell/em-script-tests.el b/test/lisp/eshell/em-script-tests.el
index b837d464ccd40e18aae012700b4afb6c71e4be25..f720f697c67da5442861e6d2dcef3226467b99ff 100644 (file)
--- a/test/lisp/eshell/em-script-tests.el
+++ b/test/lisp/eshell/em-script-tests.el
@@ -35,21 +35,43 @@
 ;;; Tests:
 
 (ert-deftest em-script-test/source-script ()
-  "Test sourcing script with no argumentss"
+  "Test sourcing a simple script."
   (ert-with-temp-file temp-file :text "echo hi"
     (with-temp-eshell
      (eshell-match-command-output (format "source %s" temp-file)
                                   "hi\n"))))
 
-(ert-deftest em-script-test/source-script-arg-vars ()
-  "Test sourcing script with $0, $1, ... variables"
+(ert-deftest em-script-test/source-script/redirect ()
+  "Test sourcing a script and redirecting its output."
+  (ert-with-temp-file temp-file
+    :text "echo hi\necho bye"
+    (eshell-with-temp-buffer bufname "old"
+      (with-temp-eshell
+       (eshell-match-command-output
+        (format "source %s > #<%s>" temp-file bufname)
+        "\\`\\'"))
+      (should (equal (buffer-string) "hibye")))))
+
+(ert-deftest em-script-test/source-script/redirect/dev-null ()
+  "Test sourcing a script and redirecting its output, including to /dev/null."
+  (ert-with-temp-file temp-file
+    :text "echo hi\necho bad > /dev/null\necho bye"
+    (eshell-with-temp-buffer bufname "old"
+      (with-temp-eshell
+       (eshell-match-command-output
+        (format "source %s > #<%s>" temp-file bufname)
+        "\\`\\'"))
+      (should (equal (buffer-string) "hibye")))))
+
+(ert-deftest em-script-test/source-script/arg-vars ()
+  "Test sourcing script with $0, $1, ... variables."
   (ert-with-temp-file temp-file :text "printnl $0 \"$1 $2\""
     (with-temp-eshell
      (eshell-match-command-output (format "source %s one two" temp-file)
                                   (format "%s\none two\n" temp-file)))))
 
-(ert-deftest em-script-test/source-script-all-args-var ()
-  "Test sourcing script with the $* variable"
+(ert-deftest em-script-test/source-script/all-args-var ()
+  "Test sourcing script with the $* variable."
   (ert-with-temp-file temp-file :text "printnl $*"
     (with-temp-eshell
      (eshell-match-command-output (format "source %s" temp-file)

diff --git a/test/lisp/eshell/em-tramp-tests.el b/test/lisp/eshell/em-tramp-tests.el
index 6cc35ecdb1b1f1fb15a72d85f5787bdcedbb9f4b..982a1eba2796afe8e40d63eda541d3f33a68413a 100644 (file)
--- a/test/lisp/eshell/em-tramp-tests.el
+++ b/test/lisp/eshell/em-tramp-tests.el
@@ -27,21 +27,23 @@
   "Test Eshell `su' command with no arguments."
   (should (equal
            (catch 'eshell-replace-command (eshell/su))
-           `(eshell-trap-errors
-             (eshell-named-command
-              "cd"
-              (list ,(format "/su:root@%s:%s"
-                             tramp-default-host default-directory)))))))
+           `(eshell-with-copied-handles
+             (eshell-trap-errors
+              (eshell-named-command
+               "cd"
+               (list ,(format "/su:root@%s:%s"
+                              tramp-default-host default-directory))))))))
 
 (ert-deftest em-tramp-test/su-user ()
   "Test Eshell `su' command with USER argument."
   (should (equal
            (catch 'eshell-replace-command (eshell/su "USER"))
-           `(eshell-trap-errors
-             (eshell-named-command
-              "cd"
-              (list ,(format "/su:USER@%s:%s"
-                             tramp-default-host default-directory)))))))
+           `(eshell-with-copied-handles
+             (eshell-trap-errors
+              (eshell-named-command
+               "cd"
+               (list ,(format "/su:USER@%s:%s"
+                              tramp-default-host default-directory))))))))
 
 (ert-deftest em-tramp-test/su-login ()
   "Test Eshell `su' command with -/-l/--login option."
@@ -50,10 +52,11 @@
                   ("-")))
     (should (equal
              (catch 'eshell-replace-command (apply #'eshell/su args))
-             `(eshell-trap-errors
-               (eshell-named-command
-                "cd"
-                (list ,(format "/su:root@%s:~/" tramp-default-host))))))))
+             `(eshell-with-copied-handles
+               (eshell-trap-errors
+                (eshell-named-command
+                 "cd"
+                 (list ,(format "/su:root@%s:~/" tramp-default-host)))))))))
 
 (defun mock-eshell-named-command (&rest args)
   "Dummy function to test Eshell `sudo' command rewriting."
@@ -91,21 +94,23 @@
                   ("-s")))
     (should (equal
              (catch 'eshell-replace-command (apply #'eshell/sudo args))
-             `(eshell-trap-errors
-               (eshell-named-command
-                "cd"
-                (list ,(format "/sudo:root@%s:%s"
-                               tramp-default-host default-directory))))))))
+             `(eshell-with-copied-handles
+               (eshell-trap-errors
+                (eshell-named-command
+                 "cd"
+                 (list ,(format "/sudo:root@%s:%s"
+                                tramp-default-host default-directory)))))))))
 
 (ert-deftest em-tramp-test/sudo-user-shell ()
   "Test Eshell `sudo' command with -s and -u options."
   (should (equal
            (catch 'eshell-replace-command (eshell/sudo "-u" "USER" "-s"))
-           `(eshell-trap-errors
-             (eshell-named-command
-              "cd"
-              (list ,(format "/sudo:USER@%s:%s"
-                             tramp-default-host default-directory)))))))
+           `(eshell-with-copied-handles
+             (eshell-trap-errors
+              (eshell-named-command
+               "cd"
+               (list ,(format "/sudo:USER@%s:%s"
+                              tramp-default-host default-directory))))))))
 
 (ert-deftest em-tramp-test/doas-basic ()
   "Test Eshell `doas' command with default user."
@@ -144,20 +149,22 @@
                   ("-s")))
     (should (equal
              (catch 'eshell-replace-command (apply #'eshell/doas args))
-             `(eshell-trap-errors
-               (eshell-named-command
-                "cd"
-                (list ,(format "/doas:root@%s:%s"
-                               tramp-default-host default-directory))))))))
+             `(eshell-with-copied-handles
+               (eshell-trap-errors
+                (eshell-named-command
+                 "cd"
+                 (list ,(format "/doas:root@%s:%s"
+                                tramp-default-host default-directory)))))))))
 
 (ert-deftest em-tramp-test/doas-user-shell ()
   "Test Eshell `doas' command with -s and -u options."
   (should (equal
            (catch 'eshell-replace-command (eshell/doas "-u" "USER" "-s"))
-           `(eshell-trap-errors
-             (eshell-named-command
-              "cd"
-              (list ,(format "/doas:USER@%s:%s"
-                             tramp-default-host default-directory)))))))
+           `(eshell-with-copied-handles
+             (eshell-trap-errors
+              (eshell-named-command
+               "cd"
+               (list ,(format "/doas:USER@%s:%s"
+                              tramp-default-host default-directory))))))))
 
 ;;; em-tramp-tests.el ends here

diff --git a/test/lisp/eshell/esh-io-tests.el b/test/lisp/eshell/esh-io-tests.el
index 37b234eaf0688e7a21fd96265a34d332e7cdc2da..ccf8ac1b9a1ff836669bf93ad977cc4d4a9d486a 100644 (file)
--- a/test/lisp/eshell/esh-io-tests.el
+++ b/test/lisp/eshell/esh-io-tests.el
@@ -146,6 +146,34 @@
      (should (equal (buffer-string) "new"))
      (should (equal eshell-test-value "new")))))
 
+(ert-deftest esh-io-test/redirect-subcommands ()
+  "Check that redirecting subcommands applies to all subcommands."
+  (eshell-with-temp-buffer bufname "old"
+    (with-temp-eshell
+     (eshell-insert-command (format "{echo foo; echo bar} > #<%s>" bufname)))
+    (should (equal (buffer-string) "foobar"))))
+
+(ert-deftest esh-io-test/redirect-subcommands/override ()
+  "Check that redirecting subcommands applies to all subcommands.
+Include a redirect to another location in the subcommand to
+ensure only its statement is redirected."
+  (eshell-with-temp-buffer bufname "old"
+    (eshell-with-temp-buffer bufname-2 "also old"
+      (with-temp-eshell
+       (eshell-insert-command
+        (format "{echo foo; echo bar > #<%s>; echo baz} > #<%s>"
+                bufname-2 bufname)))
+      (should (equal (buffer-string) "bar")))
+    (should (equal (buffer-string) "foobaz"))))
+
+(ert-deftest esh-io-test/redirect-subcommands/interpolated ()
+  "Check that redirecting interpolated subcommands applies to all subcommands."
+  (eshell-with-temp-buffer bufname "old"
+    (with-temp-eshell
+     (eshell-insert-command
+      (format "echo ${echo foo; echo bar} > #<%s>" bufname)))
+    (should (equal (buffer-string) "foobar"))))
+
 \f
 ;; Redirecting specific handles