[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
master 6defbd65b6 2/3: Fix handling of output handles in nested Eshell f
|
From: |
Jim Porter |
|
Subject: |
master 6defbd65b6 2/3: Fix handling of output handles in nested Eshell forms |
|
Date: |
Thu, 22 Dec 2022 14:59:11 -0500 (EST) |
branch: master
commit 6defbd65b664b17ad7389a936743debe23d5257e
Author: Jim Porter <jporterbugs@gmail.com>
Commit: Jim Porter <jporterbugs@gmail.com>
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 f9796d69a9..118ee80acb 100644
--- 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 03388236b0..79957aeb41 100644
--- 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 04e7827942..a2646a0296 100644
--- 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 b837d464cc..f720f697c6 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 6cc35ecdb1..982a1eba27 100644
--- 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 37b234eaf0..ccf8ac1b9a 100644
--- 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"))))
+
;; Redirecting specific handles