bug#30725: eshell: built-ins do not handle command substitution

[Jim Porter <jporterbugs@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 175 bytes --]

On 1/18/2022 12:33 AM, Michael Albinus wrote:
> (skip-unless (executable-find "echo"))

Oh, right. I'd forgotten about `skip-unless'. Here's a patch with that 
added. Thanks.

[-- Attachment #2: 0001-Consider-subcommands-when-deciding-to-invoke-Eshell-.patch --]
[-- Type: text/plain, Size: 5853 bytes --]

From 5f2e3b12fd018ff64e4c8000d2e6fe293532e188 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Tue, 18 Jan 2022 10:04:22 -0800
Subject: [PATCH] Consider subcommands when deciding to invoke Eshell command
 directly

When an Eshell command contains an asynchronous subcommand (such as
calling an external process), it must be evaluated iteratively.  See
bug#30725.

* lisp/eshell/esh-cmd.el (eshell-invoke-command): Move most of the
logic from here...
(eshell--invoke-command-directly): ... to here. Also add checks for
subcommands.

* test/lisp/eshell/eshell-tests.el (eshell-test--max-subprocess-time):
New variable.
(eshell-wait-for-subprocess): New function.
(eshell-command-result-p): Use 'eshell-wait-for-subprocess'.
(eshell-test/interp-cmd-external): New test.
---
 lisp/eshell/esh-cmd.el           | 57 ++++++++++++++++++++++++--------
 test/lisp/eshell/eshell-tests.el | 23 +++++++++++++
 2 files changed, 66 insertions(+), 14 deletions(-)

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index a2d7d9431a..25e3a5a205 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -903,21 +903,50 @@ pcomplete/eshell-mode/eshell-debug
   "Completion for the `debug' command."
   (while (pcomplete-here '("errors" "commands"))))
 
+(defun eshell--invoke-command-directly (command)
+  "Determine whether the given COMMAND can be invoked directly.
+COMMAND should be a non-top-level Eshell command in parsed form.
+
+A command can be invoked directly if all of the following are true:
+
+* The command is of the form
+  \"(eshell-trap-errors (eshell-named-command NAME ARGS))\",
+  where ARGS is optional.
+
+* NAME is a string referring to an alias function and isn't a
+  complex command (see `eshell-complex-commands').
+
+* Any argument in ARGS that calls a subcommand can also be
+  invoked directly."
+  (when (and (eq (car command) 'eshell-trap-errors)
+             (eq (car (cadr command)) 'eshell-named-command))
+    (let ((name (cadr (cadr command)))
+          (args (cdr-safe (nth 2 (cadr command)))))
+      (and name (stringp name)
+	   (not (member name eshell-complex-commands))
+	   (catch 'simple
+	     (dolist (pred eshell-complex-commands t)
+	       (when (and (functionp pred)
+		          (funcall pred name))
+	         (throw 'simple nil))))
+	   (eshell-find-alias-function name)
+           (catch 'indirect-subcommand
+	     (dolist (arg args t)
+               (pcase arg
+                 (`(eshell-escape-arg
+                    (let ,_
+                      (eshell-convert
+                       (eshell-command-to-value
+                        (eshell-as-subcommand ,subcommand)))))
+                  (unless (eshell--invoke-command-directly subcommand)
+                    (throw 'indirect-subcommand nil))))))))))
+
 (defun eshell-invoke-directly (command)
-  (let ((base (cadr (nth 2 (nth 2 (cadr command))))) name)
-    (if (and (eq (car base) 'eshell-trap-errors)
-	     (eq (car (cadr base)) 'eshell-named-command))
-	(setq name (cadr (cadr base))))
-    (and name (stringp name)
-	 (not (member name eshell-complex-commands))
-	 (catch 'simple
-	   (progn
-	    (dolist (pred eshell-complex-commands)
-	      (if (and (functionp pred)
-		       (funcall pred name))
-		  (throw 'simple nil)))
-	    t))
-	 (eshell-find-alias-function name))))
+  "Determine whether the given COMMAND can be invoked directly.
+COMMAND should be a top-level Eshell command in parsed form, as
+produced by `eshell-parse-command'."
+  (let ((base (cadr (nth 2 (nth 2 (cadr command))))))
+    (eshell--invoke-command-directly base)))
 
 (defun eshell-eval-command (command &optional input)
   "Evaluate the given COMMAND iteratively."
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index aef1447907..c4cb9bf485 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -30,6 +30,10 @@
 (require 'esh-mode)
 (require 'eshell)
 
+(defvar eshell-test--max-subprocess-time 5
+  "The maximum amount of time to wait for a subprocess to finish, in seconds.
+See `eshell-wait-for-subprocess'.")
+
 (defmacro with-temp-eshell (&rest body)
   "Evaluate BODY in a temporary Eshell buffer."
   `(ert-with-temp-directory eshell-directory-name
@@ -44,6 +48,17 @@ with-temp-eshell
          (let (kill-buffer-query-functions)
            (kill-buffer eshell-buffer))))))
 
+(defun eshell-wait-for-subprocess ()
+  "Wait until there is no interactive subprocess running in Eshell.
+If this takes longer than `eshell-test--max-subprocess-time',
+raise an error."
+  (let ((start (current-time)))
+    (while (eshell-interactive-process)
+      (when (> (float-time (time-since start))
+               eshell-test--max-subprocess-time)
+        (error "timed out waiting for subprocess"))
+      (sit-for 0.1))))
+
 (defun eshell-insert-command (text &optional func)
   "Insert a command at the end of the buffer."
   (goto-char eshell-last-output-end)
@@ -59,6 +74,7 @@ eshell-match-result
 (defun eshell-command-result-p (text regexp &optional func)
   "Insert a command at the end of the buffer."
   (eshell-insert-command text func)
+  (eshell-wait-for-subprocess)
   (eshell-match-result regexp))
 
 (defvar eshell-history-file-name)
@@ -144,6 +160,13 @@ eshell-test/interp-concat-lisp2
   "Interpolate and concat two Lisp forms"
   (should (equal (eshell-test-command-result "+ $(+ 1 2)$(+ 1 2) 3") 36)))
 
+(ert-deftest eshell-test/interp-cmd-external ()
+  "Interpolate command result from external command"
+  (skip-unless (executable-find "echo"))
+  (with-temp-eshell
+   (eshell-command-result-p "echo ${*echo hi}"
+                            "hi\n")))
+
 (ert-deftest eshell-test/window-height ()
   "$LINES should equal (window-height)"
   (should (eshell-test-command-result "= $LINES (window-height)")))
-- 
2.25.1