bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation

[Jim Porter]

On 3/3/2022 9:03 AM, Eli Zaretskii wrote:
> > From: Jim Porter <jporterbugs@gmail.com>
> > Date: Wed, 2 Mar 2022 22:35:22 -0800
> >
> > +(defmacro eshell-with-temp-command (command &rest body)
> > +  "Narrow the buffer to COMMAND and execute the forms in BODY.
>
> What does it mean to "narrow the buffer to COMMAND"?
>
> Imagine that the user only sees this one line of the doc string --
> that actually happens in apropos commands.  How can such a user
> understand what this macro does?

The macro's job is to take an Eshell command (or some fragment thereof) 
and narrow the buffer so that it's just looking at that part. This is to 
make sure that whatever is called in the body knows where to start and 
stop looking.

I agree that this isn't very clear, but I had trouble coming up with a 
concise explanation. It's essentially a workaround for how Eshell 
expects things; a lot of the Eshell command parsing functions operate on 
a range of text in the buffer. Normally, if you wanted to use those 
functions with a temporary string, you'd use `with-temp-buffer' and 
insert the string there. That doesn't work here though, since Eshell 
uses lots of buffer-local state. This function tries to abstract that 
out in a way that's useful for a few different places in Eshell.

If you have any ideas about how to improve the wording, I'm happy to 
update it though. I'll try to keep thinking as well.

> > +COMMAND can either be a string, or a cons cell demarcating a
> > +buffer region.  If COMMAND is a string, temporarily insert it
> > +into the buffer before narrowing.  Point will be set to the
> > +beginning of the narrowed region.
>
> After reading this several time and looking at the implementation, I'm
> beginning to think that COMMAND is not a good name for this argument.

Perhaps not. That comes from `eshell-parse-command' below, which takes a 
COMMAND argument of the same possible forms. There's probably a better 
name to use...

> > +(defun eshell-parse-inner-double-quote (bound)
[snip]
> This seems to just unescape characters in the string?  If so, "parse"
> is not the best name for it, and the first line of the doc string
> should say "unescape", not "parse".

Fixed.

I also reworded the manual entries. Hopefully they're a bit better.

Finally, I made a very small tweak to how quoted variable expansions 
(like $"foo") are detected. The old code wasn't reporting the right 
error if you typed:

  echo $\"foo\"

That's not correct and it should be considered invalid syntax (which it 
is now).

0001-Improve-wording-of-Eshell-variable-interpolation-cod.patch
Description: Text document