Subject: 31.0.50; [PATCH] Improve correctness of Eshell conditional forms and allow if/else chaining

From: Jim Porter <[email-masked]>
Date: Sat, 5 Oct 2024 18:34:24 -0700

These patches improve a few inconsistencies in how Eshell conditionals 
work, especially when using Lisp forms inside of the command-form of 
"if". For example, before this patch, the following command outputs 
nothing at all:

   if (zerop 0) (identity \"yes\") (identity \"no\")

With the patches, it now correctly outputs "yes".

I also added the ability to use the "else" keyword so that you can chain 
together if/else blocks like so:

   if {[ -f file.txt ]} {
     echo found file
   } else if {[ -f alternate.txt ]} {
     echo found alternate
   } else {
     echo not found!
   }

From dbd223d4cfbdb07ed87c7f497c2ba691a112a16c Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Tue, 17 Sep 2024 22:05:50 -0700
Subject: [PATCH 1/3] Simplify creation of Eshell command forms

Previously, Eshell over-aggressively converted subcommands, which forced
control flow commands to undo that change.

* lisp/eshell/esh-cmd.el (eshell-pre-rewrite-command-hook): Remove
'eshell-subcommand-arg-values' call from here...
(eshell-rewrite-named-command): ... and put it here.
(eshell-invokify-arg): Make obsolete.
(eshell-structure-basic-command): Handle subcommand test forms.
(eshell-rewrite-for-command, eshell-rewrite-while-command)
(eshell-rewrite-if-command): Don't call 'eshell-invokify-arg'; it's not
necessary.
---
 lisp/eshell/esh-cmd.el | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 09fc65522ad..2a299125f22 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -181,8 +181,7 @@ eshell-named-command-hook
   :type 'hook)
 
 (defcustom eshell-pre-rewrite-command-hook
-  '(eshell-no-command-conversion
-    eshell-subcommand-arg-values)
+  '(eshell-no-command-conversion)
   "A hook run before command rewriting begins.
 The terms of the command to be rewritten is passed as arguments, and
 may be modified in place.  Any return value is ignored."
@@ -478,6 +477,7 @@ eshell-rewrite-initial-subcommand
 
 (defun eshell-rewrite-named-command (terms)
   "If no other rewriting rule transforms TERMS, assume a named command."
+  (eshell-subcommand-arg-values terms)
   (let ((sym (if eshell-in-pipeline-p
 		 'eshell-named-command*
 	       'eshell-named-command))
@@ -503,6 +503,7 @@ eshell-invokify-arg
 means the user and/or any redirections shouldn't see any output
 from this command.  If both SHARE-OUTPUT and SILENT are non-nil,
 the second is ignored."
+  (declare (obsolete nil "31.1"))
   ;; something that begins with `eshell-convert' means that it
   ;; intends to return a Lisp value.  We want to get past this,
   ;; but if it's not _actually_ a value interpolation -- in which
@@ -543,7 +544,7 @@ eshell-rewrite-for-command
              (let ((,(intern (cadr terms)) (car ,for-items))
 		   (eshell--local-vars (cons ',(intern (cadr terms))
                                              eshell--local-vars)))
-	       ,(eshell-invokify-arg body t))
+	       ,body)
              (setq ,for-items (cdr ,for-items)))))))
 
 (defun eshell-structure-basic-command (func names keyword test body
@@ -552,6 +553,11 @@ eshell-structure-basic-command
 The first of NAMES should be the positive form, and the second the
 negative.  It's not likely that users should ever need to call this
 function."
+  ;; If the test form is a subcommand, wrap it in `eshell-commands' to
+  ;; silence the output.
+  (when (eq (car test) 'eshell-as-subcommand)
+    (setq test `(eshell-commands ,test t)))
+
   ;; If the test form begins with `eshell-convert' or
   ;; `eshell-escape-arg', it means something data-wise will be
   ;; returned, and we should let that determine the truth of the
@@ -583,8 +589,8 @@ eshell-rewrite-while-command
 	   (member (car terms) '("while" "until")))
       (eshell-structure-basic-command
        'while '("while" "until") (car terms)
-       (eshell-invokify-arg (cadr terms) nil t)
-       (eshell-invokify-arg (car (last terms)) t))))
+       (cadr terms)
+       (car (last terms)))))
 
 (defun eshell-rewrite-if-command (terms)
   "Rewrite an `if' command into its equivalent Eshell command form.
@@ -595,10 +601,10 @@ eshell-rewrite-if-command
 	   (member (car terms) '("if" "unless")))
       (eshell-structure-basic-command
        'if '("if" "unless") (car terms)
-       (eshell-invokify-arg (cadr terms) nil t)
-       (eshell-invokify-arg (car (last terms (if (= (length terms) 4) 2))) t)
+       (cadr terms)
+       (car (last terms (if (= (length terms) 4) 2)))
        (when (= (length terms) 4)
-         (eshell-invokify-arg (car (last terms)) t)))))
+         (car (last terms))))))
 
 (defun eshell-set-exit-info (status &optional result)
   "Set the exit status and result for the last command.
-- 
2.25.1

From 15dcc7505b5edc1f6039b0d57ec0033748062a93 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Fri, 4 Oct 2024 21:45:04 -0700
Subject: [PATCH 2/3] Improve correctness of Eshell sub-forms

This makes sure that we treat Eshell sub-forms (whether Lisp or command
forms) as values when appropriate, or as regular invocations.  This
requires a bit more explicit work, but helps to resolve some of the
surprising differences between Lisp and command forms in complex Eshell
statements.

* lisp/eshell/esh-cmd.el (eshell-subcommand-arg-values): Make obsolete.
(eshell-parse-lisp-argument): Don't add 'eshell-command-to-value' here.
(eshell-rewrite-sexp-command): Don't check for 'eshell-command-to-value
here'; instead check for 'eshell-lisp-command'.
(eshell-structure-basic-command): Check for 'eshell-lisp-command'.
(eshell-term-as-value): New function...
(eshell-rewrite-named-command, eshell-rewrite-for-command): ... call it.

* lisp/eshell/esh-arg.el (eshell-parse-special-reference):
* lisp/eshell/esh-io.el (eshell-strip-redirections):
* lisp/eshell/esh-var.el (eshell-prepare-indices): Call
'eshell-term-as-value'.

* test/lisp/eshell/esh-arg-tests.el
(esh-arg-test/special-reference/command-form):
* test/lisp/eshell/esh-cmd-tests.el (esh-cmd-test/for-loop-lisp-body)
(esh-cmd-test/while-loop-lisp-body)
(esh-cmd-test/if-else-statement-lisp-body): New tests.

* test/lisp/eshell/esh-var-tests.el
(esh-var-test/interp-var-indices-subcommand): Add another command to
test.

* doc/misc/eshell.texi (Control Flow): Update documentation.
---
 doc/misc/eshell.texi              | 43 ++++++++++++------------
 lisp/eshell/esh-arg.el            |  5 ++-
 lisp/eshell/esh-cmd.el            | 54 +++++++++++++++++++------------
 lisp/eshell/esh-io.el             |  5 +--
 lisp/eshell/esh-var.el            |  4 ++-
 test/lisp/eshell/esh-arg-tests.el | 13 ++++++++
 test/lisp/eshell/esh-cmd-tests.el | 26 +++++++++++++++
 test/lisp/eshell/esh-var-tests.el |  3 ++
 8 files changed, 108 insertions(+), 45 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index f8abb3d860a..bbb6b2e6aac 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1689,36 +1689,39 @@ Control Flow
 
 Most of Eshell's control flow statements accept a @var{conditional}.
 This can take a few different forms.  If @var{conditional} is a dollar
-expansion, the condition is satisfied if the result is a
-non-@code{nil} value.  If @var{conditional} is a @samp{@{
-@var{subcommand} @}} or @samp{(@var{lisp form})}, the condition is
-satisfied if the command's exit status is 0.
+expansion, the condition is satisfied if the result is a non-@code{nil}
+value.  Alternately, @var{conditional} may be a subcommand, either in
+command form, e.g.@: @samp{@{@var{subcommand}@}}; or in Lisp form,
+e.g.@: @samp{(@var{lisp form})}.  In that case, the condition is
+satisfied if the subcommand's exit status is 0.
 
 @table @code
 
-@item if @var{conditional} @{ @var{true-commands} @}
-@itemx if @var{conditional} @{ @var{true-commands} @} @{ @var{false-commands} @}
-Evaluate @var{true-commands} if @var{conditional} is satisfied;
-otherwise, evaluate @var{false-commands}.
+@item if @var{conditional} @var{true-subcommand}
+@itemx if @var{conditional} @var{true-subcommand} @var{false-subcommand}
+Evaluate @var{true-subcommand} if @var{conditional} is satisfied;
+otherwise, evaluate @var{false-subcommand}.  Both @var{true-subcommand}
+and @var{false-subcommand} should be subcommands, as with
+@var{conditional}.
 
-@item unless @var{conditional} @{ @var{false-commands} @}
-@itemx unless @var{conditional} @{ @var{false-commands} @} @{ @var{true-commands} @}
-Evaluate @var{false-commands} if @var{conditional} is not satisfied;
-otherwise, evaluate @var{true-commands}.
+@item unless @var{conditional} @var{false-subcommand}
+@itemx unless @var{conditional} @var{false-subcommand} @var{true-subcommand}
+Evaluate @var{false-subcommand} if @var{conditional} is not satisfied;
+otherwise, evaluate @var{true-subcommand}.
 
-@item while @var{conditional} @{ @var{commands} @}
-Repeatedly evaluate @var{commands} so long as @var{conditional} is
+@item while @var{conditional} @var{subcommand}
+Repeatedly evaluate @var{subcommand} so long as @var{conditional} is
 satisfied.
 
-@item until @var{conditional} @{ @var{commands} @}
-Repeatedly evaluate @var{commands} until @var{conditional} is
+@item until @var{conditional} @var{subcommand}
+Repeatedly evaluate @var{subcommand} until @var{conditional} is
 satisfied.
 
-@item for @var{var} in @var{list}@dots{} @{ @var{commands} @}
+@item for @var{var} in @var{list}@dots{} @var{subcommand}
 Iterate over each element of @var{list}, storing the element in
-@var{var} and evaluating @var{commands}.  If @var{list} is not a list,
-treat it as a list of one element.  If you specify multiple
-@var{lists}, this will iterate over each of them in turn.
+@var{var} and evaluating @var{subcommand}.  If @var{list} is not a list,
+treat it as a list of one element.  If you specify multiple @var{lists},
+this will iterate over each of them in turn.
 
 @end table
 
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index 6fc700cce89..b441cbfc274 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -35,6 +35,8 @@
 (eval-when-compile
   (require 'cl-lib))
 
+(declare-function eshell-term-as-value "esh-cmd" (term))
+
 (defgroup eshell-arg nil
   "Argument parsing involves transforming the arguments passed on the
 command line into equivalent Lisp forms that, when evaluated, will
@@ -626,7 +628,8 @@ eshell-parse-special-reference
             (prog1
                 (cons creation-fun
                       (let ((eshell-current-argument-plain t))
-                        (eshell-parse-arguments (point) end)))
+                        (mapcar #'eshell-term-as-value
+                                (eshell-parse-arguments (point) end))))
               (goto-char (1+ end)))
           (ignore (goto-char here)))))))
 
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 2a299125f22..65f997e5b88 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -454,6 +454,7 @@ eshell-no-command-conversion
 
 (defun eshell-subcommand-arg-values (terms)
   "Convert subcommand arguments {x} to ${x}, in order to take their values."
+  (declare (obsolete nil "31.1"))
   (setq terms (cdr terms))		; skip command argument
   (while terms
     (if (and (listp (car terms))
@@ -465,9 +466,9 @@ eshell-subcommand-arg-values
 (defun eshell-rewrite-sexp-command (terms)
   "Rewrite a sexp in initial position, such as `(+ 1 2)'."
   ;; this occurs when a Lisp expression is in first position
-  (if (and (listp (car terms))
-	   (eq (caar terms) 'eshell-command-to-value))
-      (car (cdar terms))))
+  (when (and (listp (car terms))
+             (eq (caar terms) 'eshell-lisp-command))
+    (car terms)))
 
 (defun eshell-rewrite-initial-subcommand (terms)
   "Rewrite a subcommand in initial position, such as `{+ 1 2}'."
@@ -477,20 +478,23 @@ eshell-rewrite-initial-subcommand
 
 (defun eshell-rewrite-named-command (terms)
   "If no other rewriting rule transforms TERMS, assume a named command."
-  (eshell-subcommand-arg-values terms)
-  (let ((sym (if eshell-in-pipeline-p
-		 'eshell-named-command*
-	       'eshell-named-command))
-        (grouped-terms (eshell-prepare-splice terms)))
-    (cond
-     (grouped-terms
-      `(let ((terms (nconc ,@grouped-terms)))
-         (,sym (car terms) (cdr terms))))
-     ;; If no terms are spliced, use a simpler command form.
-     ((cdr terms)
-      (list sym (car terms) `(list ,@(cdr terms))))
-     (t
-      (list sym (car terms))))))
+  (when terms
+    (setq terms (cons (car terms)
+                      ;; Convert arguments to take their values.
+                      (mapcar #'eshell-term-as-value (cdr terms))))
+    (let ((sym (if eshell-in-pipeline-p
+		   'eshell-named-command*
+	         'eshell-named-command))
+          (grouped-terms (eshell-prepare-splice terms)))
+      (cond
+       (grouped-terms
+        `(let ((new-terms (nconc ,@grouped-terms)))
+           (,sym (car new-terms) (cdr new-terms))))
+       ;; If no terms are spliced, use a simpler command form.
+       ((cdr terms)
+        (list sym (car terms) `(list ,@(cdr terms))))
+       (t
+        (list sym (car terms)))))))
 
 (defvar eshell--command-body)
 (defvar eshell--test-body)
@@ -537,7 +541,7 @@ eshell-rewrite-for-command
                  ,@(mapcar
                     (lambda (elem)
                       (if (listp elem)
-                          elem
+                          (eshell-term-as-value elem)
                         `(list ,elem)))
                     (nthcdr 3 terms)))))
            (while ,for-items
@@ -555,7 +559,7 @@ eshell-structure-basic-command
 function."
   ;; If the test form is a subcommand, wrap it in `eshell-commands' to
   ;; silence the output.
-  (when (eq (car test) 'eshell-as-subcommand)
+  (when (memq (car test) '(eshell-as-subcommand eshell-lisp-command))
     (setq test `(eshell-commands ,test t)))
 
   ;; If the test form begins with `eshell-convert' or
@@ -686,8 +690,7 @@ eshell-parse-lisp-argument
 		(end-of-file
                  (throw 'eshell-incomplete "(")))))
 	(if (eshell-arg-delimiter)
-	    `(eshell-command-to-value
-              (eshell-lisp-command (quote ,obj)))
+	    `(eshell-lisp-command (quote ,obj))
 	  (ignore (goto-char here))))))
 
 (defun eshell-split-commands (terms separator &optional
@@ -912,6 +915,15 @@ eshell-command-to-value
          ,command
          ,value))))
 
+(defun eshell-term-as-value (term)
+  "Convert an Eshell TERM to take its value."
+  (cond
+   ((eq (car-safe term) 'eshell-as-subcommand) ; {x} -> ${x}
+    `(eshell-convert (eshell-command-to-value ,term)))
+   ((eq (car-safe term) 'eshell-lisp-command)  ; (x) -> $(x)
+    `(eshell-command-to-value ,term))
+   (t term)))
+
 ;;;_* Iterative evaluation
 ;;
 ;; Eshell runs all of its external commands asynchronously, so that
diff --git a/lisp/eshell/esh-io.el b/lisp/eshell/esh-io.el
index feb4bf8959f..443c39ff0d1 100644
--- a/lisp/eshell/esh-io.el
+++ b/lisp/eshell/esh-io.el
@@ -75,6 +75,7 @@
   (require 'cl-lib))
 
 (declare-function eshell-interactive-print "esh-mode" (string))
+(declare-function eshell-term-as-value "esh-cmd" (term))
 
 (defgroup eshell-io nil
   "Eshell's I/O management code provides a scheme for treating many
@@ -301,8 +302,8 @@ eshell-strip-redirections
         (unless (cdr tt)
           (error "Missing redirection target"))
         (nconc eshell-current-redirections
-               (list (list 'ignore
-                           (append (car tt) (list (cadr tt))))))
+               `((ignore ,(append (car tt)
+                                  (list (eshell-term-as-value (cadr tt)))))))
         (setcdr tl (cddr tt))
         (setq tt (cddr tt)))
        (t
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 0e6c01d2774..5b4e7d27882 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -669,7 +669,9 @@ eshell-eval-indices
 (defun eshell-prepare-indices (indices)
   "Prepare INDICES to be evaluated by Eshell.
 INDICES is a list of index-lists generated by `eshell-parse-indices'."
-  `(list ,@(mapcar (lambda (idx-list) (cons 'list idx-list)) indices)))
+  `(list ,@(mapcar (lambda (idx-list)
+                     (cons 'list (mapcar #'eshell-term-as-value idx-list)))
+                   indices)))
 
 (defun eshell-get-variable (name &optional indices quoted)
   "Get the value for the variable NAME.
diff --git a/test/lisp/eshell/esh-arg-tests.el b/test/lisp/eshell/esh-arg-tests.el
index b748c5ab4c0..209c4fa8ea9 100644
--- a/test/lisp/eshell/esh-arg-tests.el
+++ b/test/lisp/eshell/esh-arg-tests.el
@@ -181,6 +181,19 @@ esh-arg-test/special-reference/lisp-form
       "setq eshell-test-value #<marker 1 #<buffer (buffer-name)>>")
      (should (equal eshell-test-value marker)))))
 
+(ert-deftest esh-arg-test/special-reference/command-form ()
+  "Test that command forms inside special references work."
+  (with-temp-eshell
+   (let ((marker (make-marker))
+         eshell-test-value)
+     (set-marker marker 1 (current-buffer))
+     (eshell-insert-command
+      "setq eshell-test-value #<marker 1 {current-buffer}>")
+     (should (equal eshell-test-value marker))
+     (eshell-insert-command
+      "setq eshell-test-value #<marker 1 #<buffer {buffer-name}>>")
+     (should (equal eshell-test-value marker)))))
+
 (ert-deftest esh-arg-test/special-reference/special-characters ()
   "Test that \"#<...>\" works correctly when escaping special characters."
   (with-temp-buffer
diff --git a/test/lisp/eshell/esh-cmd-tests.el b/test/lisp/eshell/esh-cmd-tests.el
index cac349a2616..9e4cbc58201 100644
--- a/test/lisp/eshell/esh-cmd-tests.el
+++ b/test/lisp/eshell/esh-cmd-tests.el
@@ -325,6 +325,12 @@ esh-cmd-test/for-loop-for-items-shadow
    (eshell-match-command-output "for i in 1 { echo $for-items }"
                                 "hello\n")))
 
+(ert-deftest esh-cmd-test/for-loop-lisp-body ()
+  "Test invocation of a for loop with a Lisp body form."
+  (with-temp-eshell
+   (eshell-match-command-output "for i in 1 2 3 (format \"%s\" i)"
+                                "1\n2\n3\n")))
+
 (ert-deftest esh-cmd-test/for-loop-pipe ()
   "Test invocation of a for loop piped to another command."
   (skip-unless (executable-find "rev"))
@@ -350,6 +356,15 @@ esh-cmd-test/while-loop-lisp-form
               "{ setq eshell-test-value (1+ eshell-test-value) }")
       "1\n2\n3\n"))))
 
+(ert-deftest esh-cmd-test/while-loop-lisp-body ()
+  "Test invocation of a while loop using a Lisp form for the body."
+  (with-temp-eshell
+   (let ((eshell-test-value 0))
+     (eshell-match-command-output
+      (concat "while (/= eshell-test-value 3) "
+              "(setq eshell-test-value (1+ eshell-test-value))")
+      "1\n2\n3\n"))))
+
 (ert-deftest esh-cmd-test/while-loop-ext-cmd ()
   "Test invocation of a while loop using an external command."
   (skip-unless (executable-find "["))
@@ -440,6 +455,17 @@ esh-cmd-test/if-else-statement-lisp-form-2
       (eshell-command-result-equal "if (zerop \"foo\") {echo yes} {echo no}"
                                    "no"))))
 
+(ert-deftest esh-cmd-test/if-else-statement-lisp-body ()
+  "Test invocation of an if/else statement using Lisp forms for the bodies."
+  (eshell-command-result-equal "if (zerop 0) (format \"yes\") (format \"no\")"
+                               "yes")
+  (eshell-command-result-equal "if (zerop 1) (format \"yes\") (format \"no\")"
+                               "no")
+  (let ((debug-on-error nil))
+    (eshell-command-result-equal
+     "if (zerop \"foo\")  (format \"yes\") (format \"no\")"
+     "no")))
+
 (ert-deftest esh-cmd-test/if-else-statement-ext-cmd ()
   "Test invocation of an if/else statement using an external command."
   (skip-unless (executable-find "["))
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 6b0e225f05f..e0e8e185e4c 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -190,6 +190,9 @@ esh-var-test/interp-var-indices-subcommand
      "zero")
     (eshell-command-result-equal
      "echo $eshell-test-value[${*echo 0} ${*echo 2}]"
+     '("zero" "two"))
+    (eshell-command-result-equal
+     "echo $eshell-test-value[{*echo 0} {*echo 2}]"
      '("zero" "two"))))
 
 (ert-deftest esh-var-test/interp-var-length-list ()
-- 
2.25.1

From b7ed8aa37dad9d32ab9738aa1a19a36c892f8017 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Fri, 4 Oct 2024 22:26:01 -0700
Subject: [PATCH 3/3] Add support for chaining conditionals in Eshell

* lisp/eshell/esh-cmd.el (eshell-structure-basic-command): Check for the
presence of the conditional.  Allow any number of BODY forms.
(eshell-rewrite-if-command): Add support for 'else' keyword and chained
conditionals.

* test/lisp/eshell/esh-cmd-tests.el (esh-cmd-test/if-else-statement):
Test 'else' keyword.
(esh-cmd-test/if-else-statement-chain): New test.

* doc/misc/eshell.texi (Control Flow): Document this change.

* etc/NEWS: Announce this change.
---
 doc/misc/eshell.texi              | 19 +++++++++++---
 etc/NEWS                          | 14 +++++++++++
 lisp/eshell/esh-cmd.el            | 42 ++++++++++++++++++-------------
 test/lisp/eshell/esh-cmd-tests.el | 22 +++++++++++++---
 4 files changed, 73 insertions(+), 24 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index bbb6b2e6aac..9a2714b14fb 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1698,16 +1698,29 @@ Control Flow
 @table @code
 
 @item if @var{conditional} @var{true-subcommand}
-@itemx if @var{conditional} @var{true-subcommand} @var{false-subcommand}
+@itemx if @var{conditional} @var{true-subcommand} else @var{false-subcommand}
 Evaluate @var{true-subcommand} if @var{conditional} is satisfied;
 otherwise, evaluate @var{false-subcommand}.  Both @var{true-subcommand}
 and @var{false-subcommand} should be subcommands, as with
 @var{conditional}.
 
+You can also chain together @code{if}/@code{else} forms, for example:
+
+@example
+if @{[ -f file.txt ]@} @{
+  echo found file
+@} else if @{[ -f alternate.txt ]@} @{
+  echo found alternate
+@} else @{
+  echo not found!
+@}
+@end example
+
 @item unless @var{conditional} @var{false-subcommand}
-@itemx unless @var{conditional} @var{false-subcommand} @var{true-subcommand}
+@itemx unless @var{conditional} @var{false-subcommand} else @var{true-subcommand}
 Evaluate @var{false-subcommand} if @var{conditional} is not satisfied;
-otherwise, evaluate @var{true-subcommand}.
+otherwise, evaluate @var{true-subcommand}.  Like above, you can also
+chain together @code{unless}/@code{else} forms.
 
 @item while @var{conditional} @var{subcommand}
 Repeatedly evaluate @var{subcommand} so long as @var{conditional} is
diff --git a/etc/NEWS b/etc/NEWS
index e4c1ef4eae0..adf47b61a5e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -217,6 +217,20 @@ These functions now take an optional ERROR-TARGET argument to control
 where to send the standard error output.  See the "(eshell) Entry
 Points" node in the Eshell manual for more details.
 
++++
+*** Conditional statements in Eshell now use an 'else' keyword.
+Eshell now prefers the following form when writing conditionals:
+
+    if {conditional} {true-subcommand} else {false-subcommand}
+
+The old form (without the 'else' keyword) is retained for compatibility.
+
++++
+*** You can now chain conditional statements in Eshell.
+When using the newly-preferred conditional form in Eshell, you can now
+chain together multiple 'if'/'else' statements.  For more information,
+see "(eshell) Control Flow" in the Eshell manual.
+
 +++
 *** Eshell's built-in 'wait' command now accepts a timeout.
 By passing '-t' or '--timeout', you can specify a maximum time to wait
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 65f997e5b88..c9096b0d159 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -551,12 +551,14 @@ eshell-rewrite-for-command
 	       ,body)
              (setq ,for-items (cdr ,for-items)))))))
 
-(defun eshell-structure-basic-command (func names keyword test body
-					    &optional else)
+(defun eshell-structure-basic-command (func names keyword test &rest body)
   "With TERMS, KEYWORD, and two NAMES, structure a basic command.
 The first of NAMES should be the positive form, and the second the
 negative.  It's not likely that users should ever need to call this
 function."
+  (unless test
+    (error "Missing test for `%s' command" keyword))
+
   ;; If the test form is a subcommand, wrap it in `eshell-commands' to
   ;; silence the output.
   (when (memq (car test) '(eshell-as-subcommand eshell-lisp-command))
@@ -582,33 +584,39 @@ eshell-structure-basic-command
       (setq test `(not ,test)))
 
   ;; Finally, create the form that represents this structured command.
-  `(,func ,test ,body ,else))
+  `(,func ,test ,@body))
 
 (defun eshell-rewrite-while-command (terms)
   "Rewrite a `while' command into its equivalent Eshell command form.
 Because the implementation of `while' relies upon conditional
 evaluation of its argument (i.e., use of a Lisp special form), it
 must be implemented via rewriting, rather than as a function."
-  (if (and (stringp (car terms))
-	   (member (car terms) '("while" "until")))
-      (eshell-structure-basic-command
-       'while '("while" "until") (car terms)
-       (cadr terms)
-       (car (last terms)))))
+  (when (and (stringp (car terms))
+             (member (car terms) '("while" "until")))
+    (eshell-structure-basic-command
+     'while '("while" "until") (car terms)
+     (cadr terms)
+     (caddr terms))))
 
 (defun eshell-rewrite-if-command (terms)
   "Rewrite an `if' command into its equivalent Eshell command form.
 Because the implementation of `if' relies upon conditional
 evaluation of its argument (i.e., use of a Lisp special form), it
 must be implemented via rewriting, rather than as a function."
-  (if (and (stringp (car terms))
-	   (member (car terms) '("if" "unless")))
-      (eshell-structure-basic-command
-       'if '("if" "unless") (car terms)
-       (cadr terms)
-       (car (last terms (if (= (length terms) 4) 2)))
-       (when (= (length terms) 4)
-         (car (last terms))))))
+  (when (and (stringp (car terms))
+             (member (car terms) '("if" "unless")))
+    (eshell-structure-basic-command
+     'if '("if" "unless") (car terms)
+     (cadr terms)
+     (caddr terms)
+     (if (equal (nth 3 terms) "else")
+         ;; If there's an "else" keyword, allow chaining together
+         ;; multiple "if" forms...
+         (or (eshell-rewrite-if-command (nthcdr 4 terms))
+             (nth 4 terms))
+       ;; ... otherwise, only allow a single "else" block (without the
+       ;; keyword) as before for compatibility.
+       (nth 3 terms)))))
 
 (defun eshell-set-exit-info (status &optional result)
   "Set the exit status and result for the last command.
diff --git a/test/lisp/eshell/esh-cmd-tests.el b/test/lisp/eshell/esh-cmd-tests.el
index 9e4cbc58201..0f388a9eba4 100644
--- a/test/lisp/eshell/esh-cmd-tests.el
+++ b/test/lisp/eshell/esh-cmd-tests.el
@@ -427,11 +427,15 @@ esh-cmd-test/if-statement
 (ert-deftest esh-cmd-test/if-else-statement ()
   "Test invocation of an if/else statement."
   (let ((eshell-test-value t))
-    (eshell-command-result-equal "if $eshell-test-value {echo yes} {echo no}"
-                                 "yes"))
+    (eshell-command-result-equal
+     "if $eshell-test-value {echo yes} {echo no}" "yes")
+    (eshell-command-result-equal
+     "if $eshell-test-value {echo yes} else {echo no}" "yes"))
   (let ((eshell-test-value nil))
-    (eshell-command-result-equal "if $eshell-test-value {echo yes} {echo no}"
-                                 "no")))
+    (eshell-command-result-equal
+     "if $eshell-test-value {echo yes} {echo no}" "no")
+    (eshell-command-result-equal
+     "if $eshell-test-value {echo yes} else {echo no}" "no")))
 
 (ert-deftest esh-cmd-test/if-else-statement-lisp-form ()
   "Test invocation of an if/else statement using a Lisp form."
@@ -474,6 +478,16 @@ esh-cmd-test/if-else-statement-ext-cmd
   (eshell-command-result-equal "if {[ foo = bar ]} {echo yes} {echo no}"
                                "no"))
 
+(ert-deftest esh-cmd-test/if-else-statement-chain ()
+  "Test invocation of a chained if/else statement."
+  (dolist (case '((1 . "one") (2 . "two") (3 . "other")))
+    (let ((eshell-test-value (car case)))
+      (eshell-command-result-equal
+       (concat "if (= eshell-test-value 1) {echo one} "
+               "else if (= eshell-test-value 2) {echo two} "
+               "else {echo other}")
+       (cdr case)))))
+
 (ert-deftest esh-cmd-test/if-statement-pipe ()
   "Test invocation of an if statement piped to another command."
   (skip-unless (executable-find "rev"))
-- 
2.25.1