From 43e9664a7a0c528a11e86dd6f2d07b3afe233f54 Mon Sep 17 00:00:00 2001 From: Jim Porter Date: Tue, 20 Dec 2022 09:39:07 -0800 Subject: [PATCH 2/3] 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. --- doc/misc/eshell.texi | 5 -- lisp/eshell/esh-cmd.el | 8 ++- test/lisp/eshell/em-extpipe-tests.el | 2 +- test/lisp/eshell/em-script-tests.el | 32 ++++++++++-- test/lisp/eshell/em-tramp-tests.el | 75 +++++++++++++++------------- test/lisp/eshell/esh-io-tests.el | 28 +++++++++++ 6 files changed, 103 insertions(+), 47 deletions(-) diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi index f9796d69a9a..118ee80acb9 100644 --- a/doc/misc/eshell.texi +++ b/doc/misc/eshell.texi @@ -2162,11 +2162,6 @@ Bugs and ideas @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 03388236b06..79957aeb416 100644 --- a/lisp/eshell/esh-cmd.el +++ b/lisp/eshell/esh-cmd.el @@ -418,8 +418,12 @@ eshell-parse-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 04e78279427..a2646a0296b 100644 --- a/test/lisp/eshell/em-extpipe-tests.el +++ b/test/lisp/eshell/em-extpipe-tests.el @@ -42,7 +42,7 @@ em-extpipe-tests--deftest (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 b837d464ccd..f720f697c67 100644 --- 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 6cc35ecdb1b..982a1eba279 100644 --- a/test/lisp/eshell/em-tramp-tests.el +++ b/test/lisp/eshell/em-tramp-tests.el @@ -27,21 +27,23 @@ em-tramp-test/su-default "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 @@ em-tramp-test/su-login ("-"))) (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 @@ em-tramp-test/sudo-shell ("-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 @@ em-tramp-test/doas-shell ("-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 37b234eaf06..ccf8ac1b9a1 100644 --- a/test/lisp/eshell/esh-io-tests.el +++ b/test/lisp/eshell/esh-io-tests.el @@ -146,6 +146,34 @@ esh-io-test/redirect-multiple/repeat (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")))) + ;; Redirecting specific handles -- 2.25.1