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
next 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
List information: https://www.gnu.org/software/emacs/
* 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 public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).