all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Jim Porter <jporterbugs@gmail.com>
To: 60942@debbugs.gnu.org
Subject: bug#60942: 30.0.50; [PATCH] Indices in Eshell variable interpolation don't work with async subcommands
Date: Wed, 18 Jan 2023 19:36:41 -0800	[thread overview]
Message-ID: <20e5cb2f-aabe-53fd-d3af-81ec5ff11e7b@gmail.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 1193 bytes --]

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.

[-- Attachment #2: 0001-Fix-evaluation-of-asynchronous-expansions-in-Eshell-.patch --]
[-- Type: text/plain, Size: 4989 bytes --]

From 1bbaf547d8d6668bca732e14dc190416c5b52671 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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


             reply	other threads:[~2023-01-19  3:36 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-19  3:36 Jim Porter [this message]
2023-01-19  6:49 ` bug#60942: 30.0.50; [PATCH] Indices in Eshell variable interpolation don't work with async subcommands Eli Zaretskii
2023-01-19  7:37   ` Jim Porter
2023-01-19 19:31     ` Jim Porter
2023-01-19 19:41       ` Eli Zaretskii
2023-01-19 20:20         ` Jim Porter
2023-01-20  1:54           ` Jim Porter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20e5cb2f-aabe-53fd-d3af-81ec5ff11e7b@gmail.com \
    --to=jporterbugs@gmail.com \
    --cc=60942@debbugs.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.