GNU bug report logs - #54190
29.0.50; [PATCH] Incorrect/missing documentation for some Eshell "$" syntaxes

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 54190 in the body.
You can then email your comments to 54190 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.50; [PATCH] Incorrect/missing documentation for some Eshell "$"
 syntaxes
Date: Sun, 27 Feb 2022 13:34:26 -0800

[Message part 1 (text/plain, inline)]

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.

[0001-Improve-correct-documentation-about-Eshell-variable-.patch (text/plain, attachment)]

Message #8 received at 54190 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: 54190 <at> debbugs.gnu.org
Subject: Re: bug#54190: 29.0.50; [PATCH] Incorrect/missing documentation for
 some Eshell "$" syntaxes
Date: Sun, 27 Feb 2022 22:11:01 -0800

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

Message #11 received at 54190 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: 54190 <at> debbugs.gnu.org
Subject: Re: bug#54190: 29.0.50; [PATCH] Incorrect/missing documentation for
 some Eshell "$" syntaxes
Date: Mon, 28 Feb 2022 00:16:21 -0800

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.

Message #14 received at 54190 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 54190 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#54190: 29.0.50; [PATCH] Incorrect/missing documentation for
 some Eshell "$" syntaxes
Date: Mon, 28 Feb 2022 10:38:31 +0100

Jim Porter <jporterbugs <at> 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

Message #17 received at 54190 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Jim Porter <jporterbugs <at> gmail.com>, 54190 <at> debbugs.gnu.org
Subject: Re: bug#54190: 29.0.50; [PATCH] Incorrect/missing documentation for
 some Eshell "$" syntaxes
Date: Mon, 28 Feb 2022 08:18:26 -0500

Lars Ingebrigtsen [2022-02-28 10:38:31] wrote:
> Jim Porter <jporterbugs <at> 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

Message #20 received at 54190 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: jporterbugs <at> gmail.com, larsi <at> gnus.org, 54190 <at> debbugs.gnu.org
Subject: Re: bug#54190: 29.0.50;
 [PATCH] Incorrect/missing documentation for some Eshell "$" syntaxes
Date: Mon, 28 Feb 2022 15:28:03 +0200

> Cc: Jim Porter <jporterbugs <at> gmail.com>, 54190 <at> 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 <at> 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.

Message #23 received at 54190 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 54190 <at> debbugs.gnu.org
Subject: Re: bug#54190: 29.0.50; [PATCH] Incorrect/missing documentation for
 some Eshell "$" syntaxes
Date: Mon, 28 Feb 2022 09:47:54 -0800

[Message part 1 (text/plain, inline)]

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

[0001-Improve-correct-documentation-about-Eshell-variable-.patch (text/plain, attachment)]

[0002-Partially-revert-b03f74e0f2a578b1580e8b1c368665850ee.patch (text/plain, attachment)]

Message #26 received at 54190 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: larsi <at> gnus.org, 54190 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#54190: 29.0.50;
 [PATCH] Incorrect/missing documentation for some Eshell "$" syntaxes
Date: Tue, 01 Mar 2022 15:10:51 +0200

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

Message #29 received at 54190 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: larsi <at> gnus.org, 54190 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#54190: 29.0.50; [PATCH] Incorrect/missing documentation for
 some Eshell "$" syntaxes
Date: Thu, 3 Mar 2022 21:55:41 -0800

[Message part 1 (text/plain, inline)]

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.

[0001-Fix-Eshell-s-command-forms-in-a-better-way.patch (text/plain, attachment)]

Message #32 received at 54190 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: larsi <at> gnus.org, 54190 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#54190: 29.0.50; [PATCH] Incorrect/missing documentation for
 some Eshell "$" syntaxes
Date: Fri, 04 Mar 2022 09:16:44 +0200

> Cc: larsi <at> gnus.org, 54190 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
> From: Jim Porter <jporterbugs <at> 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.

Message #35 received at 54190 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 54190 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#54190: 29.0.50; [PATCH] Incorrect/missing documentation for
 some Eshell "$" syntaxes
Date: Fri, 04 Mar 2022 08:35:50 -0500

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

Message #38 received at 54190 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 54190 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#54190: 29.0.50; [PATCH] Incorrect/missing documentation for
 some Eshell "$" syntaxes
Date: Fri, 4 Mar 2022 10:04:22 -0800

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.

Message #41 received at 54190 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 54190 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#54190: 29.0.50; [PATCH] Incorrect/missing documentation for
 some Eshell "$" syntaxes
Date: Tue, 06 Sep 2022 13:28:28 +0200

Jim Porter <jporterbugs <at> 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.)

Message #46 received at 54190-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 54190-done <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#54190: 29.0.50; [PATCH] Incorrect/missing documentation for
 some Eshell "$" syntaxes
Date: Tue, 6 Sep 2022 20:50:14 -0700

On 9/6/2022 4:28 AM, Lars Ingebrigtsen wrote:
> Jim Porter <jporterbugs <at> 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.

Message #49 received at 54190 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 54190 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#54190: 29.0.50; [PATCH] Incorrect/missing documentation for
 some Eshell "$" syntaxes
Date: Tue, 6 Sep 2022 20:59:51 -0700

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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.