Fix detection of directly-invokable commands in Eshell

I think this regressed partly due to eef32d13da58, so let's add some
regression tests to make sure that doesn't happen again.

* lisp/eshell/em-unix.el (eshell-unix-initialize): Add "compile".

* lisp/eshell/esh-cmd.el (eshell--find-subcommands): Yield the second
element of the subcommand.
(eshell--invoke-command-directly-p): Rename and account for
'eshell-with-copied-handles'.
(eshell-invoke-directly): Rename to...
(eshell-invoke-directly-p): ... this, and use 'pcase' to make the
logic clearer.

* lisp/eshell/esh-mode.el (eshell-send-input): Always queue input if
the process is running; rename some locals to be clearer.

* lisp/eshell/esh-var.el (eshell-var-initialize): Add "env" as a
complex command.

* test/lisp/eshell/esh-cmd-tests.el
(esh-cmd-test--deftest-invoke-directly): New macro.
(no-args, with-args, multiple-cmds, subcmd, complex, complex-subcmd):
New test cases.

5 files changed, 63 insertions, 36 deletions

diff --git a/lisp/eshell/em-unix.el b/lisp/eshell/em-unix.el
index b066e9eeb8e..dad02206759 100644
--- a/lisp/eshell/em-unix.el
+++ b/lisp/eshell/em-unix.el
@@ -166,9 +166,9 @@ Otherwise, Emacs will attempt to use rsh to invoke du on the remote machine."
     (add-hook 'pcomplete-try-first-hook
               'eshell-complete-host-reference nil t))
   (setq-local eshell-complex-commands
-              (append '("grep" "egrep" "fgrep" "agrep" "rgrep"
-                        "glimpse" "locate" "cat" "time" "cp" "mv"
-                        "make" "du" "diff")
+              (append '("compile" "grep" "egrep" "fgrep" "agrep"
+                        "rgrep" "glimpse" "locate" "cat" "time" "cp"
+                        "mv" "make" "du" "diff")
                       eshell-complex-commands)))
 
 (defalias 'eshell/date     'current-time-string)
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 2746800ea78..30494bafb48 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -934,48 +934,52 @@ This yields the SUBCOMMANDs when found in forms like
   (dolist (elem haystack)
     (cond
      ((eq (car-safe elem) 'eshell-as-subcommand)
-      (iter-yield (cdr elem)))
+      (iter-yield (cadr elem)))
      ((listp elem)
       (iter-yield-from (eshell--find-subcommands elem))))))
 
-(defun eshell--invoke-command-directly (command)
+(defun eshell--invoke-command-directly-p (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.
+  (eshell-with-copied-handles
+   (eshell-trap-errors (eshell-named-command NAME [ARGS])) _).
 
 * NAME is a string referring to an alias function and isn't a
   complex command (see `eshell-complex-commands').
 
 * Any subcommands in ARGS 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
-             (iter-do (subcommand (eshell--find-subcommands args))
-               (unless (eshell--invoke-command-directly subcommand)
-                 (throw 'indirect-subcommand nil)))
-             t)))))
-
-(defun eshell-invoke-directly (command)
+  (pcase command
+    (`(eshell-with-copied-handles
+       (eshell-trap-errors (eshell-named-command ,name . ,args))
+       ,_)
+     (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
+            (iter-do (subcommand (eshell--find-subcommands (car args)))
+              (unless (eshell--invoke-command-directly-p subcommand)
+                (throw 'indirect-subcommand nil)))
+            t)))))
+
+(defun eshell-invoke-directly-p (command)
   "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)))
+  (pcase command
+    (`(eshell-commands (progn ,_ (unwind-protect (progn ,base) . ,_)))
+     (eshell--invoke-command-directly-p base))))
+
+(define-obsolete-function-alias 'eshell-invoke-directly
+  'eshell-invoke-directly-p "30.1")
 
 (defun eshell-eval-argument (argument)
   "Evaluate a single Eshell ARGUMENT and return the result."
diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index 21e3f00086f..fd279f61673 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -619,14 +619,14 @@ If NO-NEWLINE is non-nil, the input is sent without an implied final
 newline."
   (interactive "P")
   ;; Note that the input string does not include its terminal newline.
-  (let ((proc-running-p (and (eshell-head-process)
-                             (not queue-p)))
-        (inhibit-modification-hooks t))
-    (unless (and proc-running-p
+  (let* ((proc-running-p (eshell-head-process))
+         (send-to-process-p (and proc-running-p (not queue-p)))
+         (inhibit-modification-hooks t))
+    (unless (and send-to-process-p
                  (not (eq (process-status
                            (eshell-head-process))
                           'run)))
-      (if (or proc-running-p
+      (if (or send-to-process-p
               (>= (point) eshell-last-output-end))
           (goto-char (point-max))
         (let ((copy (eshell-get-old-input use-region)))
@@ -634,7 +634,7 @@ newline."
           (insert-and-inherit copy)))
       (unless (or no-newline
                   (and eshell-send-direct-to-subprocesses
-                       proc-running-p))
+                       send-to-process-p))
         (insert-before-markers-and-inherit ?\n))
       ;; Delete and reinsert input.  This seems like a no-op, except
       ;; for the resulting entries in the undo list: undoing this
@@ -644,7 +644,7 @@ newline."
             (inhibit-read-only t))
         (delete-region eshell-last-output-end (point))
         (insert text))
-      (if proc-running-p
+      (if send-to-process-p
           (progn
             (eshell-update-markers eshell-last-output-end)
             (if (or eshell-send-direct-to-subprocesses
@@ -673,7 +673,8 @@ newline."
                       (run-hooks 'eshell-input-filter-functions)
                       (and (catch 'eshell-terminal
                              (ignore
-                              (if (eshell-invoke-directly cmd)
+                              (if (and (not proc-running-p)
+                                       (eshell-invoke-directly-p cmd))
                                   (eval cmd)
                                 (eshell-eval-command cmd input))))
                            (eshell-life-is-too-much)))))
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 627cbb17797..537bc4b0641 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -287,6 +287,8 @@ This is set to t in `eshell-local-variable-bindings' (which see).")
   (setq-local eshell-subcommand-bindings
               (append eshell-local-variable-bindings
                       eshell-subcommand-bindings))
+  (setq-local eshell-complex-commands
+              (append '("env") eshell-complex-commands))
 
   (setq-local eshell-special-chars-inside-quoting
        (append eshell-special-chars-inside-quoting '(?$)))
diff --git a/test/lisp/eshell/esh-cmd-tests.el b/test/lisp/eshell/esh-cmd-tests.el
index be31681267b..c37e6d14187 100644
--- a/test/lisp/eshell/esh-cmd-tests.el
+++ b/test/lisp/eshell/esh-cmd-tests.el
@@ -469,6 +469,26 @@ This tests when `eshell-lisp-form-nil-is-failure' is nil."
                                "no"))
 
 
+;; Direct invocation
+
+(defmacro esh-cmd-test--deftest-invoke-directly (name command expected)
+  "FIXME"
+  (declare (indent 2))
+  `(ert-deftest ,(intern (concat "esh-cmd-test/invoke-directly/"
+                                 (symbol-name name))) ()
+     (with-temp-eshell
+      (should (equal (eshell-invoke-directly
+                      (eshell-parse-command ,command nil t))
+                     ,expected)))))
+
+(esh-cmd-test--deftest-invoke-directly no-args "echo" t)
+(esh-cmd-test--deftest-invoke-directly with-args "echo hi" t)
+(esh-cmd-test--deftest-invoke-directly multiple-cmds "echo hi; echo bye" nil)
+(esh-cmd-test--deftest-invoke-directly subcmd "echo ${echo hi}" t)
+(esh-cmd-test--deftest-invoke-directly complex "ls ." nil)
+(esh-cmd-test--deftest-invoke-directly complex-subcmd "echo {ls .}" nil)
+
+
 ;; Error handling
 
 (ert-deftest esh-cmd-test/throw ()