From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 20 Jan 2022 13:37:54 +0000 (+0100)
Subject: Consider subcommands when deciding to invoke Eshell command directly
X-Git-Tag: emacs-29.0.90~2897
X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=4450c8bdd93d1b2e7f276e26be2cc37372034c22;p=emacs.git

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 (bug#30725).
---

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index a2d7d9431a9..25e3a5a2054 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -903,21 +903,50 @@ at the moment are:
   "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 aef14479078..c4cb9bf4850 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 @@
          (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 @@
 (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 @@ e.g. \"{(+ 1 2)} 3\" => 3"
   "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)")))