bug#54190: 29.0.50; [PATCH] Incorrect/missing documentation for some Eshell "$" syntaxes

[Jim Porter <jporterbugs@gmail.com>]

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

On 2/28/2022 5:18 AM, Stefan Monnier via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
> Lars Ingebrigtsen [2022-02-28 10:38:31] wrote:
>> Jim Porter <jporterbugs@gmail.com> writes:
>>> Ok, the breaking commit is b03f74e0f2a578b1580e8b1c368665850ee7f808
>>> ("Don't quote lambdas in several places").
[snip]> Looks like a bug somewhere in the `eshell-do-eval` machinery, yes.
> Until we find the problem (or rewrite `eshell-do-eval` on top of the CPS
> converter of `generator.el`), I think reverting this change (and adding
> a comment pointing to this bug) sounds like a great plan.

Here's a patch to fix this, with a test so it doesn't regress again. I 
also re-attached the doc fix for simplicity. Both should apply to the 28 
branch with no issues.

I'll try to figure out why `eshell-do-eval' expects quoted lambdas, but 
unless that's a really trivial problem, the method here is probably 
safer for the 28 branch. Besides, I'm now 4 or 5 steps removed from the 
patch I was *trying* to work on, so a quick fix here will help me get 
back on track.

Longer term, I agree that rewriting `eshell-do-eval' in terms of 
generator.el would be good. I think there are some bugs with how local 
variable state is managed by `eshell-do-eval', but I haven't had a 
chance to investigate it in any detail yet.

[-- Attachment #2: 0001-Improve-correct-documentation-about-Eshell-variable-.patch --]
[-- Type: text/plain, Size: 3055 bytes --]

From ec52b44f694f2515bb3673f998e6ea80d2d31e08 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 27 Feb 2022 13:20:51 -0800
Subject: [PATCH 1/2] Improve/correct documentation about Eshell variable
 expansion

* lisp/eshell/esh-var.el: Correct documentation comment.
(eshell-parse-variable-ref): Correct docstring.

* doc/misc/eshell.texi (Dollars Expansion): Add documentation for
$"var"/$'var' and $<command> syntaxes.
---
 doc/misc/eshell.texi   | 11 +++++++++++
 lisp/eshell/esh-var.el | 15 ++++++++++++---
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 261e88d00c..88a7d7b130 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1016,6 +1016,12 @@ Dollars Expansion
 Expands to the value bound to @code{var}.  This is the main way to use
 variables in command invocations.
 
+@item $"var"
+@item $'var'
+Expands to the value bound to @code{var}.  This is useful to
+disambiguate the variable name when concatenating it with another
+value, such as @samp{$"var"-suffix}.
+
 @item $#var
 Expands to the length of the value bound to @code{var}.  Raises an error
 if the value is not a sequence
@@ -1030,6 +1036,11 @@ Dollars Expansion
 Returns the output of @command{command}, which can be any valid Eshell
 command invocation, and may even contain expansions.
 
+@item $<command>
+As with @samp{$@{command@}}, evaluates the Eshell command invocation
+@command{command}, but writes the output to a temporary file and
+returns the file name.
+
 @item $var[i]
 Expands to the @code{i}th element of the value bound to @code{var}.  If
 the value is a string, it will be split at whitespace to make it a list.
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 5c8dacd980..cfefe15c9c 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -34,7 +34,8 @@
 ;;
 ;; "-" is a valid part of a variable name.
 ;;
-;;   $<MYVAR>-TOO
+;;   $\"MYVAR\"-TOO
+;;   $'MYVAR'-TOO
 ;;
 ;; Only "MYVAR" is part of the variable name in this case.
 ;;
@@ -55,6 +56,11 @@
 ;; Returns the value of an eshell subcommand.  See the note above
 ;; regarding Lisp evaluations.
 ;;
+;;   $<command>
+;;
+;; Evaluates an eshell subcommand, redirecting the output to a
+;; temporary file, and returning the file name.
+;;
 ;;   $ANYVAR[10]
 ;;
 ;; Return the 10th element of ANYVAR.  If ANYVAR's value is a string,
@@ -423,9 +429,12 @@ eshell-parse-variable-ref
 Possible options are:
 
   NAME          an environment or Lisp variable value
-  <LONG-NAME>   disambiguates the length of the name
+  \"LONG-NAME\"   disambiguates the length of the name
+  'LONG-NAME'   as above
   {COMMAND}     result of command is variable's value
-  (LISP-FORM)   result of Lisp form is variable's value"
+  (LISP-FORM)   result of Lisp form is variable's value
+  <COMMAND>     write the output of command to a temporary file;
+                result is the file name"
   (cond
    ((eq (char-after) ?{)
     (let ((end (eshell-find-delimiter ?\{ ?\})))
-- 
2.25.1


[-- Attachment #3: 0002-Partially-revert-b03f74e0f2a578b1580e8b1c368665850ee.patch --]
[-- Type: text/plain, Size: 2253 bytes --]

From 6d84d62ceb771b5132d051e34f458f7110a83808 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 28 Feb 2022 09:31:22 -0800
Subject: [PATCH 2/2] Partially revert b03f74e0f2a578b1580e8b1c368665850ee7f808

That commit regressed '$<command>' forms in Eshell, due to a
limitation/bug in how 'eshell-do-eval' works.

* lisp/eshell/esh-var.el (eshell-parse-variable-ref): Quote a lambda.

* test/lisp/eshell/eshell-tests.el (eshell-test/interp-temp-cmd):
New test.
---
 lisp/eshell/esh-var.el           | 8 ++++++--
 test/lisp/eshell/eshell-tests.el | 4 ++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index cfefe15c9c..ee3ffbc647 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -466,8 +466,12 @@ eshell-parse-variable-ref
                    (eshell-as-subcommand ,(eshell-parse-command cmd))
                    (ignore
                     (nconc eshell-this-command-hook
-                           (list (lambda ()
-                                   (delete-file ,temp)))))
+                           ;; Quote this lambda; it will be evaluated
+                           ;; by `eshell-do-eval', which requires very
+                           ;; particular forms in order to work
+                           ;; properly.  See bug#54190.
+                           (list (function (lambda ()
+                                   (delete-file ,temp))))))
                    (quote ,temp)))
             (goto-char (1+ end)))))))
    ((eq (char-after) ?\()
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index d6ee1bdb17..eff4edd62c 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -93,6 +93,10 @@ 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)))
-- 
2.25.1