New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weird brace expansion behavior with multiple commas #3002

Closed
andgra2 opened this Issue May 7, 2016 · 15 comments

Comments

Projects
None yet
7 participants
@andgra2
Copy link

andgra2 commented May 7, 2016

The brace expansion in x{,,}y does not work correctly.

Reproduction Steps:

  1. Run the following command: echo x{,,}y

Expected behavior:

The output should be: xy xy xy

This is also the output that bash gives.

Observed behaviour:

The observed output: xy x,y

Looks like fish incorrectly thinks that we wrote echo x{,\,}y

Additional information:


Fish version: fish, version 2.2.0-876-g1c6f6df

Operating system: ubuntu 64-bit, installed fish from github source code.

Terminal or terminal emulator: neo vims built in terminal emulator.

@ghost

This comment has been minimized.

Copy link
Contributor

ghost commented May 7, 2016

Please also take these examples into consideration:

set z
echo -{$z,$z,$z}:

should it output -: -: -:, or nothing because it is like doing echo -$z: -$z: -$z:?

set z
echo -{z,$z}:

should it output -z: -: or -z:? (but actually it outputs nothing)

@krader1961 krader1961 added the bug label May 7, 2016

@krader1961 krader1961 added this to the fish-future milestone May 7, 2016

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented May 7, 2016

The documentation says

The latter syntax works by exploiting brace expansion; care should be taken with array variables and undefined variables, as these behave very differently to POSIX shells.

It would be nice if that behavior was actually documented rather than hinted at. In fact, I can't find anywhere in the documentation where it talks about the cartesian product behavior of constructs like

$ set foo a b c
$ echo [ X$foo ]
[ Xa Xb Xc ]

Presumably the person who wrote the fish code that handles brace expansion intended it to behave like variable concatenation. If the var is empty then the cartesian product is empty:

$ set foo
$ echo [ X$foo ]
[ ]

So the question is whether {,,} should be equivalent to concatenation with a var having zero elements (set foo) or a var with three empty strings (set foo "" "" ""). Either way the current behavior is clearly wrong.

@andgra2

This comment has been minimized.

Copy link

andgra2 commented May 7, 2016

@jakwings:

Those are also interesting examples. $z is an empty array. So i expect -{$z,$z,$z}: to expand as if {$z,$z,$z} was replaced by the union of $z, $z and $z ie empty array, so echo -{$z,$z,$z}: should output nothing according to me.

In your second example z is also an empty array. So I expect -{z,$z}: to expand as if {z,$z} was replaced by the union of z and $z, ie an array with the single element z, so echo -{z,$z}: should output -z: according to me.

Ie I expect things in brace expressions to get unioned togheter.

faho added a commit that referenced this issue May 7, 2016

@faho

This comment has been minimized.

Copy link
Member

faho commented May 7, 2016

@krader1961:

It would be nice if that behavior was actually documented rather than hinted at.

Fixed by 1d101ef - we already have a "cartesian-product" subsection.

@faho

This comment has been minimized.

Copy link
Member

faho commented May 7, 2016

So the question is whether {,,} should be equivalent to concatenation with a var having zero elements (set foo) or a var with three empty strings (set foo "" "" ""). Either way the current behavior is clearly wrong.

Well, a zero element brace expansion would be {}. Otherwise {,} and {,,} would be the exact same thing. So I'd say three empty strings is the way to go.

Note that for the first and last empty element it already works like that so {,"",} expands to three empty strings (and {,} works like that), so it would be more consistent.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented May 7, 2016

Fixed by 1d101ef - we already have a "cartesian-product" subsection.

Ah, yes, you're right. I need to get in the habit of running make doc and looking at that output in addition to http://fishshell.com. Although I see you just added a much needed rewrite of the sentence in question 😄 So that just leaves my question of whether empty elements inside braces should be treated as empty strings or missing (i.e. non-existent). And we also need to carefully consider other variations on this theme such as those in issue #3004 involving variable expansion inside braces.

@andgra2

This comment has been minimized.

Copy link

andgra2 commented May 7, 2016

@krader1961:

Thats an interesting question. Currently {} expands to a single empty string, and {,} expands to two empty strings, so it seems consistent with that to let {,,} to expand to 3 empty strings.

I think I would have preferred {} to considered having zero things {''} to having a single empty string and {,} to be illegal, but that is another thing and things doesn't work that way and probably will not change.

@ghost

This comment has been minimized.

Copy link
Contributor

ghost commented May 7, 2016

@faho Yeah, I added that subsection based on the actual behavior of fish. But I am surprised by an empty $var and empty {} now.

mv file{,.bak} can prove zero elements in brace expansion should be treated as empty strings, unless most people agree they are tempted to rewrite the code as mv file{"",.bak}. Similarly, {$var}text could be rewritten to $var"text" if the braces just act as separators. However I can imagine this is a "won't fix".

@andgra2

This comment has been minimized.

Copy link

andgra2 commented May 7, 2016

@jakwings: The two examples of yours are related to #3004 I think.

@ghost

This comment has been minimized.

Copy link
Contributor

ghost commented May 7, 2016

@andgra2 There is no zero element in #3004. Your wording of "union" for my two examples is a bit obscure. Do you mean {$z,$z,$z} equals to {}, and {z,$z} equals to {z}, i.e. empty variables are removed? {,,} equals to {"","",""} because there is no empty variable?

@andgra2

This comment has been minimized.

Copy link

andgra2 commented May 7, 2016

@jakwings: Yes, but there I expected the things inside the brace expression to be unioned togheter and the same reasoning would make {$z,$z,$z} expand the same way as an empty array (the union of 3 empty arrays is an empty array). Or the same way as a theoretical empty brace expression (but such a brace expression cannot be written because {} is interpreted as {''} and thus a brace expression having a single empty string).

I consider empty variables as beeing empty arrays and in a union, empty arrays (or lists) can be ignored (they don't affect the result), so that empty variables are being removed is just a special case of how unioning works.

By union of arrays (or lists like the result of a brace expansion) I just mean array/list concatenation.

To clarify what I mean For example I think that x{{a,b},$arr1,e,$z}y where $arr is defined by set $arr c d and $z defined by set z (ie is an empty array) should be expanded the same way as x{a,b,c,d,e}y

faho added a commit that referenced this issue May 8, 2016

@stephane-chazelas

This comment has been minimized.

Copy link

stephane-chazelas commented Aug 5, 2016

A note about other shells: rc is another shell where x$a is not concatenation:

$ rc -c 'a=(a b c); echo x$a'
xa xb xc

But note that it doesn't work the same as in fish when concatenating two (or more) arrays:

$ rc -c 'a=(a b c); echo $a$a'
aa bb cc

You can only join arrays which are the same size (or have zero or one element)

$ rc -c 'a=(a) b=(1 2 3) c=(); echo $a$a$b$a$b$c'
aa1a1 aa2a2 aa3a3
$ rc -c 'a=(1 2) b=(1 2 3); echo $a$b'
rc: line 0: bad concatenation

zsh can do fish's way with:

$ zsh -c 'a=(a b c); echo $^a$^a'
aa ab ac ba bb bc ca cb cc

I personally find more intuitive $a$a being concatenation and having to ask explicitly when we want the Cartesian product.

See also zsh's zip operators for something similar to the rc way:

$ zsh -c 'a=(a b c); echo ${a:^a}'
a a b b c c
@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Aug 5, 2016

I personally find more intuitive $a$a being concatenation and having to ask explicitly when we want the Cartesian product.

I don't disagree but that ship has already sailed. We certainly can't change this in a minor release. Even changing this in a major release would require considerable discussion and planning.

@faho faho changed the title Brace expansion Weird brace expansion behavior with multiple commas Aug 17, 2016

@bootofood

This comment has been minimized.

Copy link

bootofood commented Oct 12, 2016

{a,,b}c should definitely expand to ac c bc!
Please fix this issue...

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Jun 21, 2017

Retargeting to next-major because anything we do will almost certainly break backward compatibility.

@krader1961 krader1961 modified the milestones: next-major, fish-future Jun 21, 2017

@krader1961 krader1961 added enhancement and removed bug labels Jun 21, 2017

faho added a commit to faho/fish-shell that referenced this issue Jan 2, 2018

@faho faho referenced this issue Jan 2, 2018

Closed

Fix brackets #4632

2 of 3 tasks complete

@faho faho closed this in aa58cae Jan 7, 2018

@zanchey zanchey removed this from the next-major milestone Jan 20, 2018

@zanchey zanchey added this to the fish-3.0 milestone Jan 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment