Re: Documentation issue

[Greg Wooledge]

On Mon, Oct 30, 2017 at 12:05:51AM -0400, Eli Barzilay wrote:
> Again, `unset` has different expectations, because it seems like it
> would be a special construct

Only to you, and only because you brought in some expectations from
a different language.

> -- since it's dealing with *lvalues*.

wooledg:~$ man bash | grep -i lvalue
wooledg:~$ 

Concepts from other languages do not work the same way in bash.


Here's a paragraph from bash(1):

   The unset builtin is used to  destroy  arrays.   unset  name[subscript]
   destroys  the array element at index subscript.  Negative subscripts to
   indexed arrays are interpreted as described above.  Care must be  taken
   to  avoid  unwanted  side  effects caused by pathname expansion.  unset
   name, where name is an array, or unset name[subscript], where subscript
   is * or @, removes the entire array.

This has two unquoted instances of name[subscript].  You could put quotes
around those to provide better examples.  (The man page as a whole is
quite short on examples.)

Re: Documentation issue

[Eli Barzilay]

On Sun, Oct 29, 2017 at 10:44 PM, Clark Wang  wrote:
>
> Different people have different expectations which they believe are
> all important.

Yes, and I have already went over this several times: this is an issue
that does confuse people, both ones that I have spoken to and in
tutorial/example/etc pages that come up on top when searching.


> It's not possible to cover so many tricky things in the manual.

It definitely is; it's a tradeoff between things that are worth
mentioning and the size becoming big enough that people won't bother
reading it, hence I tried to suggest a very small change.


> To me the most important thing is to describe the syntax correctly.

This is a non-point.  I obviously don't think that some explanation
should be added *instead* of existing descriptions, or instead of the
current completeness level.


> When you understand the syntax "unset arr[i]" would never surprise
> you.

Again, `unset` has different expectations, because it seems like it
would be a special construct -- since it's dealing with *lvalues*.  Like
I said, the other place where array reference lvalues appear in bash,
assignments, do treat these "lvalue expressions" differently, and even
though it is for a different reason, it makes it easier to get confused
and assume that `unset` is special too.  To summarize:

* In assignments like a[0]=foo, the a[0] part is treated in a special
  way because otherwise it will not be an assignment.  Specifically,
  quoting the whole "a[0]" is not needed, and adding it will break in an
  obvious way.

* In plain variable referendces, $a[0] is wrong anyway, and in ${a[0]}
  there is no need to quote the a[0] since no path expansion is done
  there anyway, and, again, it will break in an obvious way.

* The last kind of reference is unset, and after the above two it is
  reasonable to assume that it *will* be special and will not require
  quoting as well.  That's why I think that a warning that it is not
  special is warranted.

Yet another way to phrase the above: I don't think that you can mess up
these assignments or references with a file like "a0"; but you can do so
for `unset`.

-- 
   ((x=>x(x))(x=>x(x)))  Eli Barzilay:
   http://barzilay.org/  Maze is Life!

Re: Documentation issue

[Clark Wang]

On Fri, Oct 27, 2017 at 3:28 PM, Eli Barzilay  wrote:

> On Fri, Oct 27, 2017 at 2:37 AM, Clark Wang  wrote:
> >
> > What `unset' does is special but there's nothing special when parsing
> > the command and bash even does not care if it's built-in command or
> > not.
>
> Exactly -- and this kind of a clarification is exactly the thing that
> many people are unaware of.
>
> (And in case it wasn't clear: I said that unset is different in the
> expectations that people have with it, not in how it expands.)
>

Different people have different expectations which they believe are all
important. It's not possible to cover so many tricky things in the manual.
To me the most important thing is to describe the syntax correctly. When
you understand the syntax "unset arr[i]" would never surprise you.

Re: Documentation issue

[Eli Barzilay]

On Fri, Oct 27, 2017 at 2:12 PM, Chet Ramey  wrote:
>
> Let's see if I can find some compromise language that will take care
> of the general case. Since it's a compromise, no one will be
> satisfied, of course.

Thanks -- any improvement would be good.

-- 
   ((x=>x(x))(x=>x(x)))  Eli Barzilay:
   http://barzilay.org/  Maze is Life!

Re: Documentation issue

[Chet Ramey]

On 10/26/17 10:50 PM, Clark Wang wrote:
> On Fri, Oct 27, 2017 at 3:00 AM, Eli Barzilay  wrote:
> 
>> On Thu, Oct 26, 2017 at 2:02 PM, Chet Ramey  wrote:
>>>
>>> It's more of a general statement about arrays, though it appears in
>>> the paragraph that discusses unset, so it's in the man page section on
>>> arrays.  You have to be careful about putting the same information in
>>> too many different places -- the man page is big enough already.
>>
>> I'm very aware of man page bloat and the fact that the bash page is very
>> long as is.  But (a) I think that the builtin section for each command
>> is the more important place for such things; and (b) what I'm suggesting
>> is just a short reminder sentence about the need for quoting unsets for
>> array elements.
>>
>> To make it more concrete, I think that the following change will be
>> good:
>>
>> 1. Drop the current "Care must be taken ... the entire array." two
>>sentences and replace them with some "See the unset builtin
>>description below".
>>
> 
> It's not only about unset. You also need to take care of other builtin
> commands:

Let's see if I can find some compromise language that will take care
of the general case. Since it's a compromise, no one will be satisfied,
of course.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/

Re: Documentation issue

[Eli Barzilay]

On Fri, Oct 27, 2017 at 2:37 AM, Clark Wang  wrote:
>
> What `unset' does is special but there's nothing special when parsing
> the command and bash even does not care if it's built-in command or
> not.

Exactly -- and this kind of a clarification is exactly the thing that
many people are unaware of.

(And in case it wasn't clear: I said that unset is different in the
expectations that people have with it, not in how it expands.)

-- 
   ((x=>x(x))(x=>x(x)))  Eli Barzilay:
   http://barzilay.org/  Maze is Life!

Re: Documentation issue

[Clark Wang]

On Fri, Oct 27, 2017 at 1:17 PM, Eli Barzilay  wrote:

>
> I already said why `unset` is different.  If it wasn't clear, a direct
> example is the fact that `delete` in javascript is a special syntax
> rather than a function.  To make it more confusing, the other obvious
> place where an lvalue appears (left of a =), is special in bash for
> other reasons, but it does make it easier to assume that `unset` is
> special.
>

What `unset' does is special but there's nothing special when parsing the
command and bash even does not care if it's built-in command or not.

Re: Documentation issue

[Eli Barzilay]

On Thu, Oct 26, 2017 at 10:50 PM, Clark Wang  wrote:
> On Fri, Oct 27, 2017 at 3:00 AM, Eli Barzilay  wrote:
>>
>> 1. Drop the current "Care must be taken ... the entire array." two
>>sentences and replace them with some "See the unset builtin
>>description below".
>
> It's not only about unset. You also need to take care of other builtin
> commands: [...]

I already said why `unset` is different.  If it wasn't clear, a direct
example is the fact that `delete` in javascript is a special syntax
rather than a function.  To make it more confusing, the other obvious
place where an lvalue appears (left of a =), is special in bash for
other reasons, but it does make it easier to assume that `unset` is
special.

-- 
   ((x=>x(x))(x=>x(x)))  Eli Barzilay:
   http://barzilay.org/  Maze is Life!

Re: Documentation issue

[Clark Wang]

On Fri, Oct 27, 2017 at 3:00 AM, Eli Barzilay  wrote:

> On Thu, Oct 26, 2017 at 2:02 PM, Chet Ramey  wrote:
> >
> > It's more of a general statement about arrays, though it appears in
> > the paragraph that discusses unset, so it's in the man page section on
> > arrays.  You have to be careful about putting the same information in
> > too many different places -- the man page is big enough already.
>
> I'm very aware of man page bloat and the fact that the bash page is very
> long as is.  But (a) I think that the builtin section for each command
> is the more important place for such things; and (b) what I'm suggesting
> is just a short reminder sentence about the need for quoting unsets for
> array elements.
>
> To make it more concrete, I think that the following change will be
> good:
>
> 1. Drop the current "Care must be taken ... the entire array." two
>sentences and replace them with some "See the unset builtin
>description below".
>

It's not only about unset. You also need to take care of other builtin
commands:

cd: cd a[dir] vs. cd 'a[dir]'
echo: echo foo[bar] vs. echo 'foo[bar]'; echo * vs. echo '*'
eval: eval a[i]=b vs. eval 'a[i]=b'
printf: printf foo[bar] vs. printf 'foo[bar]'
set: set a[b] vs. set 'a[b]'
source: source file[name] vs. source 'file[name]'
test: test a[b] vs. test 'a[b]'
...

and even for external commands:

find /the/dir -name *.txt

Re: Documentation issue

[Eli Barzilay]

On Thu, Oct 26, 2017 at 2:02 PM, Chet Ramey  wrote:
>
> It's more of a general statement about arrays, though it appears in
> the paragraph that discusses unset, so it's in the man page section on
> arrays.  You have to be careful about putting the same information in
> too many different places -- the man page is big enough already.

I'm very aware of man page bloat and the fact that the bash page is very
long as is.  But (a) I think that the builtin section for each command
is the more important place for such things; and (b) what I'm suggesting
is just a short reminder sentence about the need for quoting unsets for
array elements.

To make it more concrete, I think that the following change will be
good:

1. Drop the current "Care must be taken ... the entire array." two
   sentences and replace them with some "See the unset builtin
   description below".

2. Add those two sentences to the unset builtin section, as a new
   paragraph.

3. Tweak it slightly to explicitly encourage quoting:

   Care must be taken to avoid unwanted side effects caused by
   pathname expansion, (i.e., prefer unset "name[subscript]" to
   avoid such problems).  unset name, where name is an array, or
   unset name[subscript], where subscript is * or @, removes the
   entire array.

Note that there are two very small additions to the overall text:

1. A "see also" reference (to ensure that people don't miss it from
   either place).

2. The parenthetical comment.

-- 
   ((x=>x(x))(x=>x(x)))  Eli Barzilay:
   http://barzilay.org/  Maze is Life!

Re: Documentation issue

[Chet Ramey]

On 10/26/17 11:28 AM, Eli Barzilay wrote:

>> I'm not sure why this is a surprise. Pathname expansion (globbing) is
>> one of the word expansions performed before a simple command is
>> executed. The `unset' builtin is no different.
> 
> The last sentence is showing why it's a surprise: it is confusing since
> it easy to think that unset is special, similar to languages which have
> something like `delete foo[1]` where the thing that follows delete is an
> lvalue.

I understand. There are plenty of misconceptions out there. But the bash
documentation has never implied that `unset' is special in that way, and
it's not the man page's place to say everything the shell is not.

>>> The thing is that AFAICT, there is no mention of this pitfall in the
>>> man page...
>>
>> "The  unset  builtin  is  used to destroy arrays.  unset name[subscript]
>> destroys the array element at index subscript.  Negative subscripts  to
>> indexed  arrays are interpreted as described above.  Care must be taken
>> to avoid unwanted side effects caused  by  pathname  expansion."
> 
> 1. This is much more indirect than a simple "always quote array
>references";

Because it's much more general than a blanket statement like that, and the
man page isn't the place for those statements. That's the job for a shell
programming guide, of which there are plenty.

> 
> 2. I completely missed it since it's not in the place which describes
>unset.
> 
> (BTW, when I did dare for the first time to use unset on an array I did
> go through the unset description, and got a vague impression that it's
> kind of doing the special lvalue thing, so possibly the indirect warning
> would have been sufficient to slap me back into the bash reality.)

It's more of a general statement about arrays, though it appears in the
paragraph that discusses unset, so it's in the man page section on arrays.
You have to be careful about putting the same information in too many
different places -- the man page is big enough already.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/

Re: Documentation issue

[Eli Barzilay]

Pierre Gaston:
> I think it's even more likely to happen with eg: read array[i]

Maybe, but see below.

> There is a large number of pitfalls in bash
> (http://mywiki.wooledge.org/BashPitfalls) that most people ignore.

And it doesn't even mention the unset problem, I think.


On Thu, Oct 26, 2017 at 8:56 AM, Chet Ramey  wrote:
> On 10/26/17 1:18 AM, Eli Barzilay wrote:
>> Bash surprised me with the behavior mentioned here:
>>
>> https://stackoverflow.com/questions/15897473
>>
>> This can be pretty bad in that it's very unexpected (see the
>> comments).
>
> I'm not sure why this is a surprise. Pathname expansion (globbing) is
> one of the word expansions performed before a simple command is
> executed. The `unset' builtin is no different.

The last sentence is showing why it's a surprise: it is confusing since
it easy to think that unset is special, similar to languages which have
something like `delete foo[1]` where the thing that follows delete is an
lvalue.


>> The thing is that AFAICT, there is no mention of this pitfall in the
>> man page...
>
> "The  unset  builtin  is  used to destroy arrays.  unset name[subscript]
> destroys the array element at index subscript.  Negative subscripts  to
> indexed  arrays are interpreted as described above.  Care must be taken
> to avoid unwanted side effects caused  by  pathname  expansion."

1. This is much more indirect than a simple "always quote array
   references";

2. I completely missed it since it's not in the place which describes
   unset.

(BTW, when I did dare for the first time to use unset on an array I did
go through the unset description, and got a vague impression that it's
kind of doing the special lvalue thing, so possibly the indirect warning
would have been sufficient to slap me back into the bash reality.)

-- 
   ((x=>x(x))(x=>x(x)))  Eli Barzilay:
   http://barzilay.org/  Maze is Life!

Re: Documentation issue

[Chet Ramey]

On 10/26/17 1:18 AM, Eli Barzilay wrote:
> Bash surprised me with the behavior mentioned here:
> 
> https://stackoverflow.com/questions/15897473
> 
> This can be pretty bad in that it's very unexpected (see the comments).

I'm not sure why this is a surprise. Pathname expansion (globbing) is one
of the word expansions performed before a simple command is executed. The
`unset' builtin is no different.


> The thing is that AFAICT, there is no mention of this pitfall in the man
> page...  

"The  unset  builtin  is  used to destroy arrays.  unset name[subscript]
destroys the array element at index subscript.  Negative subscripts  to
indexed  arrays are interpreted as described above.  Care must be taken
to avoid unwanted side effects caused  by  pathname  expansion."

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/

Re: Documentation issue

[Pierre Gaston]

On Thu, Oct 26, 2017 at 8:18 AM, Eli Barzilay  wrote:

> Bash surprised me with the behavior mentioned here:
>
> https://stackoverflow.com/questions/15897473
>
> This can be pretty bad in that it's very unexpected (see the comments).
> Also, the surprise can be triggered without nullglob as well:
>
> $ foo=(a b c)
> $ touch foo0
> $ unset foo[0]
> $ echo ${foo[*]}
> a b c
>
> The thing is that AFAICT, there is no mention of this pitfall in the man
> page...  It would be nice to mention using quotes in at least the
> `unset` description, and possibly also about `nullglob` too since it
> makes it easier to run into this problem.
>
> I grepped through the bash sources, and even there I found a few unsafe
> uses:
>
> grep -r 'unset[^a-z"'\'']*\[' examples tests
>
> so this is clearly something that is not well-known enough.
>
> --
>((x=>x(x))(x=>x(x)))  Eli Barzilay:
>http://barzilay.org/  Maze is Life!
>
>
I think it's even more likely to happen with eg: read array[i]

There is a large number of pitfalls in bash (
http://mywiki.wooledge.org/BashPitfalls) that most people ignore.
I'm not sure where to rank this one in the priorities of the ones that
would need to be mentioned in the manual.
Maybe one could create a separate man/info page that the manual could
reference?

Re: Documentation issue: Increments in brace expansion

[Chet Ramey]

Jan Schampera wrote:
 Good morning,
 
 http://www.gnu.org/software/bash/manual/bashref.html#Brace-Expansion
 
 The brace expansion increment syntax is shown wrong.
 
 OLD:
 
 A sequence expression takes the form {x..y[incr]}, where x and y are
 either integers or single characters, and incr, an optional increment,
 is an integer.
 
 
 PROPOSED NEW:
 
 A sequence expression takes the form {x..y[..incr]}, where x and y are
 either integers or single characters, and incr, an optional increment,
 is an integer separated by `..'.
 
 
 
 
 Sorry if this already is fixed in the base documents, I didn't have time
 to check deeply.

It is already fixed.  Thanks for the catch.

Chet
-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/