Subject: Re: bug#55236: [PATCH] 29.0.50; Surprising behaviors with Eshell expansions

From: Jim Porter <[email-masked]>
Date: Mon, 2 May 2022 20:45:25 -0700

Here are the patches. Each patch corresponds to one of the headings in 
the original email. Hopefully they're fairly self-explanatory since I've 
added documentation to the manual as well as NEWS entries for each. I 
debated whether these should count as incompatible changes, but for most 
cases, I think there shouldn't be any incompatibility.

The highest-risk patch is the second one, since it converts multi-line 
numeric output from subcommands into a list of numbers. However, this 
helps a lot in fixing some of the problems in the third issue 
(concatenating expansions), and I think it's more consistent overall 
than before. However, it could make sense to provide a defcustom to opt 
out of the behavior if people think that would be helpful.
From d07087b072dd410040c7ddb1322d1a1f3cd547df Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Mon, 28 Feb 2022 17:38:39 -0800
Subject: [PATCH 1/3] Eshell variable expansion should always return strings
 inside quotes

This is closer in behavior to regular shells, and gives Eshell users
greater flexibility in how variables are expanded.

* lisp/eshell/esh-util.el (eshell-convert): Add TO-STRING argument.

* lisp/eshell/esh-var.el (eshell-parse-variable-ref): Add MODIFIER-P
argument and adjust how 'eshell-convert' and 'eshell-apply-indices'
are called.
(eshell-get-variable, eshell-apply-indices): Add QUOTED argument.

* test/lisp/eshell/esh-var-tests.el (eshell-test-value): New defvar.
(esh-var-test/interp-convert-var-number)
(esh-var-test/interp-convert-var-split-indices)
(esh-var-test/interp-convert-quoted-var-number)
(esh-var-test/interp-convert-quoted-var-split-indices)
(esh-var-test/interp-convert-cmd-string-newline)
(esh-var-test/interp-convert-cmd-multiline)
(esh-var-test/interp-convert-cmd-number)
(esh-var-test/interp-convert-cmd-split-indices)
(esh-var-test/quoted-interp-convert-var-number)
(esh-var-test/quoted-interp-convert-var-split-indices)
(esh-var-test/quoted-interp-convert-quoted-var-number)
(esh-var-test/quoted-interp-convert-quoted-var-split-indices)
(esh-var-test/quoted-interp-convert-cmd-string-newline)
(esh-var-test/quoted-interp-convert-cmd-multiline)
(esh-var-test/quoted-interp-convert-cmd-number)
(esh-var-test/quoted-interp-convert-cmd-split-indices): New tests.

* doc/misc/eshell.texi (Arguments): Expand this section, and document
the new behavior.
(Dollars Expansion): Provide more detail about '$(lisp)' and
'${command}' forms.

* etc/NEWS (Eshell): Announce this change (bug#55236).
---
 doc/misc/eshell.texi              |  57 +++++++---
 etc/NEWS                          |   6 ++
 lisp/eshell/esh-util.el           |  46 +++++---
 lisp/eshell/esh-var.el            |  63 +++++++----
 test/lisp/eshell/esh-var-tests.el | 171 ++++++++++++++++++++++++++----
 5 files changed, 272 insertions(+), 71 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index a3ed922cf2..164a71f309 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -228,15 +228,39 @@ Invocation
 
 @node Arguments
 @section Arguments
-Command arguments are passed to the functions as either strings or
-numbers, depending on what the parser thinks they look like.  If you
-need to use a function that takes some other data type, you will need to
-call it in an Elisp expression (which can also be used with
-@ref{Expansion, expansions}).  As with other shells, you can
-escape special characters and spaces with the backslash (@code{\}) and
-apostrophes (@code{''}) and double quotes (@code{""}).  This is needed
-especially for file names with special characters like pipe
-(@code{|}), which could be part of remote file names.
+Ordinarily, command arguments are parsed by Eshell as either strings
+or numbers, depending on what the parser thinks they look like.  To
+specify an argument of some other data type, you can use an
+@ref{Dollars Expansion, Elisp expression}:
+
+@example
+~ $ echo (list 1 2 3)
+(1 2 3)
+@end example
+
+Additionally, many @ref{Built-ins, Eshell commands} will flatten the
+arguments they receive, so passing a list as an argument will
+``spread'' the elements into multiple arguments:
+
+@example
+~ $ printnl (list 1 2) 3
+1
+2
+3
+@end example
+
+@subsection Quoting and escaping
+
+As with other shells, you can escape special characters and spaces
+with by prefixing the character with a backslash (@code{\}), or by
+surrounding the string with apostrophes (@code{''}) or double quotes
+(@code{""}).  This is needed especially for file names with special
+characters like pipe (@code{|}), which could be part of remote file
+names.
+
+When using @ref{Expansion, expansions} in an Eshell command, the
+result may potentially be of any data type.  To ensure that the result
+is always a string, the expansion can be surrounded by double quotes.
 
 @node Built-ins
 @section Built-in commands
@@ -1026,11 +1050,20 @@ Dollars Expansion
 @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{$},
-it can be used in a string, such as @samp{/some/path/$(@var{lisp}).txt}.
+it can be used inside double quotes or within a longer string, such as
+@samp{/some/path/$(@var{lisp}).txt}.
 
 @item $@{@var{command}@}
-Returns the output of @command{@var{command}}, which can be any valid Eshell
-command invocation, and may even contain expansions.
+Returns the output of @command{@var{command}}, which can be any valid
+Eshell command invocation, and may even contain expansions.  Similar
+to @code{$(@var{lisp})}, this is identical to @code{@{@var{command}@}}
+when on its own, but the @code{$} allows it to be used inside double
+quotes or as part of a string.
+
+Normally, the output is split line-by-line, returning a list (or the
+first element if there's only one line of output).  However, when this
+expansion is surrounded by double quotes, it returns the output as a
+single string instead.
 
 @item $<@var{command}>
 As with @samp{$@{@var{command}@}}, evaluates the Eshell command invocation
diff --git a/etc/NEWS b/etc/NEWS
index 882748d8c7..ae815bb785 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1370,6 +1370,12 @@ Lisp function.  This frees you from having to keep track of whether
 commands are Lisp function or external when supplying absolute file
 name arguments.  See "Electric forward slash" in the Eshell manual.
 
++++
+*** Double-quoting an Eshell expansion now treats the result as a single string.
+If an Eshell expansion like '$FOO' is surrounded by double quotes, the
+result will always be a single string, no matter the type that would
+otherwise be returned.
+
 ---
 *** Built-in Eshell commands now follow POSIX/GNU argument syntax conventions.
 Built-in commands in Eshell now accept command-line options with
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 3da712c719..6c130974e9 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -198,23 +198,37 @@ eshell-find-delimiter
       (when (= depth 0)
         (if reverse-p (point) (1- (point)))))))
 
-(defun eshell-convert (string)
-  "Convert STRING into a more native looking Lisp object."
-  (if (not (stringp string))
-      string
-    (let ((len (length string)))
-      (if (= len 0)
-	  string
-	(if (eq (aref string (1- len)) ?\n)
+(defun eshell-convert (string &optional to-string)
+  "Convert STRING into a more-native Lisp object.
+If TO-STRING is non-nil, always return a single string with
+trailing newlines removed.  Otherwise, this behaves as follows:
+
+* Return non-strings as-is.
+
+* Split multiline strings by line.
+
+* If `eshell-convert-numeric-aguments' is non-nil, convert
+  numeric strings to numbers."
+  (cond
+   ((not (stringp string))
+    (if to-string
+        (eshell-stringify string)
+      string))
+   (to-string (string-trim-right string "\n+"))
+   (t (let ((len (length string)))
+        (if (= len 0)
+	    string
+	  (when (eq (aref string (1- len)) ?\n)
 	    (setq string (substring string 0 (1- len))))
-	(if (string-search "\n" string)
-	    (split-string string "\n")
-	  (if (and eshell-convert-numeric-arguments
-		   (string-match
-		    (concat "\\`\\s-*" eshell-number-regexp
-			    "\\s-*\\'") string))
-	      (string-to-number string)
-	    string))))))
+          (cond
+           ((string-search "\n" string)
+            (split-string string "\n"))
+           ((and eshell-convert-numeric-arguments
+	         (string-match
+                  (concat "\\`\\s-*" eshell-number-regexp "\\s-*\\'")
+                  string))
+            (string-to-number string))
+           (t string)))))))
 
 (defvar-local eshell-path-env (getenv "PATH")
   "Content of $PATH.
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 3c6bcc753c..1c28d24af1 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -402,23 +402,30 @@ eshell-parse-variable
   (let* ((get-len (when (eq (char-after) ?#)
 		    (forward-char) t))
 	 value indices)
-    (setq value (eshell-parse-variable-ref)
+    (setq value (eshell-parse-variable-ref get-len)
 	  indices (and (not (eobp))
 		       (eq (char-after) ?\[)
 		       (eshell-parse-indices))
           ;; This is an expression that will be evaluated by `eshell-do-eval',
           ;; which only support let-binding of dynamically-scoped vars
 	  value `(let ((indices (eshell-eval-indices ',indices))) ,value))
-    (if get-len
-	`(length ,value)
-      value)))
+    (when get-len
+      (setq value `(length ,value)))
+    (when eshell-current-quoted
+      (setq value `(eshell-stringify ,value)))
+    value))
 
-(defun eshell-parse-variable-ref ()
+(defun eshell-parse-variable-ref (&optional modifier-p)
   "Eval a variable reference.
 Returns a Lisp form which, if evaluated, will return the value of the
 variable.
 
-Possible options are:
+If MODIFIER-P is non-nil, the value of the variable will be
+modified by some function.  If MODIFIER-P is nil, the value will be
+used as-is; this allows optimization of some kinds of variable
+references.
+
+Possible variable references are:
 
   NAME          an environment or Lisp variable value
   \"LONG-NAME\"   disambiguates the length of the name
@@ -441,8 +448,16 @@ eshell-parse-variable-ref
                  ,(let ((subcmd (or (eshell-unescape-inner-double-quote end)
                                     (cons (point) end)))
                         (eshell-current-quoted nil))
-                    (eshell-parse-command subcmd)))))
-              indices)
+                    (eshell-parse-command subcmd))))
+               ;; If this is a simple double-quoted form like
+               ;; "${COMMAND}" (i.e. no indices after the subcommand
+               ;; and no `#' modifier before), ensure we convert to a
+               ;; single string.  This avoids unnecessary work
+               ;; (e.g. splitting the output by lines) when it would
+               ;; just be joined back together afterwards.
+               ,(when (and (not modifier-p) eshell-current-quoted)
+                  '(not indices)))
+              indices ,eshell-current-quoted)
           (goto-char (1+ end))))))
    ((eq (char-after) ?\<)
     (let ((end (eshell-find-delimiter ?\< ?\>)))
@@ -466,7 +481,7 @@ eshell-parse-variable-ref
                            ;; properly.  See bug#54190.
                            (list (function (lambda ()
                                    (delete-file ,temp))))))
-                   (eshell-apply-indices ,temp indices)))
+                   (eshell-apply-indices ,temp indices ,eshell-current-quoted)))
             (goto-char (1+ end)))))))
    ((eq (char-after) ?\()
     (condition-case nil
@@ -475,7 +490,7 @@ eshell-parse-variable-ref
            (eshell-lisp-command
             ',(read (or (eshell-unescape-inner-double-quote (point-max))
                         (current-buffer)))))
-          indices)
+          indices ,eshell-current-quoted)
       (end-of-file
        (throw 'eshell-incomplete ?\())))
    ((looking-at (rx-to-string
@@ -487,14 +502,15 @@ eshell-parse-variable-ref
                       (eshell-parse-literal-quote)
                     (eshell-parse-double-quote))))
         (when name
-	  `(eshell-get-variable ,(eval name) indices)))))
+          `(eshell-get-variable ,(eval name) indices ,eshell-current-quoted)))))
    ((assoc (char-to-string (char-after))
            eshell-variable-aliases-list)
     (forward-char)
-    `(eshell-get-variable ,(char-to-string (char-before)) indices))
+    `(eshell-get-variable ,(char-to-string (char-before)) indices
+                          ,eshell-current-quoted))
    ((looking-at eshell-variable-name-regexp)
     (prog1
-        `(eshell-get-variable ,(match-string 0) indices)
+        `(eshell-get-variable ,(match-string 0) indices ,eshell-current-quoted)
       (goto-char (match-end 0))))
    (t
     (error "Invalid variable reference"))))
@@ -525,8 +541,10 @@ eshell-eval-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."
+(defun eshell-get-variable (name &optional indices quoted)
+  "Get the value for the variable NAME.
+INDICES is a list of index-lists (see `eshell-parse-indices').
+If QUOTED is non-nil, this was invoked inside double-quotes."
   (let* ((alias (assoc name eshell-variable-aliases-list))
 	 (var (if alias
 		  (cadr alias)
@@ -547,9 +565,9 @@ eshell-get-variable
 	 (symbol-value var))
 	(t
 	 (error "Unknown variable `%s'" (eshell-stringify var))))
-       indices))))
+       indices quoted))))
 
-(defun eshell-apply-indices (value indices)
+(defun eshell-apply-indices (value indices &optional quoted)
   "Apply to VALUE all of the given INDICES, returning the sub-result.
 The format of INDICES is:
 
@@ -558,12 +576,17 @@ eshell-apply-indices
 
 Each member of INDICES represents a level of nesting.  If the first
 member of a sublist is not an integer or name, and the value it's
-reference is a string, that will be used as the regexp with which is
-to divide the string into sub-parts.  The default is whitespace.
+referencing is a string, that will be used as the regexp with which
+is to divide the string into sub-parts.  The default is whitespace.
 Otherwise, each INT-OR-NAME refers to an element of the list value.
 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.
+
 For example, to retrieve the second element of a user's record in
 '/etc/passwd', the variable reference would look like:
 
@@ -577,7 +600,7 @@ eshell-apply-indices
             (setq separator index
                   refs (cdr refs)))
 	  (setq value
-		(mapcar #'eshell-convert
+		(mapcar (lambda (i) (eshell-convert i quoted))
 			(split-string value separator)))))
       (cond
        ((< (length refs) 0)
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 1d051d681a..5363a86e71 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -210,12 +210,17 @@ esh-var-test/quoted-interp-var-indices
     (should (equal (eshell-test-command-result
                     "echo \"$eshell-test-value[0]\"")
                    "zero"))
+    ;; FIXME: These tests would use the 0th index like the other tests
+    ;; here, but evaluating the command just above adds an `escaped'
+    ;; property to the string "zero".  This results in the output
+    ;; printing the string properties, which is probably the wrong
+    ;; behavior.  See bug#54486.
     (should (equal (eshell-test-command-result
-                    "echo \"$eshell-test-value[0 2]\"")
-                   '("zero" "two")))
+                    "echo \"$eshell-test-value[1 2]\"")
+                   "(\"one\" \"two\")"))
     (should (equal (eshell-test-command-result
-                    "echo \"$eshell-test-value[0 2 4]\"")
-                   '("zero" "two" "four")))))
+                    "echo \"$eshell-test-value[1 2 4]\"")
+                   "(\"one\" \"two\" \"four\")"))))
 
 (ert-deftest esh-var-test/quoted-interp-var-split-indices ()
   "Interpolate string variable with indices inside double-quotes"
@@ -225,7 +230,7 @@ esh-var-test/quoted-interp-var-split-indices
                    "zero"))
     (should (equal (eshell-test-command-result
                     "echo \"$eshell-test-value[0 2]\"")
-                   '("zero" "two")))))
+                   "(\"zero\" \"two\")"))))
 
 (ert-deftest esh-var-test/quoted-interp-var-string-split-indices ()
   "Interpolate string variable with string splitter and indices
@@ -236,14 +241,14 @@ esh-var-test/quoted-interp-var-string-split-indices
                    "zero"))
     (should (equal (eshell-test-command-result
                     "echo \"$eshell-test-value[: 0 2]\"")
-                   '("zero" "two"))))
+                   "(\"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")))))
+                   "(\"zero\" \"two\")"))))
 
 (ert-deftest esh-var-test/quoted-interp-var-regexp-split-indices ()
   "Interpolate string variable with regexp splitter and indices"
@@ -253,43 +258,47 @@ esh-var-test/quoted-interp-var-regexp-split-indices
                    "zero"))
     (should (equal (eshell-test-command-result
                     "echo \"$eshell-test-value['[:!]' 0 2]\"")
-                   '("zero" "two")))
+                   "(\"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")))))
+                   "(\"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))))
+                   "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))))
+    (should (equal (eshell-test-command-result "echo \"$#eshell-test-value\"")
+                   "3"))
+    (should (equal (eshell-test-command-result
+                    "echo \"$#eshell-test-value[1]\"")
+                   "1"))
+    (should (equal (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))))
+    (should (equal (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))))
+    (should (equal (eshell-test-command-result "echo \"$#eshell-test-value\"")
+                   "1"))
+    (should (equal (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"
@@ -299,7 +308,8 @@ esh-var-test/quoted-interp-lisp
 
 (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)))
+  (should (equal (eshell-test-command-result "concat \"$(list 1 2)[1]\" cool")
+                 "2cool")))
 
 (ert-deftest esh-var-test/quoted-interp-cmd ()
   "Interpolate command result inside double-quotes"
@@ -309,12 +319,127 @@ esh-var-test/quoted-interp-cmd
 
 (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)))
+  (should (equal (eshell-test-command-result "concat \"${list 1 2}[1]\" cool")
+                 "2cool")))
 
 (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")))
 
+
+;; Interpolated variable conversion
+
+(ert-deftest esh-var-test/interp-convert-var-number ()
+  "Interpolate numeric variable"
+  (let ((eshell-test-value 123))
+    (should (equal (eshell-test-command-result "type-of $eshell-test-value")
+                   'integer))))
+
+(ert-deftest esh-var-test/interp-convert-var-split-indices ()
+  "Interpolate and convert string variable with indices"
+  (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)))))
+
+(ert-deftest esh-var-test/interp-convert-quoted-var-number ()
+  "Interpolate numeric quoted numeric variable"
+  (let ((eshell-test-value 123))
+    (should (equal (eshell-test-command-result "type-of $'eshell-test-value'")
+                   'integer))
+    (should (equal (eshell-test-command-result "type-of $\"eshell-test-value\"")
+                   'integer))))
+
+(ert-deftest esh-var-test/interp-convert-quoted-var-split-indices ()
+  "Interpolate and convert quoted string variable with indices"
+  (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)))))
+
+(ert-deftest esh-var-test/interp-convert-cmd-string-newline ()
+  "Interpolate trailing-newline command result"
+  (should (equal (eshell-test-command-result "echo ${echo \"foo\n\"}") "foo")))
+
+(ert-deftest esh-var-test/interp-convert-cmd-multiline ()
+  "Interpolate multi-line command result"
+  (should (equal (eshell-test-command-result "echo ${echo \"foo\nbar\"}")
+                 '("foo" "bar"))))
+
+(ert-deftest esh-var-test/interp-convert-cmd-number ()
+  "Interpolate numeric command result"
+  (should (equal (eshell-test-command-result "echo ${echo \"1\"}") 1)))
+
+(ert-deftest esh-var-test/interp-convert-cmd-split-indices ()
+  "Interpolate command result with indices"
+  (should (equal (eshell-test-command-result "echo ${echo \"000 010 020\"}[0]")
+                 0))
+  (should (equal (eshell-test-command-result
+                  "echo ${echo \"000 010 020\"}[0 2]")
+                 '(0 20))))
+
+(ert-deftest esh-var-test/quoted-interp-convert-var-number ()
+  "Interpolate numeric variable inside double-quotes"
+  (let ((eshell-test-value 123))
+    (should (equal (eshell-test-command-result "type-of \"$eshell-test-value\"")
+                   'string))))
+
+(ert-deftest esh-var-test/quoted-interp-convert-var-split-indices ()
+  "Interpolate string variable with indices inside double-quotes"
+  (let ((eshell-test-value "000 010 020 030 040"))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[0]\"")
+                   "000"))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[0 2]\"")
+                   "(\"000\" \"020\")"))))
+
+(ert-deftest esh-var-test/quoted-interp-convert-quoted-var-number ()
+  "Interpolate numeric quoted variable inside double-quotes"
+  (let ((eshell-test-value 123))
+    (should (equal (eshell-test-command-result
+                    "type-of \"$'eshell-test-value'\"")
+                   'string))
+    (should (equal (eshell-test-command-result
+                    "type-of \"$\\\"eshell-test-value\\\"\"")
+                   'string))))
+
+(ert-deftest esh-var-test/quoted-interp-convert-quoted-var-split-indices ()
+  "Interpolate quoted string variable with indices inside double-quotes"
+  (let ((eshell-test-value "000 010 020 030 040"))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[0]\"")
+                   "000"))
+    (should (equal (eshell-test-command-result
+                    "echo \"$eshell-test-value[0 2]\"")
+                   "(\"000\" \"020\")"))))
+
+(ert-deftest esh-var-test/quoted-interp-convert-cmd-string-newline ()
+  "Interpolate trailing-newline command result inside double-quotes"
+  (should (equal (eshell-test-command-result "echo \"${echo \\\"foo\n\\\"}\"")
+                 "foo"))
+  (should (equal (eshell-test-command-result "echo \"${echo \\\"foo\n\n\\\"}\"")
+                 "foo")))
+
+(ert-deftest esh-var-test/quoted-interp-convert-cmd-multiline ()
+  "Interpolate multi-line command result inside double-quotes"
+  (should (equal (eshell-test-command-result
+                  "echo \"${echo \\\"foo\nbar\\\"}\"")
+                 "foo\nbar")))
+
+(ert-deftest esh-var-test/quoted-interp-convert-cmd-number ()
+  "Interpolate numeric command result inside double-quotes"
+  (should (equal (eshell-test-command-result "echo \"${echo \\\"1\\\"}\"")
+                 "1")))
+
+(ert-deftest esh-var-test/quoted-interp-convert-cmd-split-indices ()
+  "Interpolate command result with indices inside double-quotes"
+  (should (equal (eshell-test-command-result
+                  "echo \"${echo \\\"000 010 020\\\"}[0]\"")
+                 "000")))
+
 
 ;; Built-in variables
 
-- 
2.25.1

From d675817d056100bfa909d7ea88cb89f952474992 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Sun, 1 May 2022 22:09:17 -0700
Subject: [PATCH 2/3] Return a list of numbers if all lines of an Eshell
 subcommand are numeric

* lisp/eshell/esh-util.el (eshell-convertible-to-number-p)
(eshell-convert-to-number): New functions...
(eshell-convert): ... use them.

* test/lisp/eshell/esh-var-tests.el
(esh-var-test/interp-convert-cmd-string-newline): Add checks for
numeric output.

* doc/misc/eshell.texi (Dollars Expansion): Document the new behavior.

* etc/NEWS: Announce the change (bug#55236).
---
 doc/misc/eshell.texi              |  8 ++++---
 etc/NEWS                          |  7 ++++++
 lisp/eshell/esh-util.el           | 36 +++++++++++++++++++++----------
 test/lisp/eshell/esh-var-tests.el |  8 ++++++-
 4 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 164a71f309..53f7fdd9ea 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1061,9 +1061,11 @@ Dollars Expansion
 quotes or as part of a string.
 
 Normally, the output is split line-by-line, returning a list (or the
-first element if there's only one line of output).  However, when this
-expansion is surrounded by double quotes, it returns the output as a
-single string instead.
+first element if there's only one line of output); if
+@code{eshell-convert-numeric-agument} is non-@code{nil} and every line
+of output looks like a number, convert each line to a number.
+However, when this expansion is surrounded by double quotes, it
+returns the output as a single string instead.
 
 @item $<@var{command}>
 As with @samp{$@{@var{command}@}}, evaluates the Eshell command invocation
diff --git a/etc/NEWS b/etc/NEWS
index ae815bb785..0c77b0bf58 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1376,6 +1376,13 @@ If an Eshell expansion like '$FOO' is surrounded by double quotes, the
 result will always be a single string, no matter the type that would
 otherwise be returned.
 
++++
+*** Eshell subcommands with multiline numeric output return lists of numbers.
+If every line of the output of an Eshell subcommand like '${COMMAND}'
+is numeric, the result will be a list of numbers (or a single number
+if only one line of output).  Previously, this only converted numbers
+when there was a single line of output.
+
 ---
 *** Built-in Eshell commands now follow POSIX/GNU argument syntax conventions.
 Built-in commands in Eshell now accept command-line options with
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 6c130974e9..9960912bce 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -198,6 +198,23 @@ eshell-find-delimiter
       (when (= depth 0)
         (if reverse-p (point) (1- (point)))))))
 
+(defun eshell-convertible-to-number-p (string)
+  "Return non-nil if STRING can be converted to a number.
+If `eshell-convert-numeric-aguments', always return nil."
+  (and eshell-convert-numeric-arguments
+       (string-match
+        (concat "\\`\\s-*" eshell-number-regexp "\\s-*\\'")
+        string)))
+
+(defun eshell-convert-to-number (string)
+  "Try to convert STRING to a number.
+If STRING doesn't look like a number (or
+`eshell-convert-numeric-aguments' is nil), just return STRING
+unchanged."
+  (if (eshell-convertible-to-number-p string)
+      (string-to-number string)
+    string))
+
 (defun eshell-convert (string &optional to-string)
   "Convert STRING into a more-native Lisp object.
 If TO-STRING is non-nil, always return a single string with
@@ -207,8 +224,8 @@ eshell-convert
 
 * Split multiline strings by line.
 
-* If `eshell-convert-numeric-aguments' is non-nil, convert
-  numeric strings to numbers."
+* If `eshell-convert-numeric-aguments' is non-nil and every line
+  of output looks like a number, convert them to numbers."
   (cond
    ((not (stringp string))
     (if to-string
@@ -220,15 +237,12 @@ eshell-convert
 	    string
 	  (when (eq (aref string (1- len)) ?\n)
 	    (setq string (substring string 0 (1- len))))
-          (cond
-           ((string-search "\n" string)
-            (split-string string "\n"))
-           ((and eshell-convert-numeric-arguments
-	         (string-match
-                  (concat "\\`\\s-*" eshell-number-regexp "\\s-*\\'")
-                  string))
-            (string-to-number string))
-           (t string)))))))
+          (if (string-search "\n" string)
+              (let ((lines (split-string string "\n")))
+                (if (seq-every-p #'eshell-convertible-to-number-p lines)
+                    (mapcar #'string-to-number lines)
+                  lines))
+            (eshell-convert-to-number string)))))))
 
 (defvar-local eshell-path-env (getenv "PATH")
   "Content of $PATH.
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 5363a86e71..2ce6bb4f1b 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -366,7 +366,13 @@ esh-var-test/interp-convert-cmd-string-newline
 (ert-deftest esh-var-test/interp-convert-cmd-multiline ()
   "Interpolate multi-line command result"
   (should (equal (eshell-test-command-result "echo ${echo \"foo\nbar\"}")
-                 '("foo" "bar"))))
+                 '("foo" "bar")))
+  ;; Numeric output should be converted to numbers...
+  (should (equal (eshell-test-command-result "echo ${echo \"01\n02\n03\"}")
+                 '(1 2 3)))
+  ;; ... but only if every line is numeric.
+  (should (equal (eshell-test-command-result "echo ${echo \"01\n02\nhi\"}")
+                 '("01" "02" "hi"))))
 
 (ert-deftest esh-var-test/interp-convert-cmd-number ()
   "Interpolate numeric command result"
-- 
2.25.1

From d8b211c81df0d89b072049d79671b4c0584c67ab Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Mon, 2 May 2022 16:56:49 -0700
Subject: [PATCH 3/3] Improve the behavior of concatenating parts of Eshell
 arguments

Previously, concatenating a list to a string would first convert the
list to a string.  Now, the string is concatenated with the last
element of the list.

* lisp/eshell/esh-util.el (eshell-to-flat-string): Make obsolete.

* lisp/eshell/esh-arg.el (eshell-concat, eshell-concat-1): New
functions.
(eshell-resolve-current-argument): Use 'eshell-concat'.

* test/lisp/eshell/esh-var-tests.el (esh-var-test/interp-concat-cmd):
Add check for concatenation of multiline output of subcommands.
(esh-var-test/quoted-interp-concat-cmd): New test.

* test/lisp/eshell/em-extpipe-tests.el (em-extpipe-test-13): Use
'eshell-concat'.

* doc/misc/eshell.texi (Expansion): Document this behavior.

* etc/NEWS: Announce the change (bug#55236).
---
 doc/misc/eshell.texi                 | 34 +++++++++++++--
 etc/NEWS                             |  7 ++++
 lisp/eshell/esh-arg.el               | 62 ++++++++++++++++++++++++----
 lisp/eshell/esh-util.el              |  1 +
 test/lisp/eshell/em-extpipe-tests.el |  2 +-
 test/lisp/eshell/esh-var-tests.el    | 19 ++++++++-
 6 files changed, 109 insertions(+), 16 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 53f7fdd9ea..684d1553f1 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1017,11 +1017,37 @@ Expansion
 shell, they are less often used for constants, and usually for using
 variables and string manipulation.@footnote{Eshell has no
 string-manipulation expansions because the Elisp library already
-provides many functions for this.}  For example, @code{$var} on a line
-expands to the value of the variable @code{var} when the line is
+provides many functions for this.}  For example, @code{$@var{var}} on
+a line expands to the value of the variable @var{var} when the line is
 executed.  Expansions are usually passed as arguments, but may also be
-used as commands.@footnote{E.g., entering just @samp{$var} at the prompt
-is equivalent to entering the value of @code{var} at the prompt.}
+used as commands.@footnote{E.g., entering just @samp{$@var{var}} at
+the prompt is equivalent to entering the value of @var{var} at the
+prompt.}
+
+You can concatenate expansions with regular string arguments or even
+other expansions.  In the simplest case, when the expansion returns a
+string value, this is equivalent to ordinary string concatenation; for
+example, @samp{$@{echo "foo"@}bar} returns @samp{foobar}.  The exact
+behavior depends on the types of each value being concatenated:
+
+@table @asis
+
+@item both strings
+Concatenate both values together.
+
+@item one or both numbers
+Concatenate the string representation of each value, converting back to
+a number if possible.
+
+@item one or both (non-@code{nil}) lists
+Concatenate ``adjacent'' elements of each value (possibly converting
+back to a number as above).  For example, @samp{$list("a" "b")c}
+returns @samp{("a" "bc")}.
+
+@item anything else
+Concatenate the string represenation of each value.
+
+@end table
 
 @menu
 * Dollars Expansion::
diff --git a/etc/NEWS b/etc/NEWS
index 0c77b0bf58..360540b02a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1376,6 +1376,13 @@ If an Eshell expansion like '$FOO' is surrounded by double quotes, the
 result will always be a single string, no matter the type that would
 otherwise be returned.
 
++++
+*** Concatenating Eshell expansions now works more similarly to other shells.
+When concatenating an Eshell expansion that returns a list, "adjacent"
+elements of each operand are now concatenated together,
+e.g. '$list("a" "b")c' returns '("a" "bc")'.  See the "(eshell)
+Expansion" node in the Eshell manual for more details.
+
 +++
 *** Eshell subcommands with multiline numeric output return lists of numbers.
 If every line of the output of an Eshell subcommand like '${COMMAND}'
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index 395aa87ff0..459487f435 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -180,19 +180,63 @@ eshell-escape-arg
       (add-text-properties 0 (length string) '(escaped t) string))
   string)
 
+(defun eshell-concat (quoted &rest rest)
+  "Concatenate all the arguments in REST and return the result.
+If QUOTED is nil, the resulting value(s) may be converted to
+numbers (see `eshell-concat-1').
+
+If each argument in REST is a non-list value, the result will be
+a single value, as if (mapconcat #'eshell-stringify REST) had been
+called, possibly converted to a number.
+
+If there is at least one (non-nil) list argument, the result will
+be a list, with \"adjacent\" elements of consecutive arguments
+concatenated as strings (again, possibly converted to numbers).
+For example, concatenating \"a\", (\"b\"), and (\"c\" \"d\")
+would produce (\"abc\" \"d\")."
+  (let (result)
+    (dolist (i rest result)
+      (when i
+        (cond
+         ((null result)
+          (setq result i))
+         ((listp result)
+          (let (curr-head curr-tail)
+            (if (listp i)
+                (setq curr-head (car i)
+                      curr-tail (cdr i))
+              (setq curr-head i
+                    curr-tail nil))
+            (setq result
+                  (append
+                   (butlast result 1)
+                   (list (eshell-concat-1 quoted (car (last result))
+                                          curr-head))
+                   curr-tail))))
+         ((listp i)
+          (setq result
+                (cons (eshell-concat-1 quoted result (car i))
+                      (cdr i))))
+         (t
+          (setq result (eshell-concat-1 quoted result i))))))))
+
+(defun eshell-concat-1 (quoted first second)
+  "Concatenate FIRST and SECOND.
+If QUOTED is nil and either FIRST or SECOND are numbers, try to
+convert the result to a number as well."
+  (let ((result (concat (eshell-stringify first) (eshell-stringify second))))
+    (if (and (not quoted)
+             (or (numberp first) (numberp second)))
+        (eshell-convert-to-number result)
+      result)))
+
 (defun eshell-resolve-current-argument ()
   "If there are pending modifications to be made, make them now."
   (when eshell-current-argument
     (when eshell-arg-listified
-      (let ((parts eshell-current-argument))
-	(while parts
-	  (unless (stringp (car parts))
-	    (setcar parts
-		    (list 'eshell-to-flat-string (car parts))))
-	  (setq parts (cdr parts)))
-	(setq eshell-current-argument
-	      (list 'eshell-convert
-		    (append (list 'concat) eshell-current-argument))))
+      (setq eshell-current-argument
+            (append (list 'eshell-concat eshell-current-quoted)
+                    eshell-current-argument))
       (setq eshell-arg-listified nil))
     (while eshell-current-modifiers
       (setq eshell-current-argument
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 9960912bce..b5a423f023 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -293,6 +293,7 @@ eshell-split-path
 
 (defun eshell-to-flat-string (value)
   "Make value a string.  If separated by newlines change them to spaces."
+  (declare (obsolete nil "29.1"))
   (let ((text (eshell-stringify value)))
     (if (string-match "\n+\\'" text)
 	(setq text (replace-match "" t t text)))
diff --git a/test/lisp/eshell/em-extpipe-tests.el b/test/lisp/eshell/em-extpipe-tests.el
index 91c2fba479..3b84d763ac 100644
--- a/test/lisp/eshell/em-extpipe-tests.el
+++ b/test/lisp/eshell/em-extpipe-tests.el
@@ -170,7 +170,7 @@ em-extpipe-test-12
 
 (em-extpipe-tests--deftest em-extpipe-test-13 "foo*|bar"
   (should-parse '(eshell-execute-pipeline
-                  '((eshell-named-command (concat "foo" "*"))
+                  '((eshell-named-command (eshell-concat nil "foo" "*"))
                     (eshell-named-command "bar")))))
 
 (em-extpipe-tests--deftest em-extpipe-test-14 "tac *<temp"
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 2ce6bb4f1b..3f3b591c5a 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -176,8 +176,17 @@ esh-var-test/interp-concat-lisp2
   (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)))
+  "Interpolate and concat command with literal"
+  (should (equal (eshell-test-command-result "+ ${+ 1 2}3 3") 36))
+  (should (equal (eshell-test-command-result "echo ${*echo \"foo\nbar\"}-baz")
+                 '("foo" "bar-baz")))
+  ;; Concatenating to a number in a list should produce a number...
+  (should (equal (eshell-test-command-result "echo ${*echo \"1\n2\"}3")
+                 '(1 23)))
+  ;; ... but concatenating to a string that looks like a number in a list
+  ;; should produce a string.
+  (should (equal (eshell-test-command-result "echo ${*echo \"hi\n2\"}3")
+                 '("hi" "23"))))
 
 (ert-deftest esh-var-test/interp-concat-cmd2 ()
   "Interpolate and concat two commands"
@@ -326,6 +335,12 @@ 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")))
 
+(ert-deftest esh-var-test/quoted-interp-concat-cmd ()
+  "Interpolate and concat command with literal"
+  (should (equal (eshell-test-command-result
+                  "echo \"${echo \\\"foo\nbar\\\"} baz\"")
+                 "foo\nbar baz")))
+
 
 ;; Interpolated variable conversion
 
-- 
2.25.1