From 54087e84df872c9aa30866b880e8ac0b917cbd94 Mon Sep 17 00:00:00 2001 From: Jim Porter Date: Mon, 19 Dec 2022 22:21:10 -0800 Subject: Add 'eshell-duplicate-handles' to return a copy of file handles * lisp/eshell/esh-io.el (eshell-create-handles): Support creating with multiple targets for stdout and/or stderr. Make the targets for a handle always be a list, and store whether the targets are the default in a separate 'default' field. (eshell-protect-handles, eshell-close-handles) (eshell-copy-output-handle, eshell-interactive-output-p) (eshell-output-object): Update for changes in 'eshell-create-handles'. (eshell-duplicate-handles, eshell-get-targets): New functions. * lisp/eshell/esh-cmd.el (eshell-copy-handles): Rename and alias to... (eshell-with-copied-handles): ... this function, and use 'eshell-duplicate-handles'. (eshell-execute-pipeline): Use 'eshell-duplicate-handles'. --- lisp/eshell/esh-cmd.el | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) (limited to 'lisp/eshell/esh-cmd.el') diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el index 1fb84991120..03388236b06 100644 --- a/lisp/eshell/esh-cmd.el +++ b/lisp/eshell/esh-cmd.el @@ -788,16 +788,15 @@ this grossness will be made to disappear by using `call/cc'..." (defvar eshell-output-handle) ;Defined in esh-io.el. (defvar eshell-error-handle) ;Defined in esh-io.el. -(defmacro eshell-copy-handles (object) +(defmacro eshell-with-copied-handles (object) "Duplicate current I/O handles, so OBJECT works with its own copy." `(let ((eshell-current-handles - (eshell-create-handles - (car (aref eshell-current-handles - eshell-output-handle)) nil - (car (aref eshell-current-handles - eshell-error-handle)) nil))) + (eshell-duplicate-handles eshell-current-handles))) ,object)) +(define-obsolete-function-alias 'eshell-copy-handles + #'eshell-with-copied-handles "30.1") + (defmacro eshell-protect (object) "Protect I/O handles, so they aren't get closed after eval'ing OBJECT." `(progn @@ -808,7 +807,7 @@ this grossness will be made to disappear by using `call/cc'..." "Execute the commands in PIPELINE, connecting each to one another. This macro calls itself recursively, with NOTFIRST non-nil." (when (setq pipeline (cadr pipeline)) - `(eshell-copy-handles + `(eshell-with-copied-handles (progn ,(when (cdr pipeline) `(let ((nextproc @@ -880,11 +879,8 @@ This is used on systems where async subprocesses are not supported." (progn ,(if (fboundp 'make-process) `(eshell-do-pipelines ,pipeline) - `(let ((tail-handles (eshell-create-handles - (car (aref eshell-current-handles - ,eshell-output-handle)) nil - (car (aref eshell-current-handles - ,eshell-error-handle)) nil))) + `(let ((tail-handles (eshell-duplicate-handles + eshell-current-handles))) (eshell-do-pipelines-synchronously ,pipeline))) (eshell-process-identity (cons (symbol-value headproc) (symbol-value tailproc)))))) -- cgit v1.2.1 From 6defbd65b664b17ad7389a936743debe23d5257e Mon Sep 17 00:00:00 2001 From: Jim Porter Date: Tue, 20 Dec 2022 09:39:07 -0800 Subject: 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. --- lisp/eshell/esh-cmd.el | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'lisp/eshell/esh-cmd.el') 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 @@ 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 -- cgit v1.2.1 From 073da412a139e317959f56e359ed12de726a0a35 Mon Sep 17 00:00:00 2001 From: Jim Porter Date: Sat, 24 Dec 2022 14:31:50 -0800 Subject: Fix reference-counting of Eshell I/O handles This ensures that output targets in Eshell are only closed when Eshell is actually done with them. In particular, this means that "{ echo foo; echo bar } | rev" prints "raboof" as expected (bug#59545). * lisp/eshell/esh-io.el (eshell-create-handles): Structure the handles differently so the targets and their ref-count can be shared. (eshell-duplicate-handles): Reimplement this to share targets between the original and new handle sets. Add STEAL-P argument. (eshell-protect-handles, eshell-copy-output-handle) (eshell-interactive-output-p, eshell-output-object): Account for changes to the handle structure. (eshell-close-handle): New function... (eshell-close-handles, eshell-set-output-handle): ... use it. (eshell-get-targets): Remove. This only existed to make the previous implementation of 'eshell-duplicate-handles' work. * lisp/eshell/esh-cmd.el (eshell-with-copied-handles): New argument STEAL-P. (eshell-do-pipelines): Use STEAL-P for the last item in the pipeline. (eshell-parse-command): Don't copy handles for the last command in the list; explain why we can't use STEAL-P here. (eshell-eval-command): When queuing input, set 'eshell-command-body' and 'eshell-test-body' for the 'if' conditional (see 'eshell-do-eval'). * test/lisp/eshell/esh-io-tests.el (esh-io-test/redirect-pipe): Split into... (esh-io-test/pipeline/default, esh-io-test/pipeline/all): ... these. (esh-io-test/pipeline/subcommands): New test. * test/lisp/eshell/esh-cmd-tests.el (esh-cmd-test/for-loop-pipe) (esh-cmd-test/while-loop-pipe, esh-cmd-test/if-statement-pipe) esh-cmd-test/if-else-statement-pipe): New tests. (esh-cmd-test/while-loop): Use 'pop' to simplify the test a bit. * test/lisp/eshell/eshell-test-helpers.el (eshell-test--max-subprocess-time): Rename to... (eshell-test--max-wait-time): ... this. (eshell-wait-for): New function... (eshell-wait-for-subprocess): ... use it. * test/lisp/eshell/eshell-tests.el (eshell-test/queue-input): Fix this test. Previously, it didn't correctly verify that the original command completed. * test/lisp/eshell/em-tramp-tests.el (em-tramp-test/should-replace-command): New macro... (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): ... use it. --- lisp/eshell/esh-cmd.el | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) (limited to 'lisp/eshell/esh-cmd.el') diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el index 79957aeb416..39579335cf7 100644 --- a/lisp/eshell/esh-cmd.el +++ b/lisp/eshell/esh-cmd.el @@ -419,11 +419,10 @@ hooks should be run before and after the command." (let ((cmd commands)) (while 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)))) + ;; if they like. Steal the handles for the last command in + ;; the list; we won't use the originals again anyway. + (setcar cmd `(eshell-with-copied-handles + ,(car cmd) ,(not (cdr cmd)))) (setq cmd (cdr cmd)))) (if toplevel `(eshell-commands (progn @@ -792,10 +791,12 @@ this grossness will be made to disappear by using `call/cc'..." (defvar eshell-output-handle) ;Defined in esh-io.el. (defvar eshell-error-handle) ;Defined in esh-io.el. -(defmacro eshell-with-copied-handles (object) - "Duplicate current I/O handles, so OBJECT works with its own copy." +(defmacro eshell-with-copied-handles (object &optional steal-p) + "Duplicate current I/O handles, so OBJECT works with its own copy. +If STEAL-P is non-nil, these new handles will be stolen from the +current ones (see `eshell-duplicate-handles')." `(let ((eshell-current-handles - (eshell-duplicate-handles eshell-current-handles))) + (eshell-duplicate-handles eshell-current-handles ,steal-p))) ,object)) (define-obsolete-function-alias 'eshell-copy-handles @@ -836,7 +837,9 @@ This macro calls itself recursively, with NOTFIRST non-nil." (let ((proc ,(car pipeline))) (set headproc (or proc (symbol-value headproc))) (set tailproc (or (symbol-value tailproc) proc)) - proc)))))) + proc))) + ;; Steal handles if this is the last item in the pipeline. + ,(null (cdr pipeline))))) (defmacro eshell-do-pipelines-synchronously (pipeline) "Execute the commands in PIPELINE in sequence synchronously. @@ -1024,7 +1027,9 @@ produced by `eshell-parse-command'." ;; We can just stick the new command at the end of the current ;; one, and everything will happen as it should. (setcdr (last (cdr eshell-current-command)) - (list `(let ((here (and (eobp) (point)))) + (list `(let ((here (and (eobp) (point))) + (eshell-command-body '(nil)) + (eshell-test-body '(nil))) ,(and input `(insert-and-inherit ,(concat input "\n"))) (if here -- cgit v1.2.1