unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation
@ 2022-03-03  6:35 Jim Porter
  2022-03-03 13:58 ` Lars Ingebrigtsen
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Jim Porter @ 2022-03-03  6:35 UTC (permalink / raw)
  To: 54227

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

There are a few related issues with how Eshell parses variable 
interpolations, especially when mixing with double-quotes. Starting from 
"emacs -Q --eval '(eshell)':

1. Quoted subcommands

   ~ $ echo ${echo hi}
   hi  ;; good, this is what we want
   ~ $ echo "${echo hi}"
   echo hi: command not found

In this case, Eshell gets confused by the double quotes and looks for 
the command named `echo hi', not the command named `echo' with the 
argument `hi'.

2. Complex list indexing

   ~ $ echo $PATH[: 0]
   /usr/local/bin  ;; good (the first element of your $PATH)
   ~ $ echo $PATH[":" 0]
   Wrong type argument: number-or-marker-p, (eshell-escape-arg ":")

Here, Eshell neglects to evaluate the subscript arguments, so only 
literal, unquoted values work. Note that the ":" in this case means 
"split the string $PATH with colon as a delimiter". The same happens 
with Elisp subexpressions:

   ~ $ echo $exec-path[0]
   /usr/local/bin  ;; good, again
   ~ $ echo $exec-path[$(+ 1 -1)]
   Wrong type argument: number-or-marker-p, (eshell-escape-arg (let 
((indices 'nil)) (eshell-command-to-value (eshell-lisp-command '(+ 1 -1)))))

3. Bare-string indexing

   ~ $ setq foo "fooXbarXbaz"
   fooXbarXbaz
   ~ $ echo $foo[X 0]
   (nil
     #("fooXbarXbaz" 0 11
     (escaped t)))

This is similar to the above case, but here Eshell thinks that "X" is a 
literal index to use. First, it splits `foo' into a list using space as 
a delimiter. Then gets confused when it uses a string as an index into a 
list, so it returns nil. The second element of the result is the 0th 
element of `foo'-as-a-list, aka the whole string since there are no 
spaces. (Just ignore for now the fact that the string "fooXbarXbaz" 
mysteriously gained an `escaped' property. That's a separate bug.)

This last one is less of a bug and more of a missing feature: if a bare 
(non-numeric) string is used as an index for a string variable, we know 
it *must* be a delimiter argument. Since we're indexing into a list, a 
string index doesn't really make sense.

--------------------

Attached is a patch series to fix these cases, as well as a lot more 
tests for Eshell variable interpolation, since this can get tricky. I've 
also updated the documentation to clarify how indexing works. I didn't 
initially realize that you could do quite so many complex things with 
it, so I figured it'd be good to explain in more detail so it's easier 
to discover.

[-- Attachment #2: 0001-Move-Eshell-variable-interpolation-tests-to-their-ow.patch --]
[-- Type: text/plain, Size: 8882 bytes --]

From beea98cec6e56fcff7abc63eda00fecb448a3a8c Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 27 Feb 2022 18:34:22 -0800
Subject: [PATCH 1/5] Move Eshell variable interpolation tests to their own
 file

* test/lisp/eshell/eshell-tests.el (eshell-test/interp-cmd)
(eshell-test/interp-lisp, eshell-test/interp-temp-cmd)
(eshell-test/interp-concat, eshell-test/interp-concat-lisp)
(eshell-test/interp-concat2, eshell-test/interp-concat-lisp2)
(eshell-test/interp-cmd-external)
(eshell-test/interp-cmd-external-concat, eshell-test/window-height)
(eshell-test/window-width, eshell-test/last-result-var)
(eshell-test/last-result-var2, eshell-test/last-arg-var):
Move from here...

* test/lisp/eshell/esh-var-test.el (esh-var-test/interp-lisp)
(esh-var-test/interp-cmd, esh-var-test/interp-cmd-external)
(esh-var-test/interp-temp-cmd, esh-var-test/interp-concat-lisp)
(esh-var-test/interp-concat-lisp2, esh-var-test/interp-concat-cmd)
(esh-var-test/interp-concat-cmd2)
(esh-var-test/interp-concat-cmd-external, esh-var-test/window-height)
(esh-var-test/window-width, esh-var-test/last-result-var)
(esh-var-test/last-result-var2, esh-var-test/last-arg-var):
... to here.
---
 test/lisp/eshell/esh-var-tests.el | 111 ++++++++++++++++++++++++++++++
 test/lisp/eshell/eshell-tests.el  |  68 ------------------
 2 files changed, 111 insertions(+), 68 deletions(-)
 create mode 100644 test/lisp/eshell/esh-var-tests.el

diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
new file mode 100644
index 0000000000..8d803e5ca4
--- /dev/null
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -0,0 +1,111 @@
+;;; esh-var-tests.el --- esh-var 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 variable handling.
+
+;;; 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))))
+
+;;; Tests:
+
+\f
+;; Variable interpolation
+
+(ert-deftest esh-var-test/interp-lisp ()
+  "Interpolate Lisp form evaluation"
+  (should (equal (eshell-test-command-result "+ $(+ 1 2) 3") 6)))
+
+(ert-deftest esh-var-test/interp-cmd ()
+  "Interpolate command result"
+  (should (equal (eshell-test-command-result "+ ${+ 1 2} 3") 6)))
+
+(ert-deftest esh-var-test/interp-cmd-external ()
+  "Interpolate command result from external command"
+  (skip-unless (executable-find "echo"))
+  (with-temp-eshell
+   (eshell-command-result-p "echo ${*echo hi}"
+                            "hi\n")))
+
+(ert-deftest esh-var-test/interp-temp-cmd ()
+  "Interpolate command result redirected to temp file"
+  (should (equal (eshell-test-command-result "cat $<echo hi>") "hi")))
+
+(ert-deftest esh-var-test/interp-concat-lisp ()
+  "Interpolate and concat Lisp form"
+  (should (equal (eshell-test-command-result "+ $(+ 1 2)3 3") 36)))
+
+(ert-deftest esh-var-test/interp-concat-lisp2 ()
+  "Interpolate and concat two Lisp forms"
+  (should (equal (eshell-test-command-result "+ $(+ 1 2)$(+ 1 2) 3") 36)))
+
+(ert-deftest esh-var-test/interp-concat-cmd ()
+  "Interpolate and concat command"
+  (should (equal (eshell-test-command-result "+ ${+ 1 2}3 3") 36)))
+
+(ert-deftest esh-var-test/interp-concat-cmd2 ()
+  "Interpolate and concat two commands"
+  (should (equal (eshell-test-command-result "+ ${+ 1 2}${+ 1 2} 3") 36)))
+
+(ert-deftest esh-var-test/interp-concat-cmd-external ()
+  "Interpolate command result from external command with concatenation"
+  (skip-unless (executable-find "echo"))
+  (with-temp-eshell
+   (eshell-command-result-p "echo ${echo hi}-${*echo there}"
+                            "hi-there\n")))
+
+\f
+;; Built-in variables
+
+(ert-deftest esh-var-test/window-height ()
+  "$LINES should equal (window-height)"
+  (should (eshell-test-command-result "= $LINES (window-height)")))
+
+(ert-deftest esh-var-test/window-width ()
+  "$COLUMNS should equal (window-width)"
+  (should (eshell-test-command-result "= $COLUMNS (window-width)")))
+
+(ert-deftest esh-var-test/last-result-var ()
+  "Test using the \"last result\" ($$) variable"
+  (with-temp-eshell
+   (eshell-command-result-p "+ 1 2; + $$ 2"
+                            "3\n5\n")))
+
+(ert-deftest esh-var-test/last-result-var2 ()
+  "Test using the \"last result\" ($$) variable twice"
+  (with-temp-eshell
+   (eshell-command-result-p "+ 1 2; + $$ $$"
+                             "3\n6\n")))
+
+(ert-deftest esh-var-test/last-arg-var ()
+  "Test using the \"last arg\" ($_) variable"
+  (with-temp-eshell
+   (eshell-command-result-p "+ 1 2; + $_ 4"
+                             "3\n6\n")))
+
+;; esh-var-tests.el ends here
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index eff4edd62c..e31db07c61 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -85,48 +85,6 @@ eshell-test/subcommand-lisp
 e.g. \"{(+ 1 2)} 3\" => 3"
   (should (equal (eshell-test-command-result "{(+ 1 2)} 3") 3)))
 
-(ert-deftest eshell-test/interp-cmd ()
-  "Interpolate command result"
-  (should (equal (eshell-test-command-result "+ ${+ 1 2} 3") 6)))
-
-(ert-deftest eshell-test/interp-lisp ()
-  "Interpolate Lisp form evaluation"
-  (should (equal (eshell-test-command-result "+ $(+ 1 2) 3") 6)))
-
-(ert-deftest eshell-test/interp-temp-cmd ()
-  "Interpolate command result redirected to temp file"
-  (should (equal (eshell-test-command-result "cat $<echo hi>") "hi")))
-
-(ert-deftest eshell-test/interp-concat ()
-  "Interpolate and concat command"
-  (should (equal (eshell-test-command-result "+ ${+ 1 2}3 3") 36)))
-
-(ert-deftest eshell-test/interp-concat-lisp ()
-  "Interpolate and concat Lisp form"
-  (should (equal (eshell-test-command-result "+ $(+ 1 2)3 3") 36)))
-
-(ert-deftest eshell-test/interp-concat2 ()
-  "Interpolate and concat two commands"
-  (should (equal (eshell-test-command-result "+ ${+ 1 2}${+ 1 2} 3") 36)))
-
-(ert-deftest eshell-test/interp-concat-lisp2 ()
-  "Interpolate and concat two Lisp forms"
-  (should (equal (eshell-test-command-result "+ $(+ 1 2)$(+ 1 2) 3") 36)))
-
-(ert-deftest eshell-test/interp-cmd-external ()
-  "Interpolate command result from external command"
-  (skip-unless (executable-find "echo"))
-  (with-temp-eshell
-   (eshell-command-result-p "echo ${*echo hi}"
-                            "hi\n")))
-
-(ert-deftest eshell-test/interp-cmd-external-concat ()
-  "Interpolate command result from external command with concatenation"
-  (skip-unless (executable-find "echo"))
-  (with-temp-eshell
-   (eshell-command-result-p "echo ${echo hi}-${*echo there}"
-                            "hi-there\n")))
-
 (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"))
@@ -152,32 +110,6 @@ eshell-test/pipe-headproc-stdin
    (eshell-wait-for-subprocess)
    (eshell-match-result "OLLEH\n")))
 
-(ert-deftest eshell-test/window-height ()
-  "$LINES should equal (window-height)"
-  (should (eshell-test-command-result "= $LINES (window-height)")))
-
-(ert-deftest eshell-test/window-width ()
-  "$COLUMNS should equal (window-width)"
-  (should (eshell-test-command-result "= $COLUMNS (window-width)")))
-
-(ert-deftest eshell-test/last-result-var ()
-  "Test using the \"last result\" ($$) variable"
-  (with-temp-eshell
-   (eshell-command-result-p "+ 1 2; + $$ 2"
-                            "3\n5\n")))
-
-(ert-deftest eshell-test/last-result-var2 ()
-  "Test using the \"last result\" ($$) variable twice"
-  (with-temp-eshell
-   (eshell-command-result-p "+ 1 2; + $$ $$"
-                             "3\n6\n")))
-
-(ert-deftest eshell-test/last-arg-var ()
-  "Test using the \"last arg\" ($_) variable"
-  (with-temp-eshell
-   (eshell-command-result-p "+ 1 2; + $_ 4"
-                             "3\n6\n")))
-
 (ert-deftest eshell-test/inside-emacs-var ()
   "Test presence of \"INSIDE_EMACS\" in subprocesses"
   (with-temp-eshell
-- 
2.25.1


[-- Attachment #3: 0002-Add-a-new-macro-to-simplify-parsing-temporary-Eshell.patch --]
[-- Type: text/plain, Size: 3012 bytes --]

From 7a64056e44a6f789fb2f58cca2c6876cfdcc107d Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 26 Feb 2022 20:55:22 -0800
Subject: [PATCH 2/5] Add a new macro to simplify parsing temporary Eshell
 command strings

This abstracts out the somewhat-unusual "insert&delete" logic in
'eshell-parse-command' so that it can be used elsewhere, and also
ensures that the deletion occurs even if an an error occurs.

* lisp/eshell/esh-cmd.el (eshell-with-temp-command): New macro.
(eshell-parse-command): Use it.
---
 lisp/eshell/esh-cmd.el | 42 +++++++++++++++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index dceb061c8f..04b54d9d79 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -350,6 +350,36 @@ eshell-complete-lisp-symbols
 
 (defvar eshell--sep-terms)
 
+(defmacro eshell-with-temp-command (command &rest body)
+  "Narrow the buffer to COMMAND and execute the forms in BODY.
+COMMAND can either be a string, or a cons cell demarcating a
+buffer region.  If COMMAND is a string, temporarily insert it
+into the buffer before narrowing.  Point will be set to the
+beginning of the narrowed region.
+
+The value returned is the last form in BODY."
+  (declare (indent 1))
+  `(let ((cmd ,command))
+     (if (stringp cmd)
+         ;; Since parsing relies partly on buffer-local state
+         ;; (e.g. that of `eshell-parse-argument-hook'), we need to
+         ;; perform the parsing in the Eshell buffer.
+         (let ((begin (point)) end
+	       (inhibit-point-motion-hooks t))
+           (with-silent-modifications
+             (insert cmd)
+             (setq end (point))
+             (unwind-protect
+                 (save-restriction
+                   (narrow-to-region begin end)
+                   (goto-char begin)
+                   ,@body)
+               (delete-region begin end))))
+       (save-restriction
+         (narrow-to-region (car cmd) (cdr cmd))
+         (goto-char (car cmd))
+         ,@body))))
+
 (defun eshell-parse-command (command &optional args toplevel)
   "Parse the COMMAND, adding ARGS if given.
 COMMAND can either be a string, or a cons cell demarcating a buffer
@@ -361,15 +391,9 @@ eshell-parse-command
 	  (append
 	   (if (consp command)
 	       (eshell-parse-arguments (car command) (cdr command))
-	     (let ((here (point))
-		   (inhibit-point-motion-hooks t))
-               (with-silent-modifications
-                 ;; FIXME: Why not use a temporary buffer and avoid this
-                 ;; "insert&delete" business?  --Stef
-                 (insert command)
-                 (prog1
-                     (eshell-parse-arguments here (point))
-                   (delete-region here (point))))))
+             (eshell-with-temp-command command
+               (goto-char (point-max))
+               (eshell-parse-arguments (point-min) (point-max))))
 	   args))
 	 (commands
 	  (mapcar
-- 
2.25.1


[-- Attachment #4: 0003-Fix-Eshell-dollar-interpolation-inside-of-double-quo.patch --]
[-- Type: text/plain, Size: 7805 bytes --]

From 64484ad751bfebc638afaf87b2cc8be63c550ba8 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 27 Feb 2022 21:04:30 -0800
Subject: [PATCH 3/5] Fix Eshell dollar interpolation inside of double-quotes

For example,

  echo "${echo hi}"

previously tried to run the program named 'echo hi', instead of 'echo'
with the argument 'hi'.

* lisp/eshell/esh-arg.el (eshell-parse-inner-double-quote):
New function.

* lisp/eshell/esh-var.el (eshell-parse-variable-ref): Support parsing
when wrapped in double-quiotes.

* test/lisp/eshell/esh-var-tests.el (esh-var-test/interp-var)
(esh-var-test/interp-quoted-var)
(esh-var-test/interp-quoted-var-concat)
(esh-var-test/quoted-interp-var)
(esh-var-test/quoted-interp-quoted-var)
(esh-var-test/quoted-interp-lisp, esh-var-test/quoted-interp-cmd)
(esh-var-test/quoted-interp-temp-cmd): New tests.
---
 lisp/eshell/esh-arg.el            | 24 +++++++++++++++
 lisp/eshell/esh-var.el            | 27 +++++++++++------
 test/lisp/eshell/esh-var-tests.el | 49 +++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+), 9 deletions(-)

diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index 1a2f2a57e8..e19481c4ba 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -354,6 +354,30 @@ eshell-parse-double-quote
 		  (list 'eshell-escape-arg arg))))
 	  (goto-char (1+ end)))))))
 
+(defun eshell-parse-inner-double-quote (bound)
+  "Parse the inner part of a double quoted string.
+The string to parse starts at point and ends at BOUND.
+
+If Eshell is currently parsing a quoted string and there are any
+backslash-escaped characters, this will return the unescaped
+string, updating point to BOUND.  Otherwise, this returns nil and
+leaves point where it was."
+  (when eshell-current-quoted
+    (let (strings
+          (start (point))
+          (special-char
+           (rx-to-string
+            `(seq "\\" (group (any ,@eshell-special-chars-inside-quoting))))))
+      (while (re-search-forward special-char bound t)
+        (push (concat (buffer-substring start (match-beginning 0))
+                      (match-string 1))
+              strings)
+        (setq start (match-end 0)))
+      (when strings
+        (push (buffer-substring start bound) strings)
+        (goto-char bound)
+        (apply #'concat (nreverse strings))))))
+
 (defun eshell-parse-special-reference ()
   "Parse a special syntax reference, of the form `#<args>'.
 
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index ee3ffbc647..24fdbde3cf 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -440,18 +440,16 @@ eshell-parse-variable-ref
     (let ((end (eshell-find-delimiter ?\{ ?\})))
       (if (not end)
           (throw 'eshell-incomplete ?\{)
+        (forward-char)
         (prog1
             `(eshell-convert
               (eshell-command-to-value
                (eshell-as-subcommand
-                ,(eshell-parse-command (cons (1+ (point)) end)))))
+                ,(let ((subcmd (or (eshell-parse-inner-double-quote end)
+                                   (cons (point) end)))
+                       (eshell-current-quoted nil))
+                   (eshell-parse-command subcmd)))))
           (goto-char (1+ end))))))
-   ((memq (char-after) '(?\' ?\"))
-    (let ((name (if (eq (char-after) ?\')
-                    (eshell-parse-literal-quote)
-                  (eshell-parse-double-quote))))
-      (if name
-	  `(eshell-get-variable ,(eval name) indices))))
    ((eq (char-after) ?\<)
     (let ((end (eshell-find-delimiter ?\< ?\>)))
       (if (not end)
@@ -463,7 +461,9 @@ eshell-parse-variable-ref
               `(let ((eshell-current-handles
                       (eshell-create-handles ,temp 'overwrite)))
                  (progn
-                   (eshell-as-subcommand ,(eshell-parse-command cmd))
+                   (eshell-as-subcommand
+                    ,(let ((eshell-current-quoted nil))
+                       (eshell-parse-command cmd)))
                    (ignore
                     (nconc eshell-this-command-hook
                            ;; Quote this lambda; it will be evaluated
@@ -478,9 +478,18 @@ eshell-parse-variable-ref
     (condition-case nil
         `(eshell-command-to-value
           (eshell-lisp-command
-           ',(read (current-buffer))))
+           ',(read (or (eshell-parse-inner-double-quote (point-max))
+                       (current-buffer)))))
       (end-of-file
        (throw 'eshell-incomplete ?\())))
+   ((looking-at (rx (or "'" "\"" "\\\"")))
+    (eshell-with-temp-command (or (eshell-parse-inner-double-quote (point-max))
+                                  (cons (point) (point-max)))
+      (let ((name (if (eq (char-after) ?\')
+                      (eshell-parse-literal-quote)
+                    (eshell-parse-double-quote))))
+        (when name
+	  `(eshell-get-variable ,(eval name) indices)))))
    ((assoc (char-to-string (char-after))
            eshell-variable-aliases-list)
     (forward-char)
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 8d803e5ca4..7ec6a97519 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -37,6 +37,25 @@
 \f
 ;; Variable interpolation
 
+(ert-deftest esh-var-test/interp-var ()
+  "Interpolate variable"
+  (should (equal (eshell-test-command-result "echo $user-login-name")
+                 user-login-name)))
+
+(ert-deftest esh-var-test/interp-quoted-var ()
+  "Interpolate quoted variable"
+  (should (equal (eshell-test-command-result "echo $'user-login-name'")
+                 user-login-name))
+  (should (equal (eshell-test-command-result "echo $\"user-login-name\"")
+                 user-login-name)))
+
+(ert-deftest esh-var-test/interp-quoted-var-concat ()
+  "Interpolate and concat quoted variable"
+  (should (equal (eshell-test-command-result "echo $'user-login-name'-foo")
+                 (concat user-login-name "-foo")))
+  (should (equal (eshell-test-command-result "echo $\"user-login-name\"-foo")
+                 (concat user-login-name "-foo"))))
+
 (ert-deftest esh-var-test/interp-lisp ()
   "Interpolate Lisp form evaluation"
   (should (equal (eshell-test-command-result "+ $(+ 1 2) 3") 6)))
@@ -79,6 +98,36 @@ esh-var-test/interp-concat-cmd-external
    (eshell-command-result-p "echo ${echo hi}-${*echo there}"
                             "hi-there\n")))
 
+(ert-deftest esh-var-test/quoted-interp-var ()
+  "Interpolate variable inside double-quotes"
+  (should (equal (eshell-test-command-result "echo \"$user-login-name\"")
+                 user-login-name)))
+
+(ert-deftest esh-var-test/quoted-interp-quoted-var ()
+  "Interpolate quoted variable inside double-quotes"
+  (should (equal (eshell-test-command-result
+                  "echo \"hi, $'user-login-name'\"")
+                 (concat "hi, " user-login-name)))
+  (should (equal (eshell-test-command-result
+                  "echo \"hi, $\\\"user-login-name\\\"\"")
+                 (concat "hi, " user-login-name))))
+
+(ert-deftest esh-var-test/quoted-interp-lisp ()
+  "Interpolate Lisp form evaluation inside double-quotes"
+  (should (equal (eshell-test-command-result
+                  "echo \"hi $(concat \\\"the\\\" \\\"re\\\")\"")
+                 "hi there")))
+
+(ert-deftest esh-var-test/quoted-interp-cmd ()
+  "Interpolate command result inside double-quotes"
+  (should (equal (eshell-test-command-result
+                  "echo \"hi ${echo \\\"there\\\"}\"")
+                 "hi there")))
+
+(ert-deftest esh-var-test/quoted-interp-temp-cmd ()
+  "Interpolate command result redirected to temp file inside double-quotes"
+  (should (equal (eshell-test-command-result "cat \"$<echo hi>\"") "hi")))
+
 \f
 ;; Built-in variables
 
-- 
2.25.1


[-- Attachment #5: 0004-Fix-parsing-of-indices-in-Eshell-expansions.patch --]
[-- Type: text/plain, Size: 17720 bytes --]

From 3d93eba3ebe8396c0d349cad15f40e2c387f4e91 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Tue, 1 Mar 2022 18:36:08 -0800
Subject: [PATCH 4/5] Fix parsing of indices in Eshell expansions

Previously, more-complex index expansions, like '$var[":" 0]' or
'$var[$(expr) 0]' failed to parse correctly.

* lisp/eshell/esh-var.el (Commentary): Clarify indexing and length
expansions.
(eshell-parse-indices): Expand docstring and support parsing inside
double-quotes.
(eshell-eval-indices): New function.
(eshell-parse-variable): Use it.

* test/lisp/eshell/esh-var-tests.el (eshell-test-value): New defvar.
(esh-var-test/interp-var-indices,
(esh-var-test/interp-var-split-indices)
(esh-var-test/interp-var-string-split-indices)
(esh-var-test/interp-var-regexp-split-indices)
(esh-var-test/interp-var-assoc, esh-var-test/interp-var-length-list)
(esh-var-test/interp-var-length-string)
(esh-var-test/interp-var-length-alist)
(esh-var-test/quoted-interp-var-indices)
(esh-var-test/quoted-interp-var-split-indices)
(esh-var-test/quoted-interp-var-string-split-indices)
(esh-var-test/quoted-interp-var-regexp-split-indices)
(esh-var-test/quoted-interp-var-assoc)
(esh-var-test/quoted-interp-var-length-list)
(esh-var-test/quoted-interp-var-length-string)
(esh-var-test/quoted-interp-var-length-alist): New tests.

* doc/misc/eshell.texi (Dollars Expansion): Expand and reword
documentation for indexing and length expansions.
---
 doc/misc/eshell.texi              |  62 ++++++------
 lisp/eshell/esh-var.el            |  59 ++++++------
 test/lisp/eshell/esh-var-tests.el | 152 ++++++++++++++++++++++++++++++
 3 files changed, 212 insertions(+), 61 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index bbf8ca6b8b..3301a854eb 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1022,11 +1022,6 @@ Dollars Expansion
 disambiguate the variable name when concatenating it with another
 value, such as @samp{$"@var{var}"-suffix}.
 
-@item $#@var{var}
-Expands to the length of the value bound to @var{var}.  Raises an error
-if the value is not a sequence
-(@pxref{Sequences Arrays Vectors, Sequences, , elisp, The Emacs Lisp Reference Manual}).
-
 @item $(@var{lisp})
 Expands to the result of evaluating the S-expression @code{(@var{lisp})}.  On
 its own, this is identical to just @code{(@var{lisp})}, but with the @code{$},
@@ -1041,34 +1036,35 @@ Dollars Expansion
 @command{@var{command}}, but writes the output to a temporary file and
 returns the file name.
 
-@item $@var{var}[i]
-Expands to the @code{i}th element of the value bound to @var{var}.  If
-the value is a string, it will be split at whitespace to make it a list.
-Again, raises an error if the value is not a sequence.
-
-@item $@var{var}[: i]
-As above, but now splitting occurs at the colon character.
-
-@item $@var{var}[: i j]
-As above, but instead of returning just a string, it now returns a list
-of two strings.  If the result is being interpolated into a larger
-string, this list will be flattened into one big string, with each
-element separated by a space.
-
-@item $@var{var}["\\\\" i]
-Separate on backslash characters.  Actually, the first argument -- if it
-doesn't have the form of a number, or a plain variable name -- can be
-any regular expression.  So to split on numbers, use
-@samp{$@var{var}["[0-9]+" 10 20]}.
-
-@item $@var{var}[hello]
-Calls @code{assoc} on @var{var} with @code{"hello"}, expecting it to be
-an alist (@pxref{Association List Type, Association Lists, , elisp,
-The Emacs Lisp Reference Manual}).
-
-@item $#@var{var}[hello]
-Returns the length of the @code{cdr} of the element of @var{var} whose
-car is equal to @code{"hello"}.
+@item $@var{expr}[@var{i...}]
+Expands to the @var{i}th element of the result of @var{expr}, an
+expression in one of the above forms listed here.  If multiple indices
+are supplied, this will return a list containing the elements for each
+index.  If @var{expr}'s value is a string, it will first be split at
+whitespace to make it a list.  If @var{expr}'s value is an alist
+(@pxref{Association List Type, Association Lists, , elisp, The Emacs
+Lisp Reference Manual}), this will call @code{assoc} on the result of
+@var{expr}, returning the @code{cdr} of the element of the result
+whose car is equal to @code{"i"}.  Raises an error if the value is not
+a sequence (@pxref{Sequences Arrays Vectors, Sequences, , elisp, The
+Emacs Lisp Reference Manual}).
+
+Multiple sets of indices can also be specified. For example, if
+@var{var} is a list of lists, @samp{$@var{var}[0][0]} is equivalent to
+@samp{(caar @var{var})}.
+
+@item $@var{expr}[@var{regexp} @var{i...}]
+As above (when @var{expr} expands to a string), but use @var{regexp}
+to split the string.  @var{regexp} can be any form other than a number
+or a plain variable name.  For example, @samp{$@var{var}[: 0]} will
+return the first element of a colon-delimited string.
+
+@item $#@var{expr}
+Expands to the length of the result of @var{expr}, an expression in
+one of the above forms.  For example, @samp{$#@var{var}} returns the
+length of the variable @var{var} and @samp{$#@var{var}[0]} returns the
+length of the first element of @var{var}.  Again, raises an error if
+the result of @var{expr} is not a sequence.
 
 @end table
 
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 24fdbde3cf..6f08a3fbc4 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -39,11 +39,6 @@
 ;;
 ;; Only "MYVAR" is part of the variable name in this case.
 ;;
-;;   $#VARIABLE
-;;
-;; Returns the length of the value of VARIABLE.  This could also be
-;; done using the `length' Lisp function.
-;;
 ;;   $(lisp)
 ;;
 ;; Returns result of Lisp evaluation.  Note: Used alone like this, it
@@ -61,38 +56,36 @@
 ;; Evaluates an eshell subcommand, redirecting the output to a
 ;; temporary file, and returning the file name.
 ;;
-;;   $ANYVAR[10]
+;;   $EXPR[10]
 ;;
-;; Return the 10th element of ANYVAR.  If ANYVAR's value is a string,
-;; it will be split in order to make it a list.  The splitting will
-;; occur at whitespace.
+;; Return the 10th element of $EXPR, which can be any dollar
+;; expression.  If $EXPR's value is a string, it will be split in
+;; order to make it a list.  The splitting will occur at whitespace.
 ;;
-;;   $ANYVAR[: 10]
+;;   $EXPR[10 20]
 ;;
-;; As above, except that splitting occurs at the colon now.
+;; As above, but instead of returning a single element, it now returns a
+;; list of two elements.
 ;;
-;;   $ANYVAR[: 10 20]
+;;   $EXPR[: 10]
 ;;
-;; As above, but instead of returning just a string, it now returns a
-;; list of two strings.  If the result is being interpolated into a
-;; larger string, this list will be flattened into one big string,
-;; with each element separated by a space.
+;; Like $EXPR[10], except that splitting occurs at the colon now.
 ;;
-;;   $ANYVAR["\\\\" 10]
+;;   $EXPR["\\\\" 10]
 ;;
 ;; Separate on backslash characters.  Actually, the first argument --
 ;; if it doesn't have the form of a number, or a plain variable name
 ;; -- can be any regular expression.  So to split on numbers, use
-;; '$ANYVAR["[0-9]+" 10 20]'.
+;; '$EXPR["[0-9]+" 10 20]'.
 ;;
-;;   $ANYVAR[hello]
+;;   $EXPR[hello]
 ;;
-;; Calls `assoc' on ANYVAR with 'hello', expecting it to be an alist.
+;; Calls `assoc' on $EXPR with 'hello', expecting it to be an alist.
 ;;
-;;   $#ANYVAR[hello]
+;;   $#EXPR
 ;;
-;; Returns the length of the cdr of the element of ANYVAR who car is
-;; equal to "hello".
+;; Returns the length of the value of $EXPR.  This could also be
+;; done using the `length' Lisp function.
 ;;
 ;; There are also a few special variables defined by Eshell.  '$$' is
 ;; the value of the last command (t or nil, in the case of an external
@@ -416,7 +409,7 @@ eshell-parse-variable
 		       (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 ',indices)) ,value))
+	  value `(let ((indices (eshell-eval-indices ',indices))) ,value))
     (if get-len
 	`(length ,value)
       value)))
@@ -504,19 +497,29 @@ eshell-parse-variable-ref
 (defvar eshell-glob-function)
 
 (defun eshell-parse-indices ()
-  "Parse and return a list of list of indices."
+  "Parse and return a list of index-lists.
+
+For example, \"[0 1][2]\" becomes:
+  ((\"0\" \"1\") (\"2\")."
   (let (indices)
     (while (eq (char-after) ?\[)
       (let ((end (eshell-find-delimiter ?\[ ?\])))
 	(if (not end)
 	    (throw 'eshell-incomplete ?\[)
 	  (forward-char)
-	  (let (eshell-glob-function)
-	    (setq indices (cons (eshell-parse-arguments (point) end)
-				indices)))
+          (eshell-with-temp-command (or (eshell-parse-inner-double-quote end)
+                                        (cons (point) end))
+	    (let (eshell-glob-function (eshell-current-quoted nil))
+	      (setq indices (cons (eshell-parse-arguments
+                                   (point-min) (point-max))
+				  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-get-variable (name &optional indices)
   "Get the value for the variable NAME."
   (let* ((alias (assoc name eshell-variable-aliases-list))
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 7ec6a97519..e679174939 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -32,6 +32,8 @@
                            (file-name-directory (or load-file-name
                                                     default-directory))))
 
+(defvar eshell-test-value nil)
+
 ;;; Tests:
 
 \f
@@ -56,6 +58,76 @@ esh-var-test/interp-quoted-var-concat
   (should (equal (eshell-test-command-result "echo $\"user-login-name\"-foo")
                  (concat user-login-name "-foo"))))
 
+(ert-deftest esh-var-test/interp-var-indices ()
+  "Interpolate list variable with indices"
+  (let ((eshell-test-value '("zero" "one" "two" "three" "four")))
+    (should (equal (eshell-test-command-result "echo $eshell-test-value[0]")
+                   "zero"))
+    (should (equal (eshell-test-command-result "echo $eshell-test-value[0 2]")
+                   '("zero" "two")))
+    (should (equal (eshell-test-command-result "echo $eshell-test-value[0 2 4]")
+                   '("zero" "two" "four")))))
+
+(ert-deftest esh-var-test/interp-var-split-indices ()
+  "Interpolate string variable with indices"
+  (let ((eshell-test-value "zero one two three four"))
+    (should (equal (eshell-test-command-result "echo $eshell-test-value[0]")
+                   "zero"))
+    (should (equal (eshell-test-command-result "echo $eshell-test-value[0 2]")
+                   '("zero" "two")))
+    (should (equal (eshell-test-command-result "echo $eshell-test-value[0 2 4]")
+                   '("zero" "two" "four")))))
+
+(ert-deftest esh-var-test/interp-var-string-split-indices ()
+  "Interpolate string variable with string splitter and indices"
+  (let ((eshell-test-value "zero:one:two:three:four"))
+    (should (equal (eshell-test-command-result "echo $eshell-test-value[: 0]")
+                   "zero"))
+    (should (equal (eshell-test-command-result "echo $eshell-test-value[: 0 2]")
+                   '("zero" "two")))))
+
+(ert-deftest esh-var-test/interp-var-regexp-split-indices ()
+  "Interpolate string variable with regexp splitter and indices"
+  (let ((eshell-test-value "zero:one!two:three!four"))
+    (should (equal (eshell-test-command-result
+                    "echo $eshell-test-value['[:!]' 0]")
+                   "zero"))
+    (should (equal (eshell-test-command-result
+                    "echo $eshell-test-value['[:!]' 0 2]")
+                   '("zero" "two")))
+    (should (equal (eshell-test-command-result
+                    "echo $eshell-test-value[\"[:!]\" 0]")
+                   "zero"))
+    (should (equal (eshell-test-command-result
+                    "echo $eshell-test-value[\"[:!]\" 0 2]")
+                   '("zero" "two")))))
+
+(ert-deftest esh-var-test/interp-var-assoc ()
+  "Interpolate alist variable with index"
+  (let ((eshell-test-value '(("foo" . 1))))
+    (should (eq (eshell-test-command-result "echo $eshell-test-value[foo]")
+                1))))
+
+(ert-deftest esh-var-test/interp-var-length-list ()
+  "Interpolate length of list variable"
+  (let ((eshell-test-value '((1 2) (3) (5 (6 7 8 9)))))
+    (should (eq (eshell-test-command-result "echo $#eshell-test-value") 3))
+    (should (eq (eshell-test-command-result "echo $#eshell-test-value[1]") 1))
+    (should (eq (eshell-test-command-result "echo $#eshell-test-value[2][1]")
+                4))))
+
+(ert-deftest esh-var-test/interp-var-length-string ()
+  "Interpolate length of string variable"
+  (let ((eshell-test-value "foobar"))
+    (should (eq (eshell-test-command-result "echo $#eshell-test-value") 6))))
+
+(ert-deftest esh-var-test/interp-var-length-alist ()
+  "Interpolate length of alist variable"
+  (let ((eshell-test-value '(("foo" . (1 2 3)))))
+    (should (eq (eshell-test-command-result "echo $#eshell-test-value") 1))
+    (should (eq (eshell-test-command-result "echo $#eshell-test-value[foo]")
+                3))))
+
 (ert-deftest esh-var-test/interp-lisp ()
   "Interpolate Lisp form evaluation"
   (should (equal (eshell-test-command-result "+ $(+ 1 2) 3") 6)))
@@ -112,6 +184,86 @@ esh-var-test/quoted-interp-quoted-var
                   "echo \"hi, $\\\"user-login-name\\\"\"")
                  (concat "hi, " user-login-name))))
 
+(ert-deftest esh-var-test/quoted-interp-var-indices ()
+  "Interpolate string variable with indices inside double-quotes"
+  (let ((eshell-test-value '("zero" "one" "two" "three" "four")))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[0]\"")
+                   "zero"))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[0 2]\"")
+                   '("zero" "two")))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[0 2 4]\"")
+                   '("zero" "two" "four")))))
+
+(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"))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[0]\"")
+                   "zero"))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[0 2]\"")
+                   '("zero" "two")))))
+
+(ert-deftest esh-var-test/quoted-interp-var-string-split-indices ()
+  "Interpolate string variable with string splitter and indices
+inside double-quotes"
+  (let ((eshell-test-value "zero:one:two:three:four"))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[: 0]\"")
+                   "zero"))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[: 0 2]\"")
+                   '("zero" "two")))))
+
+(ert-deftest esh-var-test/quoted-interp-var-regexp-split-indices ()
+  "Interpolate string variable with regexp splitter and indices"
+  (let ((eshell-test-value "zero:one!two:three!four"))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value['[:!]' 0]\"")
+                   "zero"))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value['[:!]' 0 2]\"")
+                   '("zero" "two")))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[\\\"[:!]\\\" 0]\"")
+                   "zero"))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[\\\"[:!]\\\" 0 2]\"")
+                   '("zero" "two")))))
+
+(ert-deftest esh-var-test/quoted-interp-var-assoc ()
+  "Interpolate alist variable with index inside double-quotes"
+  (let ((eshell-test-value '(("foo" . 1))))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[foo]\"")
+                   1))))
+
+(ert-deftest esh-var-test/quoted-interp-var-length-list ()
+  "Interpolate length of list variable inside double-quotes"
+  (let ((eshell-test-value '((1 2) (3) (5 (6 7 8 9)))))
+    (should (eq (eshell-test-command-result "echo \"$#eshell-test-value\"") 3))
+    (should (eq (eshell-test-command-result "echo \"$#eshell-test-value[1]\"")
+                1))
+    (should (eq (eshell-test-command-result
+                 "echo \"$#eshell-test-value[2][1]\"")
+                4))))
+
+(ert-deftest esh-var-test/quoted-interp-var-length-string ()
+  "Interpolate length of string variable inside double-quotes"
+  (let ((eshell-test-value "foobar"))
+    (should (eq (eshell-test-command-result "echo \"$#eshell-test-value\"")
+                6))))
+
+(ert-deftest esh-var-test/quoted-interp-var-length-alist ()
+  "Interpolate length of alist variable inside double-quotes"
+  (let ((eshell-test-value '(("foo" . (1 2 3)))))
+    (should (eq (eshell-test-command-result "echo \"$#eshell-test-value\"") 1))
+    (should (eq (eshell-test-command-result "echo \"$#eshell-test-value[foo]\"")
+                3))))
+
 (ert-deftest esh-var-test/quoted-interp-lisp ()
   "Interpolate Lisp form evaluation inside double-quotes"
   (should (equal (eshell-test-command-result
-- 
2.25.1


[-- Attachment #6: 0005-Allow-splitting-strings-in-Eshell-expansions-with-pl.patch --]
[-- Type: text/plain, Size: 4427 bytes --]

From 2d73ca67b844df6e1ed0319f5d454646d685caaa Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Tue, 1 Mar 2022 18:53:42 -0800
Subject: [PATCH 5/5] Allow splitting strings in Eshell expansions with "plain"
 strings

Since '$var[hello 0]' doesn't make sense when 'var' is a string, the
previous restriction was unnecessary.

* lisp/eshell/esh-var.el (Commentary): Update documentation.
(eshell-apply-indices): Allow "plain" strings to split strings.

* test/lisp/eshell/esh-var-test.el
(esh-var-test/interp-var-string-split-indices)
(esh-var-test/quoted-interp-var-string-split-indices): Update tests.

* doc/misc/eshell.texi (Dollars expansion): Update documentation.
---
 doc/misc/eshell.texi              |  6 +++---
 lisp/eshell/esh-var.el            | 17 +++++++----------
 test/lisp/eshell/esh-var-tests.el | 12 ++++++++++++
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 3301a854eb..5581e5cd9e 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1055,9 +1055,9 @@ Dollars Expansion
 
 @item $@var{expr}[@var{regexp} @var{i...}]
 As above (when @var{expr} expands to a string), but use @var{regexp}
-to split the string.  @var{regexp} can be any form other than a number
-or a plain variable name.  For example, @samp{$@var{var}[: 0]} will
-return the first element of a colon-delimited string.
+to split the string.  @var{regexp} can be any form other than a
+number.  For example, @samp{$@var{var}[: 0]} will return the first
+element of a colon-delimited string.
 
 @item $#@var{expr}
 Expands to the length of the result of @var{expr}, an expression in
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 6f08a3fbc4..af89e35f55 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -74,9 +74,8 @@
 ;;   $EXPR["\\\\" 10]
 ;;
 ;; Separate on backslash characters.  Actually, the first argument --
-;; if it doesn't have the form of a number, or a plain variable name
-;; -- can be any regular expression.  So to split on numbers, use
-;; '$EXPR["[0-9]+" 10 20]'.
+;; if it doesn't have the form of a number -- can be any regular
+;; expression.  So to split on numbers, use '$EXPR["[0-9]+" 10 20]'.
 ;;
 ;;   $EXPR[hello]
 ;;
@@ -566,13 +565,11 @@ eshell-apply-indices
   (while indices
     (let ((refs (car indices)))
       (when (stringp value)
-	(let (separator)
-	  (if (not (or (not (stringp (caar indices)))
-		       (string-match
-			(concat "^" eshell-variable-name-regexp "$")
-			(caar indices))))
-	      (setq separator (caar indices)
-		    refs (cdr refs)))
+	(let (separator (index (caar indices)))
+          (when (and (stringp index)
+                     (not (get-text-property 0 'number index)))
+            (setq separator index
+                  refs (cdr refs)))
 	  (setq value
 		(mapcar #'eshell-convert
 			(split-string value separator)))))
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index e679174939..d09dd614de 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -84,6 +84,11 @@ esh-var-test/interp-var-string-split-indices
     (should (equal (eshell-test-command-result "echo $eshell-test-value[: 0]")
                    "zero"))
     (should (equal (eshell-test-command-result "echo $eshell-test-value[: 0 2]")
+                   '("zero" "two"))))
+  (let ((eshell-test-value "zeroXoneXtwoXthreeXfour"))
+    (should (equal (eshell-test-command-result "echo $eshell-test-value[X 0]")
+                   "zero"))
+    (should (equal (eshell-test-command-result "echo $eshell-test-value[X 0 2]")
                    '("zero" "two")))))
 
 (ert-deftest esh-var-test/interp-var-regexp-split-indices ()
@@ -216,6 +221,13 @@ esh-var-test/quoted-interp-var-string-split-indices
                    "zero"))
     (should (equal (eshell-test-command-result
                     "echo \"$eshell-test-value[: 0 2]\"")
+                   '("zero" "two"))))
+  (let ((eshell-test-value "zeroXoneXtwoXthreeXfour"))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[X 0]\"")
+                   "zero"))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[X 0 2]\"")
                    '("zero" "two")))))
 
 (ert-deftest esh-var-test/quoted-interp-var-regexp-split-indices ()
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation
  2022-03-03  6:35 bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation Jim Porter
@ 2022-03-03 13:58 ` Lars Ingebrigtsen
  2022-03-03 16:57 ` Eli Zaretskii
  2022-03-03 17:03 ` Eli Zaretskii
  2 siblings, 0 replies; 13+ messages in thread
From: Lars Ingebrigtsen @ 2022-03-03 13:58 UTC (permalink / raw)
  To: Jim Porter; +Cc: 54227

Jim Porter <jporterbugs@gmail.com> writes:

> Attached is a patch series to fix these cases, as well as a lot more
> tests for Eshell variable interpolation, since this can get
> tricky. I've also updated the documentation to clarify how indexing
> works. I didn't initially realize that you could do quite so many
> complex things with it, so I figured it'd be good to explain in more
> detail so it's easier to discover.

Looks good to me; pushed to Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation
  2022-03-03  6:35 bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation Jim Porter
  2022-03-03 13:58 ` Lars Ingebrigtsen
@ 2022-03-03 16:57 ` Eli Zaretskii
  2022-03-03 17:03 ` Eli Zaretskii
  2 siblings, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2022-03-03 16:57 UTC (permalink / raw)
  To: Jim Porter; +Cc: 54227

> From: Jim Porter <jporterbugs@gmail.com>
> Date: Wed, 2 Mar 2022 22:35:22 -0800
> 
> +@item $@var{expr}[@var{i...}]
> +Expands to the @var{i}th element of the result of @var{expr}, an
> +expression in one of the above forms listed here.  If multiple indices
> +are supplied, this will return a list containing the elements for each
> +index.  If @var{expr}'s value is a string, it will first be split at
> +whitespace to make it a list.  If @var{expr}'s value is an alist
> +(@pxref{Association List Type, Association Lists, , elisp, The Emacs
> +Lisp Reference Manual}), this will call @code{assoc} on the result of
> +@var{expr}, returning the @code{cdr} of the element of the result
> +whose car is equal to @code{"i"}.

Thanks.

I think the documentation parts of this should be reworded to be
consistent with the manual's audience.  The Eshell manual is for
users, not necessarily for Lisp programmers.  So explaining features
in terms of Emacs primitives, such as assoc and car/cdr/caar is not
the best way of documenting them.

Can we reword this text such that the way the feature works is clear
to non-programmers as well?





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation
  2022-03-03  6:35 bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation Jim Porter
  2022-03-03 13:58 ` Lars Ingebrigtsen
  2022-03-03 16:57 ` Eli Zaretskii
@ 2022-03-03 17:03 ` Eli Zaretskii
  2022-03-03 17:56   ` Jim Porter
  2 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2022-03-03 17:03 UTC (permalink / raw)
  To: Jim Porter; +Cc: 54227

> From: Jim Porter <jporterbugs@gmail.com>
> Date: Wed, 2 Mar 2022 22:35:22 -0800
> 
> +(defmacro eshell-with-temp-command (command &rest body)
> +  "Narrow the buffer to COMMAND and execute the forms in BODY.

What does it mean to "narrow the buffer to COMMAND"?

Imagine that the user only sees this one line of the doc string --
that actually happens in apropos commands.  How can such a user
understand what this macro does?

> +COMMAND can either be a string, or a cons cell demarcating a
> +buffer region.  If COMMAND is a string, temporarily insert it
> +into the buffer before narrowing.  Point will be set to the
> +beginning of the narrowed region.

After reading this several time and looking at the implementation, I'm
beginning to think that COMMAND is not a good name for this argument.

> +(defun eshell-parse-inner-double-quote (bound)
> +  "Parse the inner part of a double quoted string.
> +The string to parse starts at point and ends at BOUND.
> +
> +If Eshell is currently parsing a quoted string and there are any
> +backslash-escaped characters, this will return the unescaped
> +string, updating point to BOUND.  Otherwise, this returns nil and
> +leaves point where it was."

This seems to just unescape characters in the string?  If so, "parse"
is not the best name for it, and the first line of the doc string
should say "unescape", not "parse".

Thanks.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation
  2022-03-03 17:03 ` Eli Zaretskii
@ 2022-03-03 17:56   ` Jim Porter
  2022-03-03 18:43     ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Jim Porter @ 2022-03-03 17:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 54227

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

On 3/3/2022 9:03 AM, Eli Zaretskii wrote:
>> From: Jim Porter <jporterbugs@gmail.com>
>> Date: Wed, 2 Mar 2022 22:35:22 -0800
>>
>> +(defmacro eshell-with-temp-command (command &rest body)
>> +  "Narrow the buffer to COMMAND and execute the forms in BODY.
> 
> What does it mean to "narrow the buffer to COMMAND"?
> 
> Imagine that the user only sees this one line of the doc string --
> that actually happens in apropos commands.  How can such a user
> understand what this macro does?

The macro's job is to take an Eshell command (or some fragment thereof) 
and narrow the buffer so that it's just looking at that part. This is to 
make sure that whatever is called in the body knows where to start and 
stop looking.

I agree that this isn't very clear, but I had trouble coming up with a 
concise explanation. It's essentially a workaround for how Eshell 
expects things; a lot of the Eshell command parsing functions operate on 
a range of text in the buffer. Normally, if you wanted to use those 
functions with a temporary string, you'd use `with-temp-buffer' and 
insert the string there. That doesn't work here though, since Eshell 
uses lots of buffer-local state. This function tries to abstract that 
out in a way that's useful for a few different places in Eshell.

If you have any ideas about how to improve the wording, I'm happy to 
update it though. I'll try to keep thinking as well.

> 
>> +COMMAND can either be a string, or a cons cell demarcating a
>> +buffer region.  If COMMAND is a string, temporarily insert it
>> +into the buffer before narrowing.  Point will be set to the
>> +beginning of the narrowed region.
> 
> After reading this several time and looking at the implementation, I'm
> beginning to think that COMMAND is not a good name for this argument.

Perhaps not. That comes from `eshell-parse-command' below, which takes a 
COMMAND argument of the same possible forms. There's probably a better 
name to use...

>> +(defun eshell-parse-inner-double-quote (bound)
[snip]
> 
> This seems to just unescape characters in the string?  If so, "parse"
> is not the best name for it, and the first line of the doc string
> should say "unescape", not "parse".

Fixed.

I also reworded the manual entries. Hopefully they're a bit better.

Finally, I made a very small tweak to how quoted variable expansions 
(like $"foo") are detected. The old code wasn't reporting the right 
error if you typed:

   echo $\"foo\"

That's not correct and it should be considered invalid syntax (which it 
is now).

[-- Attachment #2: 0001-Improve-wording-of-Eshell-variable-interpolation-cod.patch --]
[-- Type: text/plain, Size: 5254 bytes --]

From cbf43bb3d9fbb05f6a67d780cb8053ff5e0d700b Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 3 Mar 2022 09:37:25 -0800
Subject: [PATCH] Improve wording of Eshell variable interpolation
 code/documentation

* lisp/eshell/esh-arg.el (eshell-unescape-inner-double-quote): Rename
from 'eshell-parse-inner-double-quote'.

* lisp/eshell/esh-var.el (eshell-parse-variable-ref): Use
'eshell-unescape-inner-double-quote' and improve robustness of quoted
variable name matching.
(eshell-parse-indices): Use 'eshell-unescape-inner-double-quote'.

* doc/misc/eshell.texi (Dollars Expansion): Improve wording of
subscript notation.
---
 doc/misc/eshell.texi   | 19 ++++++++++---------
 lisp/eshell/esh-arg.el |  4 ++--
 lisp/eshell/esh-var.el | 14 ++++++++------
 3 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 5581e5cd9e..47f8902d5a 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1043,15 +1043,16 @@ Dollars Expansion
 index.  If @var{expr}'s value is a string, it will first be split at
 whitespace to make it a list.  If @var{expr}'s value is an alist
 (@pxref{Association List Type, Association Lists, , elisp, The Emacs
-Lisp Reference Manual}), this will call @code{assoc} on the result of
-@var{expr}, returning the @code{cdr} of the element of the result
-whose car is equal to @code{"i"}.  Raises an error if the value is not
-a sequence (@pxref{Sequences Arrays Vectors, Sequences, , elisp, The
-Emacs Lisp Reference Manual}).
+Lisp Reference Manual}), this will return the value associated with
+the key @code{"i"}.
 
-Multiple sets of indices can also be specified. For example, if
-@var{var} is a list of lists, @samp{$@var{var}[0][0]} is equivalent to
-@samp{(caar @var{var})}.
+Multiple sets of indices can also be specified.  For example, if
+@var{var} is @samp{((1 2) (3 4))}, then @samp{$@var{var}[0][1]} will
+expand to @code{2}.
+
+Raises an error if the result of @var{expr} is not a string or a
+sequence (@pxref{Sequences Arrays Vectors, Sequences, , elisp, The
+Emacs Lisp Reference Manual}.)
 
 @item $@var{expr}[@var{regexp} @var{i...}]
 As above (when @var{expr} expands to a string), but use @var{regexp}
@@ -1064,7 +1065,7 @@ Dollars Expansion
 one of the above forms.  For example, @samp{$#@var{var}} returns the
 length of the variable @var{var} and @samp{$#@var{var}[0]} returns the
 length of the first element of @var{var}.  Again, raises an error if
-the result of @var{expr} is not a sequence.
+the result of @var{expr} is not a string or a sequence.
 
 @end table
 
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index e19481c4ba..a3c961f546 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -354,8 +354,8 @@ eshell-parse-double-quote
 		  (list 'eshell-escape-arg arg))))
 	  (goto-char (1+ end)))))))
 
-(defun eshell-parse-inner-double-quote (bound)
-  "Parse the inner part of a double quoted string.
+(defun eshell-unescape-inner-double-quote (bound)
+  "Unescape the inner part of a double quoted string.
 The string to parse starts at point and ends at BOUND.
 
 If Eshell is currently parsing a quoted string and there are any
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index af89e35f55..8746f2bb93 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -437,7 +437,7 @@ eshell-parse-variable-ref
             `(eshell-convert
               (eshell-command-to-value
                (eshell-as-subcommand
-                ,(let ((subcmd (or (eshell-parse-inner-double-quote end)
+                ,(let ((subcmd (or (eshell-unescape-inner-double-quote end)
                                    (cons (point) end)))
                        (eshell-current-quoted nil))
                    (eshell-parse-command subcmd)))))
@@ -470,13 +470,15 @@ eshell-parse-variable-ref
     (condition-case nil
         `(eshell-command-to-value
           (eshell-lisp-command
-           ',(read (or (eshell-parse-inner-double-quote (point-max))
+           ',(read (or (eshell-unescape-inner-double-quote (point-max))
                        (current-buffer)))))
       (end-of-file
        (throw 'eshell-incomplete ?\())))
-   ((looking-at (rx (or "'" "\"" "\\\"")))
-    (eshell-with-temp-command (or (eshell-parse-inner-double-quote (point-max))
-                                  (cons (point) (point-max)))
+   ((looking-at (rx-to-string
+                 `(or "'" ,(if eshell-current-quoted "\\\"" "\""))))
+    (eshell-with-temp-command
+        (or (eshell-unescape-inner-double-quote (point-max))
+            (cons (point) (point-max)))
       (let ((name (if (eq (char-after) ?\')
                       (eshell-parse-literal-quote)
                     (eshell-parse-double-quote))))
@@ -506,7 +508,7 @@ eshell-parse-indices
 	(if (not end)
 	    (throw 'eshell-incomplete ?\[)
 	  (forward-char)
-          (eshell-with-temp-command (or (eshell-parse-inner-double-quote end)
+          (eshell-with-temp-command (or (eshell-unescape-inner-double-quote end)
                                         (cons (point) end))
 	    (let (eshell-glob-function (eshell-current-quoted nil))
 	      (setq indices (cons (eshell-parse-arguments
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation
  2022-03-03 17:56   ` Jim Porter
@ 2022-03-03 18:43     ` Eli Zaretskii
  2022-03-03 19:29       ` Jim Porter
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2022-03-03 18:43 UTC (permalink / raw)
  To: Jim Porter; +Cc: 54227

> Cc: 54227@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> Date: Thu, 3 Mar 2022 09:56:14 -0800
> 
> >> +(defmacro eshell-with-temp-command (command &rest body)
> >> +  "Narrow the buffer to COMMAND and execute the forms in BODY.
> > 
> > What does it mean to "narrow the buffer to COMMAND"?
> > 
> > Imagine that the user only sees this one line of the doc string --
> > that actually happens in apropos commands.  How can such a user
> > understand what this macro does?
> 
> The macro's job is to take an Eshell command (or some fragment thereof) 
> and narrow the buffer so that it's just looking at that part. This is to 
> make sure that whatever is called in the body knows where to start and 
> stop looking.
> 
> I agree that this isn't very clear, but I had trouble coming up with a 
> concise explanation. It's essentially a workaround for how Eshell 
> expects things; a lot of the Eshell command parsing functions operate on 
> a range of text in the buffer. Normally, if you wanted to use those 
> functions with a temporary string, you'd use `with-temp-buffer' and 
> insert the string there. That doesn't work here though, since Eshell 
> uses lots of buffer-local state. This function tries to abstract that 
> out in a way that's useful for a few different places in Eshell.
> 
> If you have any ideas about how to improve the wording, I'm happy to 
> update it though. I'll try to keep thinking as well.

Something like the below:

  (defmacro eshell-with-temp-command (region &rest body)
    "Narrow the buffer to REGION and execute the forms in BODY.

  REGION is a cons cell (START . END) that specifies the region
  to which to narrow the buffer.  REGION can also be a string,
  in which case the macro temporarily inserts it into the
  buffer at point, and narrows the buffer to the inserted string.
  Before executing BODY, point is set to the beginning of the
  narrowed REGION.

> diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
> index 5581e5cd9e..47f8902d5a 100644
> --- a/doc/misc/eshell.texi
> +++ b/doc/misc/eshell.texi
> @@ -1043,15 +1043,16 @@ Dollars Expansion
>  index.  If @var{expr}'s value is a string, it will first be split at
>  whitespace to make it a list.  If @var{expr}'s value is an alist
>  (@pxref{Association List Type, Association Lists, , elisp, The Emacs
> -Lisp Reference Manual}), this will call @code{assoc} on the result of
> -@var{expr}, returning the @code{cdr} of the element of the result
> -whose car is equal to @code{"i"}.  Raises an error if the value is not
> -a sequence (@pxref{Sequences Arrays Vectors, Sequences, , elisp, The
> -Emacs Lisp Reference Manual}).
> +Lisp Reference Manual}), this will return the value associated with
> +the key @code{"i"}.
>  
> -Multiple sets of indices can also be specified. For example, if
> -@var{var} is a list of lists, @samp{$@var{var}[0][0]} is equivalent to
> -@samp{(caar @var{var})}.
> +Multiple sets of indices can also be specified.  For example, if
> +@var{var} is @samp{((1 2) (3 4))}, then @samp{$@var{var}[0][1]} will
> +expand to @code{2}.

I would add to the last sentence: ", i.e.@: the second element of the
first list member (all indices are zero-based)."

Also, it sounds like you just dropped the ball on the alist use case?

> -(defun eshell-parse-inner-double-quote (bound)
> -  "Parse the inner part of a double quoted string.
> +(defun eshell-unescape-inner-double-quote (bound)
> +  "Unescape the inner part of a double quoted string.

"Unescape escaped characters of a double-quoted string."

Thanks.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation
  2022-03-03 18:43     ` Eli Zaretskii
@ 2022-03-03 19:29       ` Jim Porter
  2022-03-03 19:50         ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Jim Porter @ 2022-03-03 19:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 54227

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

On 3/3/2022 10:43 AM, Eli Zaretskii wrote:
>> Cc: 54227@debbugs.gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>> Date: Thu, 3 Mar 2022 09:56:14 -0800
>>
>> If you have any ideas about how to improve the wording, I'm happy to
>> update it though. I'll try to keep thinking as well.
> 
> Something like the below:
> 
>    (defmacro eshell-with-temp-command (region &rest body)
>      "Narrow the buffer to REGION and execute the forms in BODY.
> 
>    REGION is a cons cell (START . END) that specifies the region
>    to which to narrow the buffer.  REGION can also be a string,
>    in which case the macro temporarily inserts it into the
>    buffer at point, and narrows the buffer to the inserted string.
>    Before executing BODY, point is set to the beginning of the
>    narrowed REGION.

Thanks, updated to use that docstring.

>> diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
>> index 5581e5cd9e..47f8902d5a 100644
>> --- a/doc/misc/eshell.texi
>> +++ b/doc/misc/eshell.texi
>> @@ -1043,15 +1043,16 @@ Dollars Expansion
[snip]
>>   
>> -Multiple sets of indices can also be specified. For example, if
>> -@var{var} is a list of lists, @samp{$@var{var}[0][0]} is equivalent to
>> -@samp{(caar @var{var})}.
>> +Multiple sets of indices can also be specified.  For example, if
>> +@var{var} is @samp{((1 2) (3 4))}, then @samp{$@var{var}[0][1]} will
>> +expand to @code{2}.
> 
> I would add to the last sentence: ", i.e.@: the second element of the
> first list member (all indices are zero-based)."

Ok, added.

> Also, it sounds like you just dropped the ball on the alist use case?

I think we just had different ideas of how much detail was necessary. 
Given your above comment, I think I have a better idea of the level of 
detail, so I've expanded this section into a table. The single paragraph 
was a little too dense, so breaking it out into separate blocks for each 
data type makes it easier to provide a more thorough explanation.

>> -(defun eshell-parse-inner-double-quote (bound)
>> -  "Parse the inner part of a double quoted string.
>> +(defun eshell-unescape-inner-double-quote (bound)
>> +  "Unescape the inner part of a double quoted string.
> 
> "Unescape escaped characters of a double-quoted string."

Done, though I worded it as, "Unescape escaped characters inside a 
double-quoted string." I wanted to be extra-clear that this only 
operates on the bits *between* the double-quotes, but doesn't do 
anything with the surrounding double-quotes themselves.

[-- Attachment #2: 0001-Improve-wording-of-Eshell-variable-interpolation-cod.patch --]
[-- Type: text/plain, Size: 8351 bytes --]

From ea4c9b0b7770b73c1320cf4a99ad2ed36638c4ae Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 3 Mar 2022 09:37:25 -0800
Subject: [PATCH] Improve wording of Eshell variable interpolation
 code/documentation

* lisp/eshell/esh-arg.el (eshell-unescape-inner-double-quote): Rename
from 'eshell-parse-inner-double-quote'.

* lisp/eshell/esh-cmd.el (eshell-with-temp-command): Improve
docstring.

* lisp/eshell/esh-var.el (eshell-parse-variable-ref): Use
'eshell-unescape-inner-double-quote' and improve robustness of quoted
variable name matching.
(eshell-parse-indices): Use 'eshell-unescape-inner-double-quote'.

* doc/misc/eshell.texi (Dollars Expansion): Improve wording of
subscript notation.
---
 doc/misc/eshell.texi   | 41 ++++++++++++++++++++++++++++++-----------
 lisp/eshell/esh-arg.el |  4 ++--
 lisp/eshell/esh-cmd.el | 25 ++++++++++++++-----------
 lisp/eshell/esh-var.el | 14 ++++++++------
 4 files changed, 54 insertions(+), 30 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 5581e5cd9e..2df4de1cef 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1040,18 +1040,37 @@ Dollars Expansion
 Expands to the @var{i}th element of the result of @var{expr}, an
 expression in one of the above forms listed here.  If multiple indices
 are supplied, this will return a list containing the elements for each
-index.  If @var{expr}'s value is a string, it will first be split at
-whitespace to make it a list.  If @var{expr}'s value is an alist
-(@pxref{Association List Type, Association Lists, , elisp, The Emacs
-Lisp Reference Manual}), this will call @code{assoc} on the result of
-@var{expr}, returning the @code{cdr} of the element of the result
-whose car is equal to @code{"i"}.  Raises an error if the value is not
-a sequence (@pxref{Sequences Arrays Vectors, Sequences, , elisp, The
+index.  The exact behavior depends on the type of @var{expr}'s value:
+
+@table @asis
+
+@item a sequence
+Expands to the element at the (zero-based) index @var{i} of the
+sequence (@pxref{Sequences Arrays Vectors, Sequences, , elisp, The
 Emacs Lisp Reference Manual}).
 
-Multiple sets of indices can also be specified. For example, if
-@var{var} is a list of lists, @samp{$@var{var}[0][0]} is equivalent to
-@samp{(caar @var{var})}.
+@item a string
+Split the string at whitespace, and then expand to the @var{i}th
+element of the resulting sequence.
+
+@item an alist
+If @var{i} is a non-numeric value, expand to the value associated with
+the key @code{"i"}. For example, if @var{var} is @samp{(("dog"
+. "fido") ("cat" . "felix"))}, then @samp{$@var{var}[dog]} expands to
+@code{"fido"}.  Otherwise, this behaves as with sequences; e.g.,
+@samp{$@var{var}[0]} expands to @code{("dog" . "fido")}.
+@xref{Association List Type, Association Lists, , elisp, The Emacs
+Lisp Reference Manual}.
+
+@item anything else
+Raises an error.
+
+@end table
+
+Multiple sets of indices can also be specified.  For example, if
+@var{var} is @samp{((1 2) (3 4))}, then @samp{$@var{var}[0][1]} will
+expand to @code{2}, i.e.@: the second element of the first list member
+(all indices are zero-based).
 
 @item $@var{expr}[@var{regexp} @var{i...}]
 As above (when @var{expr} expands to a string), but use @var{regexp}
@@ -1064,7 +1083,7 @@ Dollars Expansion
 one of the above forms.  For example, @samp{$#@var{var}} returns the
 length of the variable @var{var} and @samp{$#@var{var}[0]} returns the
 length of the first element of @var{var}.  Again, raises an error if
-the result of @var{expr} is not a sequence.
+the result of @var{expr} is not a string or a sequence.
 
 @end table
 
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index e19481c4ba..ee3f907f85 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -354,8 +354,8 @@ eshell-parse-double-quote
 		  (list 'eshell-escape-arg arg))))
 	  (goto-char (1+ end)))))))
 
-(defun eshell-parse-inner-double-quote (bound)
-  "Parse the inner part of a double quoted string.
+(defun eshell-unescape-inner-double-quote (bound)
+  "Unescape escaped characters inside a double-quoted string.
 The string to parse starts at point and ends at BOUND.
 
 If Eshell is currently parsing a quoted string and there are any
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 04b54d9d79..8be1136e31 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -350,24 +350,27 @@ eshell-complete-lisp-symbols
 
 (defvar eshell--sep-terms)
 
-(defmacro eshell-with-temp-command (command &rest body)
-  "Narrow the buffer to COMMAND and execute the forms in BODY.
-COMMAND can either be a string, or a cons cell demarcating a
-buffer region.  If COMMAND is a string, temporarily insert it
-into the buffer before narrowing.  Point will be set to the
-beginning of the narrowed region.
+(defmacro eshell-with-temp-command (region &rest body)
+  "Narrow the buffer to REGION and execute the forms in BODY.
+
+REGION is a cons cell (START . END) that specifies the region to
+which to narrow the buffer.  REGION can also be a string, in
+which case the macro temporarily inserts it into the buffer at
+point, and narrows the buffer to the inserted string.  Before
+executing BODY, point is set to the beginning of the narrowed
+REGION.
 
 The value returned is the last form in BODY."
   (declare (indent 1))
-  `(let ((cmd ,command))
-     (if (stringp cmd)
+  `(let ((reg ,region))
+     (if (stringp reg)
          ;; Since parsing relies partly on buffer-local state
          ;; (e.g. that of `eshell-parse-argument-hook'), we need to
          ;; perform the parsing in the Eshell buffer.
          (let ((begin (point)) end
 	       (inhibit-point-motion-hooks t))
            (with-silent-modifications
-             (insert cmd)
+             (insert reg)
              (setq end (point))
              (unwind-protect
                  (save-restriction
@@ -376,8 +379,8 @@ eshell-with-temp-command
                    ,@body)
                (delete-region begin end))))
        (save-restriction
-         (narrow-to-region (car cmd) (cdr cmd))
-         (goto-char (car cmd))
+         (narrow-to-region (car reg) (cdr reg))
+         (goto-char (car reg))
          ,@body))))
 
 (defun eshell-parse-command (command &optional args toplevel)
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index af89e35f55..8746f2bb93 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -437,7 +437,7 @@ eshell-parse-variable-ref
             `(eshell-convert
               (eshell-command-to-value
                (eshell-as-subcommand
-                ,(let ((subcmd (or (eshell-parse-inner-double-quote end)
+                ,(let ((subcmd (or (eshell-unescape-inner-double-quote end)
                                    (cons (point) end)))
                        (eshell-current-quoted nil))
                    (eshell-parse-command subcmd)))))
@@ -470,13 +470,15 @@ eshell-parse-variable-ref
     (condition-case nil
         `(eshell-command-to-value
           (eshell-lisp-command
-           ',(read (or (eshell-parse-inner-double-quote (point-max))
+           ',(read (or (eshell-unescape-inner-double-quote (point-max))
                        (current-buffer)))))
       (end-of-file
        (throw 'eshell-incomplete ?\())))
-   ((looking-at (rx (or "'" "\"" "\\\"")))
-    (eshell-with-temp-command (or (eshell-parse-inner-double-quote (point-max))
-                                  (cons (point) (point-max)))
+   ((looking-at (rx-to-string
+                 `(or "'" ,(if eshell-current-quoted "\\\"" "\""))))
+    (eshell-with-temp-command
+        (or (eshell-unescape-inner-double-quote (point-max))
+            (cons (point) (point-max)))
       (let ((name (if (eq (char-after) ?\')
                       (eshell-parse-literal-quote)
                     (eshell-parse-double-quote))))
@@ -506,7 +508,7 @@ eshell-parse-indices
 	(if (not end)
 	    (throw 'eshell-incomplete ?\[)
 	  (forward-char)
-          (eshell-with-temp-command (or (eshell-parse-inner-double-quote end)
+          (eshell-with-temp-command (or (eshell-unescape-inner-double-quote end)
                                         (cons (point) end))
 	    (let (eshell-glob-function (eshell-current-quoted nil))
 	      (setq indices (cons (eshell-parse-arguments
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation
  2022-03-03 19:29       ` Jim Porter
@ 2022-03-03 19:50         ` Eli Zaretskii
  2022-03-05 20:06           ` Jim Porter
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2022-03-03 19:50 UTC (permalink / raw)
  To: Jim Porter; +Cc: 54227

> Cc: 54227@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> Date: Thu, 3 Mar 2022 11:29:49 -0800
> 
> On 3/3/2022 10:43 AM, Eli Zaretskii wrote:
> >> Cc: 54227@debbugs.gnu.org
> >> From: Jim Porter <jporterbugs@gmail.com>
> >> Date: Thu, 3 Mar 2022 09:56:14 -0800
> >>
> >> If you have any ideas about how to improve the wording, I'm happy to
> >> update it though. I'll try to keep thinking as well.
> > 
> > Something like the below:
> > 
> >    (defmacro eshell-with-temp-command (region &rest body)
> >      "Narrow the buffer to REGION and execute the forms in BODY.
> > 
> >    REGION is a cons cell (START . END) that specifies the region
> >    to which to narrow the buffer.  REGION can also be a string,
> >    in which case the macro temporarily inserts it into the
> >    buffer at point, and narrows the buffer to the inserted string.
> >    Before executing BODY, point is set to the beginning of the
> >    narrowed REGION.
> 
> Thanks, updated to use that docstring.
> 
> >> diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
> >> index 5581e5cd9e..47f8902d5a 100644
> >> --- a/doc/misc/eshell.texi
> >> +++ b/doc/misc/eshell.texi
> >> @@ -1043,15 +1043,16 @@ Dollars Expansion
> [snip]
> >>   
> >> -Multiple sets of indices can also be specified. For example, if
> >> -@var{var} is a list of lists, @samp{$@var{var}[0][0]} is equivalent to
> >> -@samp{(caar @var{var})}.
> >> +Multiple sets of indices can also be specified.  For example, if
> >> +@var{var} is @samp{((1 2) (3 4))}, then @samp{$@var{var}[0][1]} will
> >> +expand to @code{2}.
> > 
> > I would add to the last sentence: ", i.e.@: the second element of the
> > first list member (all indices are zero-based)."
> 
> Ok, added.
> 
> > Also, it sounds like you just dropped the ball on the alist use case?
> 
> I think we just had different ideas of how much detail was necessary. 
> Given your above comment, I think I have a better idea of the level of 
> detail, so I've expanded this section into a table. The single paragraph 
> was a little too dense, so breaking it out into separate blocks for each 
> data type makes it easier to provide a more thorough explanation.
> 
> >> -(defun eshell-parse-inner-double-quote (bound)
> >> -  "Parse the inner part of a double quoted string.
> >> +(defun eshell-unescape-inner-double-quote (bound)
> >> +  "Unescape the inner part of a double quoted string.
> > 
> > "Unescape escaped characters of a double-quoted string."
> 
> Done, though I worded it as, "Unescape escaped characters inside a 
> double-quoted string." I wanted to be extra-clear that this only 
> operates on the bits *between* the double-quotes, but doesn't do 
> anything with the surrounding double-quotes themselves.

Thanks, this now LGTM.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation
  2022-03-03 19:50         ` Eli Zaretskii
@ 2022-03-05 20:06           ` Jim Porter
  2022-03-05 21:44             ` Jim Porter
  0 siblings, 1 reply; 13+ messages in thread
From: Jim Porter @ 2022-03-05 20:06 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 54227

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

On 3/3/2022 11:50 AM, Eli Zaretskii wrote:
> Thanks, this now LGTM.

Thanks.

I found one more issue with the code though: the subscript operator 
doesn't work on subcommands. For example, from "emacs -Q --eval '(eshell)'":

   ~ $ echo ${*echo -e "hi\nbye"}[0]
   ("hi" "bye")

Since `${COMMAND}' forms split the output line-by-line, you'd expect 
this to say "hi", but the subscript operator is a no-op in this case. 
This was previously documented to work in the docstring for 
`eshell-apply-indices':

   For example, to retrieve the second element of a user's record in
   '/etc/passwd', the variable reference would look like:

     ${grep johnw /etc/passwd}[: 2]

I also updated the manual to indicate that this is possible (though I 
didn't provide any direct examples), since I thought this already worked 
based on that docstring.

Attached is a patch with some tests for this.

Just a note: using subscript on `$<COMMAND>' forms is probably not 
super-useful (at least not currently), though I added support for it 
anyway for consistency and future improvement. Since the result of that 
form is the name of a temp file, there's not much reason to do something 
like `$<COMMAND>[0]'. However in the future, if the subscript operator 
were more advanced, you could do something like `$<COMMAND>[/ *]' to 
split the file name by directory separators. (The "*" is a suggested 
feature in the "Bugs and ideas" section to return the whole list.)

[-- Attachment #2: 0001-Support-applying-indices-to-more-Eshell-dollar-expan.patch --]
[-- Type: text/plain, Size: 5502 bytes --]

From 1e7bb60551612b16ac5b9490603eb93d5db1921d Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 5 Mar 2022 11:45:49 -0800
Subject: [PATCH] Support applying indices to more Eshell dollar expansions

For example, '${echo -e "hi\nbye"}[1]' should expand to "bye".

* lisp/eshell/esh-var.el (eshell-parse-variable-ref): Support applying
indices to '${}', '$()', and '$<>' forms.

* lisp/eshell/esh-var-tests.el (esh-var-test/interp-lisp-indices)
(esh-var-test/interp-cmd-indices)
(esh-var-test/interp-cmd-external-indices)
(esh-var-test/quoted-interp-lisp-indices)
(esh-var-test/quoted-interp-cmd-indices): New tests.
---
 lisp/eshell/esh-var.el            | 28 ++++++++++++++++------------
 test/lisp/eshell/esh-var-tests.el | 23 +++++++++++++++++++++++
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index af89e35f55..e8df66d999 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -434,13 +434,15 @@ eshell-parse-variable-ref
           (throw 'eshell-incomplete ?\{)
         (forward-char)
         (prog1
-            `(eshell-convert
-              (eshell-command-to-value
-               (eshell-as-subcommand
-                ,(let ((subcmd (or (eshell-parse-inner-double-quote end)
-                                   (cons (point) end)))
-                       (eshell-current-quoted nil))
-                   (eshell-parse-command subcmd)))))
+            `(eshell-apply-indices
+              (eshell-convert
+               (eshell-command-to-value
+                (eshell-as-subcommand
+                 ,(let ((subcmd (or (eshell-parse-inner-double-quote end)
+                                    (cons (point) end)))
+                        (eshell-current-quoted nil))
+                    (eshell-parse-command subcmd)))))
+              indices)
           (goto-char (1+ end))))))
    ((eq (char-after) ?\<)
     (let ((end (eshell-find-delimiter ?\< ?\>)))
@@ -464,14 +466,16 @@ eshell-parse-variable-ref
                            ;; properly.  See bug#54190.
                            (list (function (lambda ()
                                    (delete-file ,temp))))))
-                   (quote ,temp)))
+                   (eshell-apply-indices ,temp indices)))
             (goto-char (1+ end)))))))
    ((eq (char-after) ?\()
     (condition-case nil
-        `(eshell-command-to-value
-          (eshell-lisp-command
-           ',(read (or (eshell-parse-inner-double-quote (point-max))
-                       (current-buffer)))))
+        `(eshell-apply-indices
+          (eshell-command-to-value
+           (eshell-lisp-command
+            ',(read (or (eshell-parse-inner-double-quote (point-max))
+                        (current-buffer)))))
+          indices)
       (end-of-file
        (throw 'eshell-incomplete ?\())))
    ((looking-at (rx (or "'" "\"" "\\\"")))
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index d09dd614de..1d051d681a 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -137,10 +137,18 @@ esh-var-test/interp-lisp
   "Interpolate Lisp form evaluation"
   (should (equal (eshell-test-command-result "+ $(+ 1 2) 3") 6)))
 
+(ert-deftest esh-var-test/interp-lisp-indices ()
+  "Interpolate Lisp form evaluation with index"
+  (should (equal (eshell-test-command-result "+ $(list 1 2)[1] 3") 5)))
+
 (ert-deftest esh-var-test/interp-cmd ()
   "Interpolate command result"
   (should (equal (eshell-test-command-result "+ ${+ 1 2} 3") 6)))
 
+(ert-deftest esh-var-test/interp-cmd-indices ()
+  "Interpolate command result with index"
+  (should (equal (eshell-test-command-result "+ ${list 1 2}[1] 3") 5)))
+
 (ert-deftest esh-var-test/interp-cmd-external ()
   "Interpolate command result from external command"
   (skip-unless (executable-find "echo"))
@@ -148,6 +156,13 @@ esh-var-test/interp-cmd-external
    (eshell-command-result-p "echo ${*echo hi}"
                             "hi\n")))
 
+(ert-deftest esh-var-test/interp-cmd-external-indices ()
+  "Interpolate command result from external command with index"
+  (skip-unless (executable-find "echo"))
+  (with-temp-eshell
+   (eshell-command-result-p "echo ${*echo \"hi\nbye\"}[1]"
+                            "bye\n")))
+
 (ert-deftest esh-var-test/interp-temp-cmd ()
   "Interpolate command result redirected to temp file"
   (should (equal (eshell-test-command-result "cat $<echo hi>") "hi")))
@@ -282,12 +297,20 @@ esh-var-test/quoted-interp-lisp
                   "echo \"hi $(concat \\\"the\\\" \\\"re\\\")\"")
                  "hi there")))
 
+(ert-deftest esh-var-test/quoted-interp-lisp-indices ()
+  "Interpolate Lisp form evaluation with index"
+  (should (equal (eshell-test-command-result "+ \"$(list 1 2)[1]\" 3") 5)))
+
 (ert-deftest esh-var-test/quoted-interp-cmd ()
   "Interpolate command result inside double-quotes"
   (should (equal (eshell-test-command-result
                   "echo \"hi ${echo \\\"there\\\"}\"")
                  "hi there")))
 
+(ert-deftest esh-var-test/quoted-interp-cmd-indices ()
+  "Interpolate command result with index inside double-quotes"
+  (should (equal (eshell-test-command-result "+ \"${list 1 2}[1]\" 3") 5)))
+
 (ert-deftest esh-var-test/quoted-interp-temp-cmd ()
   "Interpolate command result redirected to temp file inside double-quotes"
   (should (equal (eshell-test-command-result "cat \"$<echo hi>\"") "hi")))
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation
  2022-03-05 20:06           ` Jim Porter
@ 2022-03-05 21:44             ` Jim Porter
  2022-03-07 12:52               ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Jim Porter @ 2022-03-05 21:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 54227

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

On 3/5/2022 12:06 PM, Jim Porter wrote:
> Attached is a patch with some tests for this.

Oops, that patch had a conflict with the previous one. Here are both 
patches together, so they'll apply correctly.

[-- Attachment #2: 0001-Improve-wording-of-Eshell-variable-interpolation-cod.patch --]
[-- Type: text/plain, Size: 8355 bytes --]

From ea4c9b0b7770b73c1320cf4a99ad2ed36638c4ae Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 3 Mar 2022 09:37:25 -0800
Subject: [PATCH 1/2] Improve wording of Eshell variable interpolation
 code/documentation

* lisp/eshell/esh-arg.el (eshell-unescape-inner-double-quote): Rename
from 'eshell-parse-inner-double-quote'.

* lisp/eshell/esh-cmd.el (eshell-with-temp-command): Improve
docstring.

* lisp/eshell/esh-var.el (eshell-parse-variable-ref): Use
'eshell-unescape-inner-double-quote' and improve robustness of quoted
variable name matching.
(eshell-parse-indices): Use 'eshell-unescape-inner-double-quote'.

* doc/misc/eshell.texi (Dollars Expansion): Improve wording of
subscript notation.
---
 doc/misc/eshell.texi   | 41 ++++++++++++++++++++++++++++++-----------
 lisp/eshell/esh-arg.el |  4 ++--
 lisp/eshell/esh-cmd.el | 25 ++++++++++++++-----------
 lisp/eshell/esh-var.el | 14 ++++++++------
 4 files changed, 54 insertions(+), 30 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 5581e5cd9e..2df4de1cef 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1040,18 +1040,37 @@ Dollars Expansion
 Expands to the @var{i}th element of the result of @var{expr}, an
 expression in one of the above forms listed here.  If multiple indices
 are supplied, this will return a list containing the elements for each
-index.  If @var{expr}'s value is a string, it will first be split at
-whitespace to make it a list.  If @var{expr}'s value is an alist
-(@pxref{Association List Type, Association Lists, , elisp, The Emacs
-Lisp Reference Manual}), this will call @code{assoc} on the result of
-@var{expr}, returning the @code{cdr} of the element of the result
-whose car is equal to @code{"i"}.  Raises an error if the value is not
-a sequence (@pxref{Sequences Arrays Vectors, Sequences, , elisp, The
+index.  The exact behavior depends on the type of @var{expr}'s value:
+
+@table @asis
+
+@item a sequence
+Expands to the element at the (zero-based) index @var{i} of the
+sequence (@pxref{Sequences Arrays Vectors, Sequences, , elisp, The
 Emacs Lisp Reference Manual}).
 
-Multiple sets of indices can also be specified. For example, if
-@var{var} is a list of lists, @samp{$@var{var}[0][0]} is equivalent to
-@samp{(caar @var{var})}.
+@item a string
+Split the string at whitespace, and then expand to the @var{i}th
+element of the resulting sequence.
+
+@item an alist
+If @var{i} is a non-numeric value, expand to the value associated with
+the key @code{"i"}. For example, if @var{var} is @samp{(("dog"
+. "fido") ("cat" . "felix"))}, then @samp{$@var{var}[dog]} expands to
+@code{"fido"}.  Otherwise, this behaves as with sequences; e.g.,
+@samp{$@var{var}[0]} expands to @code{("dog" . "fido")}.
+@xref{Association List Type, Association Lists, , elisp, The Emacs
+Lisp Reference Manual}.
+
+@item anything else
+Raises an error.
+
+@end table
+
+Multiple sets of indices can also be specified.  For example, if
+@var{var} is @samp{((1 2) (3 4))}, then @samp{$@var{var}[0][1]} will
+expand to @code{2}, i.e.@: the second element of the first list member
+(all indices are zero-based).
 
 @item $@var{expr}[@var{regexp} @var{i...}]
 As above (when @var{expr} expands to a string), but use @var{regexp}
@@ -1064,7 +1083,7 @@ Dollars Expansion
 one of the above forms.  For example, @samp{$#@var{var}} returns the
 length of the variable @var{var} and @samp{$#@var{var}[0]} returns the
 length of the first element of @var{var}.  Again, raises an error if
-the result of @var{expr} is not a sequence.
+the result of @var{expr} is not a string or a sequence.
 
 @end table
 
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index e19481c4ba..ee3f907f85 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -354,8 +354,8 @@ eshell-parse-double-quote
 		  (list 'eshell-escape-arg arg))))
 	  (goto-char (1+ end)))))))
 
-(defun eshell-parse-inner-double-quote (bound)
-  "Parse the inner part of a double quoted string.
+(defun eshell-unescape-inner-double-quote (bound)
+  "Unescape escaped characters inside a double-quoted string.
 The string to parse starts at point and ends at BOUND.
 
 If Eshell is currently parsing a quoted string and there are any
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 04b54d9d79..8be1136e31 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -350,24 +350,27 @@ eshell-complete-lisp-symbols
 
 (defvar eshell--sep-terms)
 
-(defmacro eshell-with-temp-command (command &rest body)
-  "Narrow the buffer to COMMAND and execute the forms in BODY.
-COMMAND can either be a string, or a cons cell demarcating a
-buffer region.  If COMMAND is a string, temporarily insert it
-into the buffer before narrowing.  Point will be set to the
-beginning of the narrowed region.
+(defmacro eshell-with-temp-command (region &rest body)
+  "Narrow the buffer to REGION and execute the forms in BODY.
+
+REGION is a cons cell (START . END) that specifies the region to
+which to narrow the buffer.  REGION can also be a string, in
+which case the macro temporarily inserts it into the buffer at
+point, and narrows the buffer to the inserted string.  Before
+executing BODY, point is set to the beginning of the narrowed
+REGION.
 
 The value returned is the last form in BODY."
   (declare (indent 1))
-  `(let ((cmd ,command))
-     (if (stringp cmd)
+  `(let ((reg ,region))
+     (if (stringp reg)
          ;; Since parsing relies partly on buffer-local state
          ;; (e.g. that of `eshell-parse-argument-hook'), we need to
          ;; perform the parsing in the Eshell buffer.
          (let ((begin (point)) end
 	       (inhibit-point-motion-hooks t))
            (with-silent-modifications
-             (insert cmd)
+             (insert reg)
              (setq end (point))
              (unwind-protect
                  (save-restriction
@@ -376,8 +379,8 @@ eshell-with-temp-command
                    ,@body)
                (delete-region begin end))))
        (save-restriction
-         (narrow-to-region (car cmd) (cdr cmd))
-         (goto-char (car cmd))
+         (narrow-to-region (car reg) (cdr reg))
+         (goto-char (car reg))
          ,@body))))
 
 (defun eshell-parse-command (command &optional args toplevel)
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index af89e35f55..8746f2bb93 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -437,7 +437,7 @@ eshell-parse-variable-ref
             `(eshell-convert
               (eshell-command-to-value
                (eshell-as-subcommand
-                ,(let ((subcmd (or (eshell-parse-inner-double-quote end)
+                ,(let ((subcmd (or (eshell-unescape-inner-double-quote end)
                                    (cons (point) end)))
                        (eshell-current-quoted nil))
                    (eshell-parse-command subcmd)))))
@@ -470,13 +470,15 @@ eshell-parse-variable-ref
     (condition-case nil
         `(eshell-command-to-value
           (eshell-lisp-command
-           ',(read (or (eshell-parse-inner-double-quote (point-max))
+           ',(read (or (eshell-unescape-inner-double-quote (point-max))
                        (current-buffer)))))
       (end-of-file
        (throw 'eshell-incomplete ?\())))
-   ((looking-at (rx (or "'" "\"" "\\\"")))
-    (eshell-with-temp-command (or (eshell-parse-inner-double-quote (point-max))
-                                  (cons (point) (point-max)))
+   ((looking-at (rx-to-string
+                 `(or "'" ,(if eshell-current-quoted "\\\"" "\""))))
+    (eshell-with-temp-command
+        (or (eshell-unescape-inner-double-quote (point-max))
+            (cons (point) (point-max)))
       (let ((name (if (eq (char-after) ?\')
                       (eshell-parse-literal-quote)
                     (eshell-parse-double-quote))))
@@ -506,7 +508,7 @@ eshell-parse-indices
 	(if (not end)
 	    (throw 'eshell-incomplete ?\[)
 	  (forward-char)
-          (eshell-with-temp-command (or (eshell-parse-inner-double-quote end)
+          (eshell-with-temp-command (or (eshell-unescape-inner-double-quote end)
                                         (cons (point) end))
 	    (let (eshell-glob-function (eshell-current-quoted nil))
 	      (setq indices (cons (eshell-parse-arguments
-- 
2.25.1


[-- Attachment #3: 0002-Support-applying-indices-to-more-Eshell-dollar-expan.patch --]
[-- Type: text/plain, Size: 5505 bytes --]

From bbd74324632228a64388689e63ea0286632c0a32 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 5 Mar 2022 11:45:49 -0800
Subject: [PATCH 2/2] Support applying indices to more Eshell dollar expansions

For example, '${echo -e "hi\nbye"}[1]' should expand to "bye".

* lisp/eshell/esh-var.el (eshell-parse-variable-ref): Support applying
indices to '${}', '$()', and '$<>' forms.

* lisp/eshell/esh-var-tests.el (esh-var-test/interp-lisp-indices)
(esh-var-test/interp-cmd-indices)
(esh-var-test/interp-cmd-external-indices)
(esh-var-test/quoted-interp-lisp-indices)
(esh-var-test/quoted-interp-cmd-indices): New tests.
---
 lisp/eshell/esh-var.el            | 28 ++++++++++++++++------------
 test/lisp/eshell/esh-var-tests.el | 23 +++++++++++++++++++++++
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 8746f2bb93..ca4cbd744c 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -434,13 +434,15 @@ eshell-parse-variable-ref
           (throw 'eshell-incomplete ?\{)
         (forward-char)
         (prog1
-            `(eshell-convert
-              (eshell-command-to-value
-               (eshell-as-subcommand
-                ,(let ((subcmd (or (eshell-unescape-inner-double-quote end)
-                                   (cons (point) end)))
-                       (eshell-current-quoted nil))
-                   (eshell-parse-command subcmd)))))
+            `(eshell-apply-indices
+              (eshell-convert
+               (eshell-command-to-value
+                (eshell-as-subcommand
+                 ,(let ((subcmd (or (eshell-unescape-inner-double-quote end)
+                                    (cons (point) end)))
+                        (eshell-current-quoted nil))
+                    (eshell-parse-command subcmd)))))
+              indices)
           (goto-char (1+ end))))))
    ((eq (char-after) ?\<)
     (let ((end (eshell-find-delimiter ?\< ?\>)))
@@ -464,14 +466,16 @@ eshell-parse-variable-ref
                            ;; properly.  See bug#54190.
                            (list (function (lambda ()
                                    (delete-file ,temp))))))
-                   (quote ,temp)))
+                   (eshell-apply-indices ,temp indices)))
             (goto-char (1+ end)))))))
    ((eq (char-after) ?\()
     (condition-case nil
-        `(eshell-command-to-value
-          (eshell-lisp-command
-           ',(read (or (eshell-unescape-inner-double-quote (point-max))
-                       (current-buffer)))))
+        `(eshell-apply-indices
+          (eshell-command-to-value
+           (eshell-lisp-command
+            ',(read (or (eshell-unescape-inner-double-quote (point-max))
+                        (current-buffer)))))
+          indices)
       (end-of-file
        (throw 'eshell-incomplete ?\())))
    ((looking-at (rx-to-string
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index d09dd614de..1d051d681a 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -137,10 +137,18 @@ esh-var-test/interp-lisp
   "Interpolate Lisp form evaluation"
   (should (equal (eshell-test-command-result "+ $(+ 1 2) 3") 6)))
 
+(ert-deftest esh-var-test/interp-lisp-indices ()
+  "Interpolate Lisp form evaluation with index"
+  (should (equal (eshell-test-command-result "+ $(list 1 2)[1] 3") 5)))
+
 (ert-deftest esh-var-test/interp-cmd ()
   "Interpolate command result"
   (should (equal (eshell-test-command-result "+ ${+ 1 2} 3") 6)))
 
+(ert-deftest esh-var-test/interp-cmd-indices ()
+  "Interpolate command result with index"
+  (should (equal (eshell-test-command-result "+ ${list 1 2}[1] 3") 5)))
+
 (ert-deftest esh-var-test/interp-cmd-external ()
   "Interpolate command result from external command"
   (skip-unless (executable-find "echo"))
@@ -148,6 +156,13 @@ esh-var-test/interp-cmd-external
    (eshell-command-result-p "echo ${*echo hi}"
                             "hi\n")))
 
+(ert-deftest esh-var-test/interp-cmd-external-indices ()
+  "Interpolate command result from external command with index"
+  (skip-unless (executable-find "echo"))
+  (with-temp-eshell
+   (eshell-command-result-p "echo ${*echo \"hi\nbye\"}[1]"
+                            "bye\n")))
+
 (ert-deftest esh-var-test/interp-temp-cmd ()
   "Interpolate command result redirected to temp file"
   (should (equal (eshell-test-command-result "cat $<echo hi>") "hi")))
@@ -282,12 +297,20 @@ esh-var-test/quoted-interp-lisp
                   "echo \"hi $(concat \\\"the\\\" \\\"re\\\")\"")
                  "hi there")))
 
+(ert-deftest esh-var-test/quoted-interp-lisp-indices ()
+  "Interpolate Lisp form evaluation with index"
+  (should (equal (eshell-test-command-result "+ \"$(list 1 2)[1]\" 3") 5)))
+
 (ert-deftest esh-var-test/quoted-interp-cmd ()
   "Interpolate command result inside double-quotes"
   (should (equal (eshell-test-command-result
                   "echo \"hi ${echo \\\"there\\\"}\"")
                  "hi there")))
 
+(ert-deftest esh-var-test/quoted-interp-cmd-indices ()
+  "Interpolate command result with index inside double-quotes"
+  (should (equal (eshell-test-command-result "+ \"${list 1 2}[1]\" 3") 5)))
+
 (ert-deftest esh-var-test/quoted-interp-temp-cmd ()
   "Interpolate command result redirected to temp file inside double-quotes"
   (should (equal (eshell-test-command-result "cat \"$<echo hi>\"") "hi")))
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation
  2022-03-05 21:44             ` Jim Porter
@ 2022-03-07 12:52               ` Eli Zaretskii
  2022-03-08  3:38                 ` Jim Porter
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2022-03-07 12:52 UTC (permalink / raw)
  To: Jim Porter; +Cc: 54227

> From: Jim Porter <jporterbugs@gmail.com>
> Cc: 54227@debbugs.gnu.org
> Date: Sat, 5 Mar 2022 13:44:24 -0800
> 
> +If @var{i} is a non-numeric value, expand to the value associated with
> +the key @code{"i"}. For example, if @var{var} is @samp{(("dog"

Why did you need quotes in "i"?

Also, it's suppoed to be @var{i}, as in the other alternatives.

> +@item anything else
> +Raises an error.

We say "Signals an error".

Otherwise, LGTM, thanks.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation
  2022-03-07 12:52               ` Eli Zaretskii
@ 2022-03-08  3:38                 ` Jim Porter
  2022-03-08 13:56                   ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Jim Porter @ 2022-03-08  3:38 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 54227

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

On 3/7/2022 4:52 AM, Eli Zaretskii wrote:
>> From: Jim Porter <jporterbugs@gmail.com>
>> Cc: 54227@debbugs.gnu.org
>> Date: Sat, 5 Mar 2022 13:44:24 -0800
>>
>> +If @var{i} is a non-numeric value, expand to the value associated with
>> +the key @code{"i"}. For example, if @var{var} is @samp{(("dog"
> 
> Why did you need quotes in "i"?

That was my attempt to indicate that this:

   $foo[bar]

means (cdr (assoc "bar" foo)), not (cdr (assoc 'bar foo)) or (cdr (assoc 
bar foo)). Hopefully that's an ok way to express that. That said, this 
means the same thing too:

   $foo["bar"]

so it's not quite as simple as wrapping quotes around the subscript. 
This seemed like the simplest way to express the general idea, but it 
might be more technically correct to say something like, "... expand to 
the value associated with @var{i}'s value as a string." That wording is 
pretty confusing to me though...
> Also, it's suppoed to be @var{i}, as in the other alternatives.

Thanks, fixed.

>> +@item anything else
>> +Raises an error.
> 
> We say "Signals an error".

Fixed there, and in the entry below about the '$#foo' syntax.

[-- Attachment #2: 0001-Improve-wording-of-Eshell-variable-interpolation-cod.patch --]
[-- Type: text/plain, Size: 8517 bytes --]

From 48dbb3d9e1f8283440bf4695708a880a372f06d2 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 3 Mar 2022 09:37:25 -0800
Subject: [PATCH 1/2] Improve wording of Eshell variable interpolation
 code/documentation

* lisp/eshell/esh-arg.el (eshell-unescape-inner-double-quote): Rename
from 'eshell-parse-inner-double-quote'.

* lisp/eshell/esh-cmd.el (eshell-with-temp-command): Improve
docstring.

* lisp/eshell/esh-var.el (eshell-parse-variable-ref): Use
'eshell-unescape-inner-double-quote' and improve robustness of quoted
variable name matching.
(eshell-parse-indices): Use 'eshell-unescape-inner-double-quote'.

* doc/misc/eshell.texi (Dollars Expansion): Improve wording of
subscript notation.
---
 doc/misc/eshell.texi   | 43 ++++++++++++++++++++++++++++++------------
 lisp/eshell/esh-arg.el |  4 ++--
 lisp/eshell/esh-cmd.el | 25 +++++++++++++-----------
 lisp/eshell/esh-var.el | 14 ++++++++------
 4 files changed, 55 insertions(+), 31 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 5581e5cd9e..372e4c3ffb 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1040,18 +1040,37 @@ Dollars Expansion
 Expands to the @var{i}th element of the result of @var{expr}, an
 expression in one of the above forms listed here.  If multiple indices
 are supplied, this will return a list containing the elements for each
-index.  If @var{expr}'s value is a string, it will first be split at
-whitespace to make it a list.  If @var{expr}'s value is an alist
-(@pxref{Association List Type, Association Lists, , elisp, The Emacs
-Lisp Reference Manual}), this will call @code{assoc} on the result of
-@var{expr}, returning the @code{cdr} of the element of the result
-whose car is equal to @code{"i"}.  Raises an error if the value is not
-a sequence (@pxref{Sequences Arrays Vectors, Sequences, , elisp, The
+index.  The exact behavior depends on the type of @var{expr}'s value:
+
+@table @asis
+
+@item a sequence
+Expands to the element at the (zero-based) index @var{i} of the
+sequence (@pxref{Sequences Arrays Vectors, Sequences, , elisp, The
 Emacs Lisp Reference Manual}).
 
-Multiple sets of indices can also be specified. For example, if
-@var{var} is a list of lists, @samp{$@var{var}[0][0]} is equivalent to
-@samp{(caar @var{var})}.
+@item a string
+Split the string at whitespace, and then expand to the @var{i}th
+element of the resulting sequence.
+
+@item an alist
+If @var{i} is a non-numeric value, expand to the value associated with
+the key @code{"@var{i}"} in the alist.  For example, if @var{var} is
+@samp{(("dog" . "fido") ("cat" . "felix"))}, then
+@samp{$@var{var}[dog]} expands to @code{"fido"}.  Otherwise, this
+behaves as with sequences; e.g., @samp{$@var{var}[0]} expands to
+@code{("dog" . "fido")}.  @xref{Association List Type, Association
+Lists, , elisp, The Emacs Lisp Reference Manual}.
+
+@item anything else
+Signals an error.
+
+@end table
+
+Multiple sets of indices can also be specified.  For example, if
+@var{var} is @samp{((1 2) (3 4))}, then @samp{$@var{var}[0][1]} will
+expand to @code{2}, i.e.@: the second element of the first list member
+(all indices are zero-based).
 
 @item $@var{expr}[@var{regexp} @var{i...}]
 As above (when @var{expr} expands to a string), but use @var{regexp}
@@ -1063,8 +1082,8 @@ Dollars Expansion
 Expands to the length of the result of @var{expr}, an expression in
 one of the above forms.  For example, @samp{$#@var{var}} returns the
 length of the variable @var{var} and @samp{$#@var{var}[0]} returns the
-length of the first element of @var{var}.  Again, raises an error if
-the result of @var{expr} is not a sequence.
+length of the first element of @var{var}.  Again, signals an error if
+the result of @var{expr} is not a string or a sequence.
 
 @end table
 
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index e19481c4ba..ee3f907f85 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -354,8 +354,8 @@ eshell-parse-double-quote
 		  (list 'eshell-escape-arg arg))))
 	  (goto-char (1+ end)))))))
 
-(defun eshell-parse-inner-double-quote (bound)
-  "Parse the inner part of a double quoted string.
+(defun eshell-unescape-inner-double-quote (bound)
+  "Unescape escaped characters inside a double-quoted string.
 The string to parse starts at point and ends at BOUND.
 
 If Eshell is currently parsing a quoted string and there are any
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 04b54d9d79..8be1136e31 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -350,24 +350,27 @@ eshell-complete-lisp-symbols
 
 (defvar eshell--sep-terms)
 
-(defmacro eshell-with-temp-command (command &rest body)
-  "Narrow the buffer to COMMAND and execute the forms in BODY.
-COMMAND can either be a string, or a cons cell demarcating a
-buffer region.  If COMMAND is a string, temporarily insert it
-into the buffer before narrowing.  Point will be set to the
-beginning of the narrowed region.
+(defmacro eshell-with-temp-command (region &rest body)
+  "Narrow the buffer to REGION and execute the forms in BODY.
+
+REGION is a cons cell (START . END) that specifies the region to
+which to narrow the buffer.  REGION can also be a string, in
+which case the macro temporarily inserts it into the buffer at
+point, and narrows the buffer to the inserted string.  Before
+executing BODY, point is set to the beginning of the narrowed
+REGION.
 
 The value returned is the last form in BODY."
   (declare (indent 1))
-  `(let ((cmd ,command))
-     (if (stringp cmd)
+  `(let ((reg ,region))
+     (if (stringp reg)
          ;; Since parsing relies partly on buffer-local state
          ;; (e.g. that of `eshell-parse-argument-hook'), we need to
          ;; perform the parsing in the Eshell buffer.
          (let ((begin (point)) end
 	       (inhibit-point-motion-hooks t))
            (with-silent-modifications
-             (insert cmd)
+             (insert reg)
              (setq end (point))
              (unwind-protect
                  (save-restriction
@@ -376,8 +379,8 @@ eshell-with-temp-command
                    ,@body)
                (delete-region begin end))))
        (save-restriction
-         (narrow-to-region (car cmd) (cdr cmd))
-         (goto-char (car cmd))
+         (narrow-to-region (car reg) (cdr reg))
+         (goto-char (car reg))
          ,@body))))
 
 (defun eshell-parse-command (command &optional args toplevel)
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index af89e35f55..8746f2bb93 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -437,7 +437,7 @@ eshell-parse-variable-ref
             `(eshell-convert
               (eshell-command-to-value
                (eshell-as-subcommand
-                ,(let ((subcmd (or (eshell-parse-inner-double-quote end)
+                ,(let ((subcmd (or (eshell-unescape-inner-double-quote end)
                                    (cons (point) end)))
                        (eshell-current-quoted nil))
                    (eshell-parse-command subcmd)))))
@@ -470,13 +470,15 @@ eshell-parse-variable-ref
     (condition-case nil
         `(eshell-command-to-value
           (eshell-lisp-command
-           ',(read (or (eshell-parse-inner-double-quote (point-max))
+           ',(read (or (eshell-unescape-inner-double-quote (point-max))
                        (current-buffer)))))
       (end-of-file
        (throw 'eshell-incomplete ?\())))
-   ((looking-at (rx (or "'" "\"" "\\\"")))
-    (eshell-with-temp-command (or (eshell-parse-inner-double-quote (point-max))
-                                  (cons (point) (point-max)))
+   ((looking-at (rx-to-string
+                 `(or "'" ,(if eshell-current-quoted "\\\"" "\""))))
+    (eshell-with-temp-command
+        (or (eshell-unescape-inner-double-quote (point-max))
+            (cons (point) (point-max)))
       (let ((name (if (eq (char-after) ?\')
                       (eshell-parse-literal-quote)
                     (eshell-parse-double-quote))))
@@ -506,7 +508,7 @@ eshell-parse-indices
 	(if (not end)
 	    (throw 'eshell-incomplete ?\[)
 	  (forward-char)
-          (eshell-with-temp-command (or (eshell-parse-inner-double-quote end)
+          (eshell-with-temp-command (or (eshell-unescape-inner-double-quote end)
                                         (cons (point) end))
 	    (let (eshell-glob-function (eshell-current-quoted nil))
 	      (setq indices (cons (eshell-parse-arguments
-- 
2.25.1


[-- Attachment #3: 0002-Support-applying-indices-to-more-Eshell-dollar-expan.patch --]
[-- Type: text/plain, Size: 5505 bytes --]

From 1f5704b24a3073f9ac1d273ff7a6b19760071711 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 5 Mar 2022 11:45:49 -0800
Subject: [PATCH 2/2] Support applying indices to more Eshell dollar expansions

For example, '${echo -e "hi\nbye"}[1]' should expand to "bye".

* lisp/eshell/esh-var.el (eshell-parse-variable-ref): Support applying
indices to '${}', '$()', and '$<>' forms.

* lisp/eshell/esh-var-tests.el (esh-var-test/interp-lisp-indices)
(esh-var-test/interp-cmd-indices)
(esh-var-test/interp-cmd-external-indices)
(esh-var-test/quoted-interp-lisp-indices)
(esh-var-test/quoted-interp-cmd-indices): New tests.
---
 lisp/eshell/esh-var.el            | 28 ++++++++++++++++------------
 test/lisp/eshell/esh-var-tests.el | 23 +++++++++++++++++++++++
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 8746f2bb93..ca4cbd744c 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -434,13 +434,15 @@ eshell-parse-variable-ref
           (throw 'eshell-incomplete ?\{)
         (forward-char)
         (prog1
-            `(eshell-convert
-              (eshell-command-to-value
-               (eshell-as-subcommand
-                ,(let ((subcmd (or (eshell-unescape-inner-double-quote end)
-                                   (cons (point) end)))
-                       (eshell-current-quoted nil))
-                   (eshell-parse-command subcmd)))))
+            `(eshell-apply-indices
+              (eshell-convert
+               (eshell-command-to-value
+                (eshell-as-subcommand
+                 ,(let ((subcmd (or (eshell-unescape-inner-double-quote end)
+                                    (cons (point) end)))
+                        (eshell-current-quoted nil))
+                    (eshell-parse-command subcmd)))))
+              indices)
           (goto-char (1+ end))))))
    ((eq (char-after) ?\<)
     (let ((end (eshell-find-delimiter ?\< ?\>)))
@@ -464,14 +466,16 @@ eshell-parse-variable-ref
                            ;; properly.  See bug#54190.
                            (list (function (lambda ()
                                    (delete-file ,temp))))))
-                   (quote ,temp)))
+                   (eshell-apply-indices ,temp indices)))
             (goto-char (1+ end)))))))
    ((eq (char-after) ?\()
     (condition-case nil
-        `(eshell-command-to-value
-          (eshell-lisp-command
-           ',(read (or (eshell-unescape-inner-double-quote (point-max))
-                       (current-buffer)))))
+        `(eshell-apply-indices
+          (eshell-command-to-value
+           (eshell-lisp-command
+            ',(read (or (eshell-unescape-inner-double-quote (point-max))
+                        (current-buffer)))))
+          indices)
       (end-of-file
        (throw 'eshell-incomplete ?\())))
    ((looking-at (rx-to-string
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index d09dd614de..1d051d681a 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -137,10 +137,18 @@ esh-var-test/interp-lisp
   "Interpolate Lisp form evaluation"
   (should (equal (eshell-test-command-result "+ $(+ 1 2) 3") 6)))
 
+(ert-deftest esh-var-test/interp-lisp-indices ()
+  "Interpolate Lisp form evaluation with index"
+  (should (equal (eshell-test-command-result "+ $(list 1 2)[1] 3") 5)))
+
 (ert-deftest esh-var-test/interp-cmd ()
   "Interpolate command result"
   (should (equal (eshell-test-command-result "+ ${+ 1 2} 3") 6)))
 
+(ert-deftest esh-var-test/interp-cmd-indices ()
+  "Interpolate command result with index"
+  (should (equal (eshell-test-command-result "+ ${list 1 2}[1] 3") 5)))
+
 (ert-deftest esh-var-test/interp-cmd-external ()
   "Interpolate command result from external command"
   (skip-unless (executable-find "echo"))
@@ -148,6 +156,13 @@ esh-var-test/interp-cmd-external
    (eshell-command-result-p "echo ${*echo hi}"
                             "hi\n")))
 
+(ert-deftest esh-var-test/interp-cmd-external-indices ()
+  "Interpolate command result from external command with index"
+  (skip-unless (executable-find "echo"))
+  (with-temp-eshell
+   (eshell-command-result-p "echo ${*echo \"hi\nbye\"}[1]"
+                            "bye\n")))
+
 (ert-deftest esh-var-test/interp-temp-cmd ()
   "Interpolate command result redirected to temp file"
   (should (equal (eshell-test-command-result "cat $<echo hi>") "hi")))
@@ -282,12 +297,20 @@ esh-var-test/quoted-interp-lisp
                   "echo \"hi $(concat \\\"the\\\" \\\"re\\\")\"")
                  "hi there")))
 
+(ert-deftest esh-var-test/quoted-interp-lisp-indices ()
+  "Interpolate Lisp form evaluation with index"
+  (should (equal (eshell-test-command-result "+ \"$(list 1 2)[1]\" 3") 5)))
+
 (ert-deftest esh-var-test/quoted-interp-cmd ()
   "Interpolate command result inside double-quotes"
   (should (equal (eshell-test-command-result
                   "echo \"hi ${echo \\\"there\\\"}\"")
                  "hi there")))
 
+(ert-deftest esh-var-test/quoted-interp-cmd-indices ()
+  "Interpolate command result with index inside double-quotes"
+  (should (equal (eshell-test-command-result "+ \"${list 1 2}[1]\" 3") 5)))
+
 (ert-deftest esh-var-test/quoted-interp-temp-cmd ()
   "Interpolate command result redirected to temp file inside double-quotes"
   (should (equal (eshell-test-command-result "cat \"$<echo hi>\"") "hi")))
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation
  2022-03-08  3:38                 ` Jim Porter
@ 2022-03-08 13:56                   ` Eli Zaretskii
  0 siblings, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2022-03-08 13:56 UTC (permalink / raw)
  To: Jim Porter; +Cc: 54227

> Cc: 54227@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> Date: Mon, 7 Mar 2022 19:38:12 -0800
> 
> > Also, it's suppoed to be @var{i}, as in the other alternatives.
> 
> Thanks, fixed.
> 
> >> +@item anything else
> >> +Raises an error.
> > 
> > We say "Signals an error".
> 
> Fixed there, and in the entry below about the '$#foo' syntax.

Thanks, installed on the master branch.





^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-03-08 13:56 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-03  6:35 bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation Jim Porter
2022-03-03 13:58 ` Lars Ingebrigtsen
2022-03-03 16:57 ` Eli Zaretskii
2022-03-03 17:03 ` Eli Zaretskii
2022-03-03 17:56   ` Jim Porter
2022-03-03 18:43     ` Eli Zaretskii
2022-03-03 19:29       ` Jim Porter
2022-03-03 19:50         ` Eli Zaretskii
2022-03-05 20:06           ` Jim Porter
2022-03-05 21:44             ` Jim Porter
2022-03-07 12:52               ` Eli Zaretskii
2022-03-08  3:38                 ` Jim Porter
2022-03-08 13:56                   ` Eli Zaretskii

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).