Subject: 30.0.50; [PATCH] Indices in Eshell variable interpolation don't work with async subcommands
From: Jim Porter <[email-masked]>
To: [email-masked]
Date: Wed, 18 Jan 2023 19:36:41 -0800
List-Id: <debbugs-submit.debbugs.gnu.org>

Starting from "emacs -Q -f eshell":

   ~ $ echo $exec-path[0]
   /usr/local/sbin
   ~ $ echo $exec-path[${echo 0}]
   /usr/local/sbin
   ~ $ echo $exec-path[${*echo 0}]
   ;; no output

This is because 'eshell-eval-indices' gets an S-expr describing code to 
evaluate for the indices, and it just passes that to 'eval'. That's not 
the right way to do things for Eshell: instead, we should rely on 
'eshell-do-eval', which properly handles asynchronous evaluation. That's 
required for working with external commands like "*echo" (which calls 
the real /bin/echo).

The attached patch fixes this by changing 'eshell-eval-indices' to 
'eshell-indices', which does some minimal transformations on the S-expr 
for the indices, and then uses it to build the final S-expr to pass to 
'eshell-do-eval'.

This could possibly go in Emacs 29, since it's a bugfix to add onto a 
previous bugfix (see commit 990f36fa10). However, I'd lean towards just 
merging to master; this is a fairly obscure issue, and we can't just fix 
*every* bug we find on the release branch, or the branch will never 
stabilize. If someone else thinks it's important enough to go on the 
release branch though, I won't argue.

From 1bbaf547d8d6668bca732e14dc190416c5b52671 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Wed, 18 Jan 2023 19:15:38 -0800
Subject: [PATCH] Fix evaluation of asynchronous expansions in Eshell indices

Previously, this code passed the indices to a separate function, which
called 'eval' on them, but it should instead make an S-expr that
'eshell-do-eval' can evaluate.

* lisp/eshell/esh-var.el (eshell-eval-indices): Rename to...
(eshell-indices): ... this, and adjust implementation to return a form
to evaluate via 'eshell-do-eval'.
(eshell-parse-variable): Use 'eshell-indices'.  Also, remove
irrelevant comment.
(eshell-parse-variable-ref): Fix quoting in docstring.
(eshell-parse-indices): Fix typo in docstring.

* test/lisp/eshell/esh-var-tests.el
(esh-var-test/interp-var-indices-subcommand)
(esh-var-test/quoted-interp-var-indices-subcommand): New tests.
---
 lisp/eshell/esh-var.el            | 15 +++++++--------
 test/lisp/eshell/esh-var-tests.el | 25 +++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index fd76a2c6f09..2da35222044 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -467,9 +467,7 @@ eshell-parse-variable
 	  indices (and (not (eobp))
 		       (eq (char-after) ?\[)
 		       (eshell-parse-indices))
-          ;; This is an expression that will be evaluated by `eshell-do-eval',
-          ;; which only support let-binding of dynamically-scoped vars
-	  value `(let ((indices (eshell-eval-indices ',indices))) ,value))
+          value `(let ((indices ,(eshell-indices indices))) ,value))
     (when get-len
       (setq value `(length ,value)))
     (when eshell-current-quoted
@@ -496,7 +494,7 @@ eshell-parse-variable-ref
 
   NAME          an environment or Lisp variable value
   \"LONG-NAME\"   disambiguates the length of the name
-  `LONG-NAME'   as above
+  \\='LONG-NAME\\='   as above
   {COMMAND}     result of command is variable's value
   (LISP-FORM)   result of Lisp form is variable's value
   <COMMAND>     write the output of command to a temporary file;
@@ -591,7 +589,7 @@ eshell-parse-indices
   "Parse and return a list of index-lists.
 
 For example, \"[0 1][2]\" becomes:
-  ((\"0\" \"1\") (\"2\")."
+  ((\"0\" \"1\") (\"2\"))."
   (let (indices)
     (while (eq (char-after) ?\[)
       (let ((end (eshell-find-delimiter ?\[ ?\])))
@@ -607,9 +605,10 @@ eshell-parse-indices
 	  (goto-char (1+ end)))))
     (nreverse indices)))
 
-(defun eshell-eval-indices (indices)
-  "Evaluate INDICES, a list of index-lists generated by `eshell-parse-indices'."
-  (mapcar (lambda (i) (mapcar #'eval i)) indices))
+(defun eshell-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)))
 
 (defun eshell-get-variable (name &optional indices quoted)
   "Get the value for the variable NAME.
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 0cc1b92266f..82324d72163 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -82,6 +82,17 @@ esh-var-test/interp-var-indices
     (eshell-command-result-equal "echo $eshell-test-value[0 2 4]"
                                  '("zero" "two" "four"))))
 
+(ert-deftest esh-var-test/interp-var-indices-subcommand ()
+  "Interpolate list variable with subcommand expansion for indices"
+  (skip-unless (executable-find "echo"))
+  (let ((eshell-test-value '("zero" "one" "two" "three" "four")))
+    (eshell-command-result-equal
+     "echo $eshell-test-value[${*echo 0}]"
+     "zero")
+    (eshell-command-result-equal
+     "echo $eshell-test-value[${*echo 0} ${*echo 2}]"
+     '("zero" "two"))))
+
 (ert-deftest esh-var-test/interp-var-split-indices ()
   "Interpolate string variable with indices"
   (let ((eshell-test-value "zero one two three four"))
@@ -271,6 +282,20 @@ esh-var-test/quoted-interp-var-indices
     (eshell-command-result-equal "echo \"$eshell-test-value[1 2 4]\""
                                  "(\"one\" \"two\" \"four\")")))
 
+(ert-deftest esh-var-test/quote-interp-var-indices-subcommand ()
+  "Interpolate list variable with subcommand expansion for indices
+inside double-quotes"
+  (skip-unless (executable-find "echo"))
+  (let ((eshell-test-value '("zero" "one" "two" "three" "four")))
+    (eshell-command-result-equal
+     "echo \"$eshell-test-value[${*echo 0}]\""
+     "zero")
+    ;; FIXME: These tests would use the 0th index like the other tests
+    ;; here, but see above.
+    (eshell-command-result-equal
+     "echo \"$eshell-test-value[${*echo 1} ${*echo 2}]\""
+     "(\"one\" \"two\")")))
+
 (ert-deftest esh-var-test/quoted-interp-var-split-indices ()
   "Interpolate string variable with indices inside double-quotes"
   (let ((eshell-test-value "zero one two three four"))
-- 
2.25.1