From 53dfcd232748fdf7f713307d3851c7e9031c9e9c Mon Sep 17 00:00:00 2001 From: Jim Porter Date: Sun, 9 Jun 2024 13:17:53 -0700 Subject: [PATCH] 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) --- lisp/eshell/esh-arg.el | 23 +++++++--------- lisp/eshell/esh-cmd.el | 44 +++++++++++++++++++------------ test/lisp/eshell/esh-cmd-tests.el | 10 +++++++ 3 files changed, 46 insertions(+), 31 deletions(-) diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el index 78cf28d785a..865c0cdf5f1 100644 --- 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 ee9143db765..aaae19df5d6 100644 --- 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 4d27f6db2ee..865a1aadcd4 100644 --- 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." ;; 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 -- 2.39.2