GNU bug report logs - #54227
29.0.50; [PATCH] Inconsistencies with Eshell variable interpolation

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 54227 in the body.
You can then email your comments to 54227 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] Inconsistencies with Eshell variable interpolation
Date: Wed, 2 Mar 2022 22:35:22 -0800

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

There are a few related issues with how Eshell parses variable 
interpolations, especially when mixing with double-quotes. Starting from 
"emacs -Q --eval '(eshell)':

1. Quoted subcommands

  ~ $ echo ${echo hi}
  hi  ;; good, this is what we want
  ~ $ echo "${echo hi}"
  echo hi: command not found

In this case, Eshell gets confused by the double quotes and looks for 
the command named `echo hi', not the command named `echo' with the 
argument `hi'.

2. Complex list indexing

  ~ $ echo $PATH[: 0]
  /usr/local/bin  ;; good (the first element of your $PATH)
  ~ $ echo $PATH[":" 0]
  Wrong type argument: number-or-marker-p, (eshell-escape-arg ":")

Here, Eshell neglects to evaluate the subscript arguments, so only 
literal, unquoted values work. Note that the ":" in this case means 
"split the string $PATH with colon as a delimiter". The same happens 
with Elisp subexpressions:

  ~ $ echo $exec-path[0]
  /usr/local/bin  ;; good, again
  ~ $ echo $exec-path[$(+ 1 -1)]
  Wrong type argument: number-or-marker-p, (eshell-escape-arg (let 
((indices 'nil)) (eshell-command-to-value (eshell-lisp-command '(+ 1 -1)))))

3. Bare-string indexing

  ~ $ setq foo "fooXbarXbaz"
  fooXbarXbaz
  ~ $ echo $foo[X 0]
  (nil
    #("fooXbarXbaz" 0 11
    (escaped t)))

This is similar to the above case, but here Eshell thinks that "X" is a 
literal index to use. First, it splits `foo' into a list using space as 
a delimiter. Then gets confused when it uses a string as an index into a 
list, so it returns nil. The second element of the result is the 0th 
element of `foo'-as-a-list, aka the whole string since there are no 
spaces. (Just ignore for now the fact that the string "fooXbarXbaz" 
mysteriously gained an `escaped' property. That's a separate bug.)

This last one is less of a bug and more of a missing feature: if a bare 
(non-numeric) string is used as an index for a string variable, we know 
it *must* be a delimiter argument. Since we're indexing into a list, a 
string index doesn't really make sense.

--------------------

Attached is a patch series to fix these cases, as well as a lot more 
tests for Eshell variable interpolation, since this can get tricky. I've 
also updated the documentation to clarify how indexing works. I didn't 
initially realize that you could do quite so many complex things with 
it, so I figured it'd be good to explain in more detail so it's easier 
to discover.

[0001-Move-Eshell-variable-interpolation-tests-to-their-ow.patch (text/plain, attachment)]

[0002-Add-a-new-macro-to-simplify-parsing-temporary-Eshell.patch (text/plain, attachment)]

[0003-Fix-Eshell-dollar-interpolation-inside-of-double-quo.patch (text/plain, attachment)]

[0004-Fix-parsing-of-indices-in-Eshell-expansions.patch (text/plain, attachment)]

[0005-Allow-splitting-strings-in-Eshell-expansions-with-pl.patch (text/plain, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 54227 <at> debbugs.gnu.org
Subject: Re: bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell
 variable interpolation
Date: Thu, 03 Mar 2022 14:58:49 +0100

Jim Porter <jporterbugs <at> gmail.com> writes:

> Attached is a patch series to fix these cases, as well as a lot more
> tests for Eshell variable interpolation, since this can get
> tricky. I've also updated the documentation to clarify how indexing
> works. I didn't initially realize that you could do quite so many
> complex things with it, so I figured it'd be good to explain in more
> detail so it's easier to discover.

Looks good to me; pushed to Emacs 29.

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 54227 <at> debbugs.gnu.org
Subject: Re: bug#54227: 29.0.50;
 [PATCH] Inconsistencies with Eshell variable interpolation
Date: Thu, 03 Mar 2022 18:57:07 +0200

> From: Jim Porter <jporterbugs <at> gmail.com>
> Date: Wed, 2 Mar 2022 22:35:22 -0800
> 
> +@item $@var{expr}[@var{i...}]
> +Expands to the @var{i}th element of the result of @var{expr}, an
> +expression in one of the above forms listed here.  If multiple indices
> +are supplied, this will return a list containing the elements for each
> +index.  If @var{expr}'s value is a string, it will first be split at
> +whitespace to make it a list.  If @var{expr}'s value is an alist
> +(@pxref{Association List Type, Association Lists, , elisp, The Emacs
> +Lisp Reference Manual}), this will call @code{assoc} on the result of
> +@var{expr}, returning the @code{cdr} of the element of the result
> +whose car is equal to @code{"i"}.

Thanks.

I think the documentation parts of this should be reworded to be
consistent with the manual's audience.  The Eshell manual is for
users, not necessarily for Lisp programmers.  So explaining features
in terms of Emacs primitives, such as assoc and car/cdr/caar is not
the best way of documenting them.

Can we reword this text such that the way the feature works is clear
to non-programmers as well?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 54227 <at> debbugs.gnu.org
Subject: Re: bug#54227: 29.0.50;
 [PATCH] Inconsistencies with Eshell variable interpolation
Date: Thu, 03 Mar 2022 19:03:42 +0200

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

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

> +(defun eshell-parse-inner-double-quote (bound)
> +  "Parse the inner part of a double quoted string.
> +The string to parse starts at point and ends at BOUND.
> +
> +If Eshell is currently parsing a quoted string and there are any
> +backslash-escaped characters, this will return the unescaped
> +string, updating point to BOUND.  Otherwise, this returns nil and
> +leaves point where it was."

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

Thanks.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 54227 <at> debbugs.gnu.org
Subject: Re: bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable
 interpolation
Date: Thu, 3 Mar 2022 09:56:14 -0800

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

On 3/3/2022 9:03 AM, Eli Zaretskii wrote:
>> From: Jim Porter <jporterbugs <at> 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 (text/plain, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 54227 <at> debbugs.gnu.org
Subject: Re: bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable
 interpolation
Date: Thu, 03 Mar 2022 20:43:14 +0200

> Cc: 54227 <at> debbugs.gnu.org
> From: Jim Porter <jporterbugs <at> gmail.com>
> Date: Thu, 3 Mar 2022 09:56:14 -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.

Something like the below:

  (defmacro eshell-with-temp-command (region &rest body)
    "Narrow the buffer to REGION and execute the forms in BODY.

  REGION is a cons cell (START . END) that specifies the region
  to which to narrow the buffer.  REGION can also be a string,
  in which case the macro temporarily inserts it into the
  buffer at point, and narrows the buffer to the inserted string.
  Before executing BODY, point is set to the beginning of the
  narrowed REGION.

> diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
> index 5581e5cd9e..47f8902d5a 100644
> --- a/doc/misc/eshell.texi
> +++ b/doc/misc/eshell.texi
> @@ -1043,15 +1043,16 @@ Dollars Expansion
>  index.  If @var{expr}'s value is a string, it will first be split at
>  whitespace to make it a list.  If @var{expr}'s value is an alist
>  (@pxref{Association List Type, Association Lists, , elisp, The Emacs
> -Lisp Reference Manual}), this will call @code{assoc} on the result of
> -@var{expr}, returning the @code{cdr} of the element of the result
> -whose car is equal to @code{"i"}.  Raises an error if the value is not
> -a sequence (@pxref{Sequences Arrays Vectors, Sequences, , elisp, The
> -Emacs Lisp Reference Manual}).
> +Lisp Reference Manual}), this will return the value associated with
> +the key @code{"i"}.
>  
> -Multiple sets of indices can also be specified. For example, if
> -@var{var} is a list of lists, @samp{$@var{var}[0][0]} is equivalent to
> -@samp{(caar @var{var})}.
> +Multiple sets of indices can also be specified.  For example, if
> +@var{var} is @samp{((1 2) (3 4))}, then @samp{$@var{var}[0][1]} will
> +expand to @code{2}.

I would add to the last sentence: ", i.e.@: the second element of the
first list member (all indices are zero-based)."

Also, it sounds like you just dropped the ball on the alist use case?

> -(defun eshell-parse-inner-double-quote (bound)
> -  "Parse the inner part of a double quoted string.
> +(defun eshell-unescape-inner-double-quote (bound)
> +  "Unescape the inner part of a double quoted string.

"Unescape escaped characters of a double-quoted string."

Thanks.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 54227 <at> debbugs.gnu.org
Subject: Re: bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable
 interpolation
Date: Thu, 3 Mar 2022 11:29:49 -0800

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

On 3/3/2022 10:43 AM, Eli Zaretskii wrote:
>> Cc: 54227 <at> debbugs.gnu.org
>> From: Jim Porter <jporterbugs <at> gmail.com>
>> Date: Thu, 3 Mar 2022 09:56:14 -0800
>>
>> 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.
> 
> Something like the below:
> 
>    (defmacro eshell-with-temp-command (region &rest body)
>      "Narrow the buffer to REGION and execute the forms in BODY.
> 
>    REGION is a cons cell (START . END) that specifies the region
>    to which to narrow the buffer.  REGION can also be a string,
>    in which case the macro temporarily inserts it into the
>    buffer at point, and narrows the buffer to the inserted string.
>    Before executing BODY, point is set to the beginning of the
>    narrowed REGION.

Thanks, updated to use that docstring.

>> diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
>> index 5581e5cd9e..47f8902d5a 100644
>> --- a/doc/misc/eshell.texi
>> +++ b/doc/misc/eshell.texi
>> @@ -1043,15 +1043,16 @@ Dollars Expansion
[snip]
>>   
>> -Multiple sets of indices can also be specified. For example, if
>> -@var{var} is a list of lists, @samp{$@var{var}[0][0]} is equivalent to
>> -@samp{(caar @var{var})}.
>> +Multiple sets of indices can also be specified.  For example, if
>> +@var{var} is @samp{((1 2) (3 4))}, then @samp{$@var{var}[0][1]} will
>> +expand to @code{2}.
> 
> I would add to the last sentence: ", i.e.@: the second element of the
> first list member (all indices are zero-based)."

Ok, added.

> Also, it sounds like you just dropped the ball on the alist use case?

I think we just had different ideas of how much detail was necessary. 
Given your above comment, I think I have a better idea of the level of 
detail, so I've expanded this section into a table. The single paragraph 
was a little too dense, so breaking it out into separate blocks for each 
data type makes it easier to provide a more thorough explanation.

>> -(defun eshell-parse-inner-double-quote (bound)
>> -  "Parse the inner part of a double quoted string.
>> +(defun eshell-unescape-inner-double-quote (bound)
>> +  "Unescape the inner part of a double quoted string.
> 
> "Unescape escaped characters of a double-quoted string."

Done, though I worded it as, "Unescape escaped characters inside a 
double-quoted string." I wanted to be extra-clear that this only 
operates on the bits *between* the double-quotes, but doesn't do 
anything with the surrounding double-quotes themselves.

[0001-Improve-wording-of-Eshell-variable-interpolation-cod.patch (text/plain, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 54227 <at> debbugs.gnu.org
Subject: Re: bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable
 interpolation
Date: Thu, 03 Mar 2022 21:50:46 +0200

> Cc: 54227 <at> debbugs.gnu.org
> From: Jim Porter <jporterbugs <at> gmail.com>
> Date: Thu, 3 Mar 2022 11:29:49 -0800
> 
> On 3/3/2022 10:43 AM, Eli Zaretskii wrote:
> >> Cc: 54227 <at> debbugs.gnu.org
> >> From: Jim Porter <jporterbugs <at> gmail.com>
> >> Date: Thu, 3 Mar 2022 09:56:14 -0800
> >>
> >> 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.
> > 
> > Something like the below:
> > 
> >    (defmacro eshell-with-temp-command (region &rest body)
> >      "Narrow the buffer to REGION and execute the forms in BODY.
> > 
> >    REGION is a cons cell (START . END) that specifies the region
> >    to which to narrow the buffer.  REGION can also be a string,
> >    in which case the macro temporarily inserts it into the
> >    buffer at point, and narrows the buffer to the inserted string.
> >    Before executing BODY, point is set to the beginning of the
> >    narrowed REGION.
> 
> Thanks, updated to use that docstring.
> 
> >> diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
> >> index 5581e5cd9e..47f8902d5a 100644
> >> --- a/doc/misc/eshell.texi
> >> +++ b/doc/misc/eshell.texi
> >> @@ -1043,15 +1043,16 @@ Dollars Expansion
> [snip]
> >>   
> >> -Multiple sets of indices can also be specified. For example, if
> >> -@var{var} is a list of lists, @samp{$@var{var}[0][0]} is equivalent to
> >> -@samp{(caar @var{var})}.
> >> +Multiple sets of indices can also be specified.  For example, if
> >> +@var{var} is @samp{((1 2) (3 4))}, then @samp{$@var{var}[0][1]} will
> >> +expand to @code{2}.
> > 
> > I would add to the last sentence: ", i.e.@: the second element of the
> > first list member (all indices are zero-based)."
> 
> Ok, added.
> 
> > Also, it sounds like you just dropped the ball on the alist use case?
> 
> I think we just had different ideas of how much detail was necessary. 
> Given your above comment, I think I have a better idea of the level of 
> detail, so I've expanded this section into a table. The single paragraph 
> was a little too dense, so breaking it out into separate blocks for each 
> data type makes it easier to provide a more thorough explanation.
> 
> >> -(defun eshell-parse-inner-double-quote (bound)
> >> -  "Parse the inner part of a double quoted string.
> >> +(defun eshell-unescape-inner-double-quote (bound)
> >> +  "Unescape the inner part of a double quoted string.
> > 
> > "Unescape escaped characters of a double-quoted string."
> 
> Done, though I worded it as, "Unescape escaped characters inside a 
> double-quoted string." I wanted to be extra-clear that this only 
> operates on the bits *between* the double-quotes, but doesn't do 
> anything with the surrounding double-quotes themselves.

Thanks, this now LGTM.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 54227 <at> debbugs.gnu.org
Subject: Re: bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable
 interpolation
Date: Sat, 5 Mar 2022 12:06:00 -0800

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

On 3/3/2022 11:50 AM, Eli Zaretskii wrote:
> Thanks, this now LGTM.

Thanks.

I found one more issue with the code though: the subscript operator 
doesn't work on subcommands. For example, from "emacs -Q --eval '(eshell)'":

  ~ $ echo ${*echo -e "hi\nbye"}[0]
  ("hi" "bye")

Since `${COMMAND}' forms split the output line-by-line, you'd expect 
this to say "hi", but the subscript operator is a no-op in this case. 
This was previously documented to work in the docstring for 
`eshell-apply-indices':

  For example, to retrieve the second element of a user's record in
  '/etc/passwd', the variable reference would look like:

    ${grep johnw /etc/passwd}[: 2]

I also updated the manual to indicate that this is possible (though I 
didn't provide any direct examples), since I thought this already worked 
based on that docstring.

Attached is a patch with some tests for this.

Just a note: using subscript on `$<COMMAND>' forms is probably not 
super-useful (at least not currently), though I added support for it 
anyway for consistency and future improvement. Since the result of that 
form is the name of a temp file, there's not much reason to do something 
like `$<COMMAND>[0]'. However in the future, if the subscript operator 
were more advanced, you could do something like `$<COMMAND>[/ *]' to 
split the file name by directory separators. (The "*" is a suggested 
feature in the "Bugs and ideas" section to return the whole list.)

[0001-Support-applying-indices-to-more-Eshell-dollar-expan.patch (text/plain, attachment)]

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 54227 <at> debbugs.gnu.org
Subject: Re: bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable
 interpolation
Date: Sat, 5 Mar 2022 13:44:24 -0800

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

On 3/5/2022 12:06 PM, Jim Porter wrote:
> Attached is a patch with some tests for this.

Oops, that patch had a conflict with the previous one. Here are both 
patches together, so they'll apply correctly.

[0001-Improve-wording-of-Eshell-variable-interpolation-cod.patch (text/plain, attachment)]

[0002-Support-applying-indices-to-more-Eshell-dollar-expan.patch (text/plain, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 54227 <at> debbugs.gnu.org
Subject: Re: bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable
 interpolation
Date: Mon, 07 Mar 2022 14:52:37 +0200

> From: Jim Porter <jporterbugs <at> gmail.com>
> Cc: 54227 <at> debbugs.gnu.org
> Date: Sat, 5 Mar 2022 13:44:24 -0800
> 
> +If @var{i} is a non-numeric value, expand to the value associated with
> +the key @code{"i"}. For example, if @var{var} is @samp{(("dog"

Why did you need quotes in "i"?

Also, it's suppoed to be @var{i}, as in the other alternatives.

> +@item anything else
> +Raises an error.

We say "Signals an error".

Otherwise, LGTM, thanks.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 54227 <at> debbugs.gnu.org
Subject: Re: bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable
 interpolation
Date: Mon, 7 Mar 2022 19:38:12 -0800

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

On 3/7/2022 4:52 AM, Eli Zaretskii wrote:
>> From: Jim Porter <jporterbugs <at> gmail.com>
>> Cc: 54227 <at> debbugs.gnu.org
>> Date: Sat, 5 Mar 2022 13:44:24 -0800
>>
>> +If @var{i} is a non-numeric value, expand to the value associated with
>> +the key @code{"i"}. For example, if @var{var} is @samp{(("dog"
> 
> Why did you need quotes in "i"?

That was my attempt to indicate that this:

  $foo[bar]

means (cdr (assoc "bar" foo)), not (cdr (assoc 'bar foo)) or (cdr (assoc 
bar foo)). Hopefully that's an ok way to express that. That said, this 
means the same thing too:

  $foo["bar"]

so it's not quite as simple as wrapping quotes around the subscript. 
This seemed like the simplest way to express the general idea, but it 
might be more technically correct to say something like, "... expand to 
the value associated with @var{i}'s value as a string." That wording is 
pretty confusing to me though...
> Also, it's suppoed to be @var{i}, as in the other alternatives.

Thanks, fixed.

>> +@item anything else
>> +Raises an error.
> 
> We say "Signals an error".

Fixed there, and in the entry below about the '$#foo' syntax.

[0001-Improve-wording-of-Eshell-variable-interpolation-cod.patch (text/plain, attachment)]

[0002-Support-applying-indices-to-more-Eshell-dollar-expan.patch (text/plain, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 54227 <at> debbugs.gnu.org
Subject: Re: bug#54227: 29.0.50; [PATCH] Inconsistencies with Eshell variable
 interpolation
Date: Tue, 08 Mar 2022 15:56:57 +0200

> Cc: 54227 <at> debbugs.gnu.org
> From: Jim Porter <jporterbugs <at> gmail.com>
> Date: Mon, 7 Mar 2022 19:38:12 -0800
> 
> > Also, it's suppoed to be @var{i}, as in the other alternatives.
> 
> Thanks, fixed.
> 
> >> +@item anything else
> >> +Raises an error.
> > 
> > We say "Signals an error".
> 
> Fixed there, and in the entry below about the '$#foo' syntax.

Thanks, installed on the master branch.

Previous Next

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