Remove empty Eshell commands when parsing

This improves the logic for copying/stealing handles when constructing
the command form: now, we should always steal the handles for the last
real command, even if there were some trailing semicolons.

* lisp/eshell/esh-arg.el (eshell-parse-delimiter): Be stricter about
parsing so that things like "& &" aren't parsed as a single "&&" token.

* lisp/eshell/esh-cmd.el (eshell-parse-command): Get the commands in
reverse, and remove any nil commands.
(eshell-split-commands): Always return the trailing terms (except when
there were no terms to begin with).

* test/lisp/eshell/esh-cmd-tests.el
(esh-cmd-test/empty-background-command): New test.

(cherry picked from commit 00649042f3057c9ea2e6a4944924293998e2a527)

diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index 78cf28d785a8dc2edbb9181dca8ca949dd936fdf..865c0cdf5f1fc14576365c6968bbf9b8100701e7 100644 (file)
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -545,22 +545,17 @@ leaves point where it was."
   ;; this `eshell-operator' keyword gets parsed out by
   ;; `eshell-split-commands'.  Right now the only possibility for
   ;; error is an incorrect output redirection specifier.
-  (when (looking-at "[&|;\n]\\s-*")
-    (let ((end (match-end 0)))
+  (when (looking-at (rx (group (or "&" "|" ";" "\n" "&&" "||"))
+                        (* (syntax whitespace))))
     (if eshell-current-argument
        (eshell-finish-arg)
-      (eshell-finish-arg
-       (prog1
-          (list 'eshell-operator
-                (cond
-                 ((eq (char-after end) ?\&)
-                  (setq end (1+ end)) "&&")
-                 ((eq (char-after end) ?\|)
-                  (setq end (1+ end)) "||")
-                 ((eq (char-after) ?\n) ";")
-                 (t
-                  (char-to-string (char-after)))))
-        (goto-char end)))))))
+      (let ((operator (match-string 1)))
+        (when (string= operator "\n")
+          (setq operator ";"))
+        (eshell-finish-arg
+         (prog1
+            `(eshell-operator ,operator)
+          (goto-char (match-end 0))))))))
 
 (defun eshell-prepare-splice (args)
   "Prepare a list of ARGS for splicing, if any arg requested a splice.

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index ee9143db765558ad4d828429f8335ade7852db0b..aaae19df5d675fdd7880e1d2057e94a6ee696cee 100644 (file)
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -410,24 +410,34 @@ command hooks should be run before and after the command."
            (goto-char (point-max))
            (eshell-parse-arguments (point-min) (point-max))))
        args))
+     ;; Split up our commands in reverse order.
      (`(,sub-chains . ,sep-terms)
-      (eshell-split-commands terms "[&;]" nil t))
+      (eshell-split-commands terms "[&;]" t t))
+     ;; The last command (first in our reversed list) is implicitly
+     ;; terminated by ";".
+     (sep-terms (cons ";" sep-terms))
+     (steal-handles t)
      (commands
-      (mapcar
-       (lambda (cmd)
-         (let ((sep (pop sep-terms)))
-           (setq cmd (eshell-parse-pipeline cmd))
-           (unless eshell-in-pipeline-p
-             (setq cmd `(eshell-trap-errors ,cmd)))
-           ;; Copy I/O handles so each full statement can manipulate
-           ;; them if they like.  Steal the handles for the last
-           ;; command in the list; we won't use the originals again
-           ;; anyway.
-           (setq cmd `(eshell-with-copied-handles ,cmd ,(not sep)))
-           (when (equal sep "&")
-             (setq cmd `(eshell-do-subjob ,cmd)))
-           cmd))
-       sub-chains)))
+      (nreverse
+       (mapcan
+        (lambda (cmd)
+          (let ((sep (pop sep-terms)))
+            (if (null cmd)
+                (when (equal sep "&")
+                  (error "Empty command before `&'"))
+              (setq cmd (eshell-parse-pipeline cmd))
+              (unless eshell-in-pipeline-p
+                (setq cmd `(eshell-trap-errors ,cmd)))
+              ;; Copy I/O handles so each full statement can manipulate
+              ;; them if they like.  Steal the handles for the last
+              ;; command (first in our reversed list); we won't use the
+              ;; originals again anyway.
+              (setq cmd `(eshell-with-copied-handles ,cmd ,steal-handles)
+                    steal-handles nil)
+              (when (equal sep "&")
+                (setq cmd `(eshell-do-subjob ,cmd)))
+              (list cmd))))
+        sub-chains))))
     (if toplevel
        `(eshell-commands (progn
                             (run-hooks 'eshell-pre-command-hook)
@@ -703,7 +713,7 @@ as a pair of lists."
             (push (nreverse sub-terms) sub-chains)
             (setq sub-terms nil))
         (push term sub-terms)))
-    (when sub-terms
+    (when terms
       (push (nreverse sub-terms) sub-chains))
     (unless reversed
       (setq sub-chains (nreverse sub-chains)

diff --git a/test/lisp/eshell/esh-cmd-tests.el b/test/lisp/eshell/esh-cmd-tests.el
index 4d27f6db2ee447a9354970cd8a7ed5729138ae3a..865a1aadcd4e845600d8a0d6046074e424ea6531 100644 (file)
--- a/test/lisp/eshell/esh-cmd-tests.el
+++ b/test/lisp/eshell/esh-cmd-tests.el
@@ -505,6 +505,16 @@ NAME is the name of the test case."
 \f
 ;; Error handling
 
+(ert-deftest esh-cmd-test/empty-background-command ()
+  "Test that Eshell reports an error when trying to background a nil command."
+  (with-temp-eshell
+    (eshell-match-command-output "echo hi & &"
+                                 "\\`Empty command before `&'\n")
+    ;; Make sure the next Eshell prompt has the original input so the
+    ;; user can fix it.
+    (should (equal (buffer-substring eshell-last-output-end (point))
+                   "echo hi & &"))))
+
 (ert-deftest esh-cmd-test/throw ()
   "Test that calling `throw' as an Eshell command unwinds everything properly."
   (with-temp-eshell