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

2 files changed, 73 insertions, 36 deletions

diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 3da712c719a..6c130974e95 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -198,23 +198,37 @@ doubling it up."
       (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 3c6bcc753c2..1c28d24af12 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -402,23 +402,30 @@ process any indices that come after the variable reference."
   (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 @@ Possible options are:
                  ,(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 @@ Possible options are:
                            ;; 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 @@ Possible options are:
            (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 @@ Possible options are:
                       (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 @@ For example, \"[0 1][2]\" becomes:
   "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 @@ For example, \"[0 1][2]\" becomes:
 	 (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 @@ The format of INDICES is:
 
 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 @@ For example, to retrieve the second element of a user's record in
             (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)