Subject: Re: bug#55838: 29.0.50; [PATCH] Eshell string-split subscript indexing splits too much

From: Jim Porter <[email-masked]>
Date: Tue, 7 Jun 2022 18:41:30 -0700

On 6/7/2022 6:36 PM, Jim Porter wrote:
>  From "emacs -Q -f eshell":
> 
>    M-: (setq foo "a\nb:c")
> 
>    ~ $ echo $foo
>    a
>    b:c
>    ~ $ echo $foo[: 0]
>    ("a" "b")
> 
> The first command is normal, and just shows that Eshell outputs the 
> string with no manipulation. In the second command, we split the string 
> on ":" and get the 0th element. However, that gets split *again* (on 
> newlines) and returns a list.

Here's a patch for this. It changes the behavior of 
`eshell-apply-indices' to use `eshell-convert-to-number' (when the 
expansion isn't wrapped in double-quotes) instead of the more-aggressive 
`eshell-convert'. I think `eshell-convert-to-number' is the right thing 
here, since Eshell already converts number-like strings to actual 
numbers in most cases.

As a note, if you wanted the old behavior, you could do something like this:

   ~ $ echo $foo[: 0][0 1]
   ("a" "b")

There's also a suggestion in the "Bugs and ideas" section of the Eshell 
manual to add "*" as a subscript to mean "all indices", so you could do 
the above in a more generic fashion like:

   ~ $ echo $foo[: 0][*]
   ;; Doesn't currently work, but it could.
From 21cda1114d6269b47963c8835ee23a5bd31dcb39 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Mon, 6 Jun 2022 19:53:39 -0700
Subject: [PATCH] Don't split Eshell expansions by line when using
 split-subscript operator

* lisp/eshell/esh-var.el (eshell-apply-indices): Use
'eshell-convert-to-number' instead of 'eshell-convert'.

* test/lisp/eshell/esh-var-tests.el
(esh-var-test/interp-convert-var-split-indices): Expand test
(bug#55838).
---
 lisp/eshell/esh-var.el            | 15 ++++++++-------
 test/lisp/eshell/esh-var-tests.el |  9 ++++++++-
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 186f6358bc..27be6e1b1a 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -582,10 +582,11 @@ eshell-apply-indices
 Integers imply a direct index, and names, an associate lookup using
 `assoc'.
 
-If QUOTED is non-nil, this was invoked inside double-quotes.  This
-affects the behavior of splitting strings: without quoting, the
-split values are converted to Lisp forms via `eshell-convert'; with
-quoting, they're left as strings.
+If QUOTED is non-nil, this was invoked inside double-quotes.
+This affects the behavior of splitting strings: without quoting,
+the split values are converted to numbers via
+`eshell-convert-to-number' if possible; with quoting, they're
+left as strings.
 
 For example, to retrieve the second element of a user's record in
 '/etc/passwd', the variable reference would look like:
@@ -599,9 +600,9 @@ eshell-apply-indices
                      (not (get-text-property 0 'number index)))
             (setq separator index
                   refs (cdr refs)))
-	  (setq value
-		(mapcar (lambda (i) (eshell-convert i quoted))
-			(split-string value separator)))))
+	  (setq value (split-string value separator))
+          (unless quoted
+            (setq value (mapcar #'eshell-convert-to-number value)))))
       (cond
        ((< (length refs) 0)
 	(error "Invalid array variable index: %s"
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 4e2a18861e..072cdb9b40 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -357,11 +357,18 @@ esh-var-test/interp-convert-var-number
 
 (ert-deftest esh-var-test/interp-convert-var-split-indices ()
   "Interpolate and convert string variable with indices"
+  ;; Check that numeric forms are converted to numbers.
   (let ((eshell-test-value "000 010 020 030 040"))
     (should (equal (eshell-test-command-result "echo $eshell-test-value[0]")
                    0))
     (should (equal (eshell-test-command-result "echo $eshell-test-value[0 2]")
-                   '(0 20)))))
+                   '(0 20))))
+  ;; Check that multiline forms are preserved as-is.
+  (let ((eshell-test-value "foo\nbar:baz\n"))
+    (should (equal (eshell-test-command-result "echo $eshell-test-value[: 0]")
+                   "foo\nbar"))
+    (should (equal (eshell-test-command-result "echo $eshell-test-value[: 1]")
+                   "baz\n"))))
 
 (ert-deftest esh-var-test/interp-convert-quoted-var-number ()
   "Interpolate numeric quoted numeric variable"
-- 
2.25.1