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

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

[Jim Porter]

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

In the documentation for lisp/eshell/esh-var.el (and 
`eshell-parse-variable-ref' in that file), it says that "$<FOO>" is a 
way of accessing the value of the variable FOO to disambiguate the 
length of the variable name, sort of like "${FOO}" in ordinary shells. 
However, that's not actually true. The correct syntax for that is:

   $"FOO"
   ;; or...
   $'FOO'

In fact, what "$<FOO>" does is to run FOO as a subcommand, writing its 
stdout to a temp file, and returning that file's name. This is (very!) 
subtly implied in the Eshell manual in the "Bugs and ideas" section, 
where it says:

   `grep python $<rpm -qa>' doesn't work, but using `*grep' does

   This happens because the `grep' Lisp function returns immediately,
   and then the asynchronous `grep' process expects to examine the
   temporary file, which has since been deleted.

Attached is a patch which updates the documentation to correctly 
describe the current behavior. Note: since this is just a documentation 
change, it might be worth pushing to the 28 branch.

[-- Attachment #2: 0001-Improve-correct-documentation-about-Eshell-variable-.patch --]
[-- Type: text/plain, Size: 3050 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] 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

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

[Jim Porter]

On 2/27/2022 1:34 PM, Jim Porter wrote:
> In fact, what "$<FOO>" does is to run FOO as a subcommand, writing its 
> stdout to a temp file, and returning that file's name.
Hmm, that's unfortunate. It seems that the "$<FOO>" syntax in Eshell 
broke sometime between 27.2 and 28. I'm bisecting to figure out where 
that happened, but figured I'd mention it now so there's no confusion if 
someone tries it out now on 28/29. Fix forthcoming (hopefully).

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

[Jim Porter]

On 2/27/2022 10:11 PM, Jim Porter wrote:
> Hmm, that's unfortunate. It seems that the "$<FOO>" syntax in Eshell 
> broke sometime between 27.2 and 28. I'm bisecting to figure out where 
> that happened, but figured I'd mention it now so there's no confusion if 
> someone tries it out now on 28/29. Fix forthcoming (hopefully).

Ok, the breaking commit is b03f74e0f2a578b1580e8b1c368665850ee7f808 
("Don't quote lambdas in several places"). Reverting the change in that 
commit in lisp/eshell/esh-var.el fixes things, although I'm not sure why 
yet. I believe that code gets evaluated by `eshell-do-eval', which 
evaluates things in a very particular way in order to support deferring 
evaluation at various points. Once I'm sure I understand why this is 
breaking, I'll post a patch to fix the bustage.

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

[Lars Ingebrigtsen]

Jim Porter <jporterbugs@gmail.com> writes:

> Ok, the breaking commit is b03f74e0f2a578b1580e8b1c368665850ee7f808
> ("Don't quote lambdas in several places"). Reverting the change in
> that commit in lisp/eshell/esh-var.el fixes things, although I'm not
> sure why yet. I believe that code gets evaluated by `eshell-do-eval',
> which evaluates things in a very particular way in order to support
> deferring evaluation at various points. Once I'm sure I understand why
> this is breaking, I'll post a patch to fix the bustage.

It's this bit?  Hm...

diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 96838d4132..7388279f15 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -463,8 +463,8 @@ eshell-parse-variable-ref
                    (eshell-as-subcommand ,(eshell-parse-command cmd))
                    (ignore
                     (nconc eshell-this-command-hook
-                           (list (function (lambda ()
-                                              (delete-file ,temp))))))
+                           (list (lambda ()
+                                   (delete-file ,temp)))))
                    (quote ,temp)))
             (goto-char (1+ end)))))))
    ((eq (char-after) ?\()


Trying to follow the logic of how this is eventually evaluated isn't,
er, obvious, but I'm not sure how that change could break anything,
either.  Perhaps Stefan has a comment; added to the CCs.

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

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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"). Reverting the change in
>> that commit in lisp/eshell/esh-var.el fixes things, although I'm not
>> sure why yet. I believe that code gets evaluated by `eshell-do-eval',
>> which evaluates things in a very particular way in order to support
>> deferring evaluation at various points. Once I'm sure I understand why
>> this is breaking, I'll post a patch to fix the bustage.
>
> It's this bit?  Hm...
>
> diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
> index 96838d4132..7388279f15 100644
> --- a/lisp/eshell/esh-var.el
> +++ b/lisp/eshell/esh-var.el
> @@ -463,8 +463,8 @@ eshell-parse-variable-ref
>                     (eshell-as-subcommand ,(eshell-parse-command cmd))
>                     (ignore
>                      (nconc eshell-this-command-hook
> -                           (list (function (lambda ()
> -                                              (delete-file ,temp))))))
> +                           (list (lambda ()
> +                                   (delete-file ,temp)))))
>                     (quote ,temp)))
>              (goto-char (1+ end)))))))
>     ((eq (char-after) ?\()
>
>
> Trying to follow the logic of how this is eventually evaluated isn't,
> er, obvious, but I'm not sure how that change could break anything,
> either.  Perhaps Stefan has a comment; added to the CCs.

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.


        Stefan

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

[Eli Zaretskii]

> Cc: Jim Porter <jporterbugs@gmail.com>, 54190@debbugs.gnu.org
> Date: Mon, 28 Feb 2022 08:18:26 -0500
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> > diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
> > index 96838d4132..7388279f15 100644
> > --- a/lisp/eshell/esh-var.el
> > +++ b/lisp/eshell/esh-var.el
> > @@ -463,8 +463,8 @@ eshell-parse-variable-ref
> >                     (eshell-as-subcommand ,(eshell-parse-command cmd))
> >                     (ignore
> >                      (nconc eshell-this-command-hook
> > -                           (list (function (lambda ()
> > -                                              (delete-file ,temp))))))
> > +                           (list (lambda ()
> > +                                   (delete-file ,temp)))))
> >                     (quote ,temp)))
> >              (goto-char (1+ end)))))))
> >     ((eq (char-after) ?\()
> >
> >
> > Trying to follow the logic of how this is eventually evaluated isn't,
> > er, obvious, but I'm not sure how that change could break anything,
> > either.  Perhaps Stefan has a comment; added to the CCs.
> 
> 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.

I agree.  We should revert this on the release branch, since this is a
regression wrt Emacs 27.

Thanks.

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

[Jim Porter]

[-- 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

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

[Eli Zaretskii]

> From: Jim Porter <jporterbugs@gmail.com>
> Date: Mon, 28 Feb 2022 09:47:54 -0800
> Cc: 54190@debbugs.gnu.org
> 
> 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.

Thanks, installed on the emacs-28 branch.  (But the markup in the
manual part was wrong; see my followup changes.)

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

Fine with me.  It's up to you whether to close this bug or leave it
open until a better solution is found for master.

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

[Jim Porter]

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

On 3/1/2022 5:10 AM, Eli Zaretskii wrote:
> Thanks, installed on the emacs-28 branch.  (But the markup in the
> manual part was wrong; see my followup changes.)

Thanks for fixing that up.

> Fine with me.  It's up to you whether to close this bug or leave it
> open until a better solution is found for master.

After a bit of looking at the `eshell-do-eval' implementation, I 
realized that there's a better solution that's very simple. See the 
attached patch. It's simple enough that it might be fine for Emacs 28, 
but the existing change on the 28 branch is probably a bit safer, since 
that change was just a partial revert. I'm not aware of anything that 
could be broken by this new patch, but you never know...

This doesn't do anything more elaborate like using generator.el's 
machinery here; while (I think) that would be nice to have eventually, 
we can address that in a separate bug, since it'd be a pretty big 
change. We could probably close this bug after this new patch merges.

[-- Attachment #2: 0001-Fix-Eshell-s-command-forms-in-a-better-way.patch --]
[-- Type: text/plain, Size: 1856 bytes --]

From 48387e827cd8d550e9627df7256fd3f1a9705a26 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 3 Mar 2022 21:46:11 -0800
Subject: [PATCH] Fix Eshell's '$<command>' forms in a better way

* lisp/eshell/esh-cmd.el (eshell-do-eval): Handle 'lambda' forms.
* lisp/eshell/esh-var.el (eshell-parse-variable-ref): Remove
workaround from commit 9e257ae (bug#54190).
---
 lisp/eshell/esh-cmd.el | 2 +-
 lisp/eshell/esh-var.el | 8 ++------
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 04b54d9d79..484888ec47 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -1077,7 +1077,7 @@ eshell-do-eval
   (cond
    ((not (listp form))
     (list 'quote (eval form)))
-   ((memq (car form) '(quote function))
+   ((memq (car form) '(quote function lambda))
     form)
    (t
     ;; skip past the call to `eshell-do-eval'
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index af89e35f55..092f9f87a4 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -458,12 +458,8 @@ eshell-parse-variable-ref
                        (eshell-parse-command cmd)))
                    (ignore
                     (nconc eshell-this-command-hook
-                           ;; 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))))))
+                           (list (lambda ()
+                                   (delete-file ,temp)))))
                    (quote ,temp)))
             (goto-char (1+ end)))))))
    ((eq (char-after) ?\()
-- 
2.25.1

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

[Eli Zaretskii]

> Cc: larsi@gnus.org, 54190@debbugs.gnu.org, monnier@iro.umontreal.ca
> From: Jim Porter <jporterbugs@gmail.com>
> Date: Thu, 3 Mar 2022 21:55:41 -0800
> 
> After a bit of looking at the `eshell-do-eval' implementation, I 
> realized that there's a better solution that's very simple. See the 
> attached patch. It's simple enough that it might be fine for Emacs 28, 
> but the existing change on the 28 branch is probably a bit safer, since 
> that change was just a partial revert. I'm not aware of anything that 
> could be broken by this new patch, but you never know...

Thanks, I think this should be installed on master.

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> @@ -1077,7 +1077,7 @@ eshell-do-eval
>    (cond
>     ((not (listp form))
>      (list 'quote (eval form)))
> -   ((memq (car form) '(quote function))
> +   ((memq (car form) '(quote function lambda))
>      form)
>     (t
>      ;; skip past the call to `eshell-do-eval'

This looks like a workaround rather than a fix.
eshell-do-eval supposedly handles code that uses macros.  `lambda` is
just one such macro (and a very simple one at that), so if the `lambda`
macro is not properly handled, then most likely there are other macros
which are similarly mishandled.


        Stefan

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

[Jim Porter]

On 3/4/2022 5:35 AM, Stefan Monnier via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
> This looks like a workaround rather than a fix.
> eshell-do-eval supposedly handles code that uses macros.  `lambda` is
> just one such macro (and a very simple one at that), so if the `lambda`
> macro is not properly handled, then most likely there are other macros
> which are similarly mishandled.

Agreed. This is just intended as a slightly cleaner workaround than the 
previous one, but it's not a true fix. I think a real fix would be 
significantly more complex (e.g. using generator.el), and that might be 
better to track in a separate bug.

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

[Lars Ingebrigtsen]

Jim Porter <jporterbugs@gmail.com> writes:

> This doesn't do anything more elaborate like using generator.el's
> machinery here; while (I think) that would be nice to have eventually,
> we can address that in a separate bug, since it'd be a pretty big
> change. We could probably close this bug after this new patch merges.

This was several months ago, but it doesn't look like this was ever
merged.  If you still think this is the right solution, please go ahead
and merge.  (The patch no longer merges cleanly.)

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

[Jim Porter]

On 9/6/2022 4:28 AM, Lars Ingebrigtsen wrote:
> Jim Porter <jporterbugs@gmail.com> writes:
> 
>> This doesn't do anything more elaborate like using generator.el's
>> machinery here; while (I think) that would be nice to have eventually,
>> we can address that in a separate bug, since it'd be a pretty big
>> change. We could probably close this bug after this new patch merges.
> 
> This was several months ago, but it doesn't look like this was ever
> merged.  If you still think this is the right solution, please go ahead
> and merge.  (The patch no longer merges cleanly.)

This got merged into Emacs 28 as 
2c3d1b6bf41509bf0ba8995925fec9f20d8ed89d, but I think it was left open 
since there was some discussion about rewriting 'eshell-do-eval' on top 
of generator.el. Let's close this, and I'll file a separate followup for 
the generator.el bit.

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

[Jim Porter]

On 9/6/2022 8:50 PM, Jim Porter wrote:
> Let's close this, and I'll file a separate followup for the
> generator.el bit.
Filed bug#57635 for the generator.el followup.