* bug#73650: 31.0.50; [PATCH] Improve correctness of Eshell conditional forms and allow if/else chaining
@ 2024-10-06 1:34 Jim Porter
2024-10-17 4:56 ` Jim Porter
0 siblings, 1 reply; 2+ messages in thread
From: Jim Porter @ 2024-10-06 1:34 UTC (permalink / raw)
To: 73650
[-- Attachment #1: Type: text/plain, Size: 589 bytes --]
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!
}
[-- Attachment #2: 0001-Simplify-creation-of-Eshell-command-forms.patch --]
[-- Type: text/plain, Size: 4203 bytes --]
From dbd223d4cfbdb07ed87c7f497c2ba691a112a16c Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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
[-- Attachment #3: 0002-Improve-correctness-of-Eshell-sub-forms.patch --]
[-- Type: text/plain, Size: 15550 bytes --]
From 15dcc7505b5edc1f6039b0d57ec0033748062a93 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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
[-- Attachment #4: 0003-Add-support-for-chaining-conditionals-in-Eshell.patch --]
[-- Type: text/plain, Size: 8492 bytes --]
From b7ed8aa37dad9d32ab9738aa1a19a36c892f8017 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-10-17 4:56 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-06 1:34 bug#73650: 31.0.50; [PATCH] Improve correctness of Eshell conditional forms and allow if/else chaining Jim Porter
2024-10-17 4:56 ` Jim Porter
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.