Only set Eshell execution result metavariables when non-nil

This simplifies usage of 'eshell-close-handles' in several places and
makes it work more like the docstring indicated it would.

* lisp/eshell/esh-io.el (eshell-close-handles): Only store EXIT-CODE
and RESULT if they're non-nil.  Also, use 'dotimes' and 'dolist' to
simplify the implementation.

* lisp/eshell/em-alias.el (eshell-write-aliases-list):
* lisp/eshell/esh-cmd.el (eshell-rewrite-for-command)
(eshell-structure-basic-command): Adapt calls to
'eshell-close-handles'.

* test/lisp/eshell/eshell-tests.el (eshell-test/simple-command-result)
(eshell-test/lisp-command, eshell-test/lisp-command-with-quote)
(eshell-test/for-loop, eshell-test/for-name-loop)
(eshell-test/for-name-shadow-loop, eshell-test/lisp-command-args)
(eshell-test/subcommand, eshell-test/subcommand-args)
(eshell-test/subcommand-lisp): Move from here...

* test/lisp/eshell/esh-cmd-tests.el
(esh-cmd-test/simple-command-result, esh-cmd-test/lisp-command)
(esh-cmd-test/lisp-command-with-quote, esh-cmd-test/for-loop)
(esh-cmd-test/for-name-loop, esh-cmd-test/for-name-shadow-loop)
(esh-cmd-test/lisp-command-args, esh-cmd-test/subcommand)
(esh-cmd-test/subcommand-args, esh-cmd-test/subcommand-lisp): ... to
here.
(esh-cmd-test/and-operator, esh-cmd-test/or-operator)
(esh-cmd-test/for-loop-list, esh-cmd-test/for-loop-multiple-args)
(esh-cmd-test/while-loop, esh-cmd-test/until-loop)
(esh-cmd-test/if-statement, esh-cmd-test/if-else-statement)
(esh-cmd-test/unless-statement, esh-cmd-test/unless-else-statement):
New tests.

* doc/misc/eshell.texi (Invocation): Explain '&&' and '||'.
(for loop): Move from here...
(Control Flow): ... to here, and add documentation for other control
flow forms.

2 files changed, 189 insertions, 53 deletions

diff --git a/test/lisp/eshell/esh-cmd-tests.el b/test/lisp/eshell/esh-cmd-tests.el
new file mode 100644
index 00000000000..1d5cd29d7cf
--- /dev/null
+++ b/test/lisp/eshell/esh-cmd-tests.el
@@ -0,0 +1,189 @@
+;;; esh-cmd-tests.el --- esh-cmd test suite  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Tests for Eshell's command invocation.
+
+;;; Code:
+
+(require 'ert)
+(require 'esh-mode)
+(require 'eshell)
+
+(require 'eshell-tests-helpers
+         (expand-file-name "eshell-tests-helpers"
+                           (file-name-directory (or load-file-name
+                                                    default-directory))))
+
+(defvar eshell-test-value nil)
+
+;;; Tests:
+
+
+;; Command invocation
+
+(ert-deftest esh-cmd-test/simple-command-result ()
+  "Test invocation with a simple command."
+  (should (equal (eshell-test-command-result "+ 1 2") 3)))
+
+(ert-deftest esh-cmd-test/lisp-command ()
+  "Test invocation with an elisp command."
+  (should (equal (eshell-test-command-result "(+ 1 2)") 3)))
+
+(ert-deftest esh-cmd-test/lisp-command-with-quote ()
+  "Test invocation with an elisp command containing a quote."
+  (should (equal (eshell-test-command-result "(eq 'foo nil)") nil)))
+
+(ert-deftest esh-cmd-test/lisp-command-args ()
+  "Test invocation with elisp and trailing args.
+Test that trailing arguments outside the S-expression are
+ignored.  e.g. \"(+ 1 2) 3\" => 3"
+  (should (equal (eshell-test-command-result "(+ 1 2) 3") 3)))
+
+(ert-deftest esh-cmd-test/subcommand ()
+  "Test invocation with a simple subcommand."
+  (should (equal (eshell-test-command-result "{+ 1 2}") 3)))
+
+(ert-deftest esh-cmd-test/subcommand-args ()
+  "Test invocation with a subcommand and trailing args.
+Test that trailing arguments outside the subcommand are ignored.
+e.g. \"{+ 1 2} 3\" => 3"
+  (should (equal (eshell-test-command-result "{+ 1 2} 3") 3)))
+
+(ert-deftest esh-cmd-test/subcommand-lisp ()
+  "Test invocation with an elisp subcommand and trailing args.
+Test that trailing arguments outside the subcommand are ignored.
+e.g. \"{(+ 1 2)} 3\" => 3"
+  (should (equal (eshell-test-command-result "{(+ 1 2)} 3") 3)))
+
+
+;; Logical operators
+
+(ert-deftest esh-cmd-test/and-operator ()
+  "Test logical && operator."
+  (skip-unless (executable-find "["))
+  (with-temp-eshell
+   (eshell-command-result-p "[ foo = foo ] && echo hi"
+                            "hi\n")
+   (eshell-command-result-p "[ foo = bar ] && echo hi"
+                            "\\`\\'")))
+
+(ert-deftest esh-cmd-test/or-operator ()
+  "Test logical || operator."
+  (skip-unless (executable-find "["))
+  (with-temp-eshell
+   (eshell-command-result-p "[ foo = foo ] || echo hi"
+                            "\\`\\'")
+   (eshell-command-result-p "[ foo = bar ] || echo hi"
+                            "hi\n")))
+
+
+;; Control flow statements
+
+(ert-deftest esh-cmd-test/for-loop ()
+  "Test invocation of a for loop."
+  (with-temp-eshell
+   (eshell-command-result-p "for i in 5 { echo $i }"
+                            "5\n")))
+
+(ert-deftest esh-cmd-test/for-loop-list ()
+  "Test invocation of a for loop iterating over a list."
+  (with-temp-eshell
+   (eshell-command-result-p "for i in (list 1 2 (list 3 4)) { echo $i }"
+                            "1\n2\n(3 4)\n")))
+
+(ert-deftest esh-cmd-test/for-loop-multiple-args ()
+  "Test invocation of a for loop iterating over multiple arguments."
+  (with-temp-eshell
+   (eshell-command-result-p "for i in 1 2 (list 3 4) { echo $i }"
+                            "1\n2\n3\n4\n")))
+
+(ert-deftest esh-cmd-test/for-name-loop () ; bug#15231
+  "Test invocation of a for loop using `name'."
+  (let ((process-environment (cons "name" process-environment)))
+    (should (equal (eshell-test-command-result
+                    "for name in 3 { echo $name }")
+                   3))))
+
+(ert-deftest esh-cmd-test/for-name-shadow-loop () ; bug#15372
+  "Test invocation of a for loop using an env-var."
+  (let ((process-environment (cons "name=env-value" process-environment)))
+    (with-temp-eshell
+     (eshell-command-result-p
+      "echo $name; for name in 3 { echo $name }; echo $name"
+      "env-value\n3\nenv-value\n"))))
+
+(ert-deftest esh-cmd-test/while-loop ()
+  "Test invocation of a while loop."
+  (skip-unless (executable-find "["))
+  (with-temp-eshell
+   (let ((eshell-test-value 0))
+     (eshell-command-result-p
+      (concat "while {[ $eshell-test-value -ne 3 ]} "
+              "{ setq eshell-test-value (1+ eshell-test-value) }")
+      "1\n2\n3\n"))))
+
+(ert-deftest esh-cmd-test/until-loop ()
+  "Test invocation of an until loop."
+  (skip-unless (executable-find "["))
+  (with-temp-eshell
+   (let ((eshell-test-value 0))
+     (eshell-command-result-p
+      (concat "until {[ $eshell-test-value -eq 3 ]} "
+              "{ setq eshell-test-value (1+ eshell-test-value) }")
+      "1\n2\n3\n"))))
+
+(ert-deftest esh-cmd-test/if-statement ()
+  "Test invocation of an if statement."
+  (skip-unless (executable-find "["))
+  (with-temp-eshell
+   (eshell-command-result-p "if {[ foo = foo ]} {echo yes}"
+                            "yes\n")
+   (eshell-command-result-p "if {[ foo = bar ]} {echo yes}"
+                            "\\`\\'")))
+
+(ert-deftest esh-cmd-test/if-else-statement ()
+  "Test invocation of an if/else statement."
+  (skip-unless (executable-find "["))
+  (with-temp-eshell
+   (eshell-command-result-p "if {[ foo = foo ]} {echo yes} {echo no}"
+                            "yes\n")
+   (eshell-command-result-p "if {[ foo = bar ]} {echo yes} {echo no}"
+                            "no\n")))
+
+(ert-deftest esh-cmd-test/unless-statement ()
+  "Test invocation of an unless statement."
+  (skip-unless (executable-find "["))
+  (with-temp-eshell
+   (eshell-command-result-p "unless {[ foo = foo ]} {echo no}"
+                            "\\`\\'")
+   (eshell-command-result-p "unless {[ foo = bar ]} {echo no}"
+                            "no\n")))
+
+(ert-deftest esh-cmd-test/unless-else-statement ()
+  "Test invocation of an unless/else statement."
+  (skip-unless (executable-find "["))
+  (with-temp-eshell
+   (eshell-command-result-p "unless {[ foo = foo ]} {echo no} {echo yes}"
+                            "yes\n")
+   (eshell-command-result-p "unless {[ foo = bar ]} {echo no} {echo yes}"
+                            "no\n")))
+
+;; esh-cmd-tests.el ends here
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index 5dc18775485..8423500ea7d 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -36,59 +36,6 @@
 
 ;;; Tests:
 
-(ert-deftest eshell-test/simple-command-result ()
-  "Test `eshell-command-result' with a simple command."
-  (should (equal (eshell-test-command-result "+ 1 2") 3)))
-
-(ert-deftest eshell-test/lisp-command ()
-  "Test `eshell-command-result' with an elisp command."
-  (should (equal (eshell-test-command-result "(+ 1 2)") 3)))
-
-(ert-deftest eshell-test/lisp-command-with-quote ()
-  "Test `eshell-command-result' with an elisp command containing a quote."
-  (should (equal (eshell-test-command-result "(eq 'foo nil)") nil)))
-
-(ert-deftest eshell-test/for-loop ()
-  "Test `eshell-command-result' with a for loop.."
-  (let ((process-environment (cons "foo" process-environment)))
-    (should (equal (eshell-test-command-result
-                    "for foo in 5 { echo $foo }") 5))))
-
-(ert-deftest eshell-test/for-name-loop () ;Bug#15231
-  "Test `eshell-command-result' with a for loop using `name'."
-  (let ((process-environment (cons "name" process-environment)))
-    (should (equal (eshell-test-command-result
-                    "for name in 3 { echo $name }") 3))))
-
-(ert-deftest eshell-test/for-name-shadow-loop () ; bug#15372
-  "Test `eshell-command-result' with a for loop using an env-var."
-  (let ((process-environment (cons "name=env-value" process-environment)))
-    (with-temp-eshell
-     (eshell-command-result-p "echo $name; for name in 3 { echo $name }; echo $name"
-                              "env-value\n3\nenv-value\n"))))
-
-(ert-deftest eshell-test/lisp-command-args ()
-  "Test `eshell-command-result' with elisp and trailing args.
-Test that trailing arguments outside the S-expression are
-ignored.  e.g. \"(+ 1 2) 3\" => 3"
-  (should (equal (eshell-test-command-result "(+ 1 2) 3") 3)))
-
-(ert-deftest eshell-test/subcommand ()
-  "Test `eshell-command-result' with a simple subcommand."
-  (should (equal (eshell-test-command-result "{+ 1 2}") 3)))
-
-(ert-deftest eshell-test/subcommand-args ()
-  "Test `eshell-command-result' with a subcommand and trailing args.
-Test that trailing arguments outside the subcommand are ignored.
-e.g. \"{+ 1 2} 3\" => 3"
-  (should (equal (eshell-test-command-result "{+ 1 2} 3") 3)))
-
-(ert-deftest eshell-test/subcommand-lisp ()
-  "Test `eshell-command-result' with an elisp subcommand and trailing args.
-Test that trailing arguments outside the subcommand are ignored.
-e.g. \"{(+ 1 2)} 3\" => 3"
-  (should (equal (eshell-test-command-result "{(+ 1 2)} 3") 3)))
-
 (ert-deftest eshell-test/pipe-headproc ()
   "Check that piping a non-process to a process command waits for the process"
   (skip-unless (executable-find "cat"))