fish-shell / fish-shell

The user-friendly command line shell.
https://fishshell.com
Other
25.99k stars 1.9k forks source link

3.1b1: The consumption of whitespace inside braces has changed weirdly since 3.0 #6564

Closed Phidica closed 4 years ago

Phidica commented 4 years ago

Tried using tagged release 3.1b1 as well as current master (1101cff56) built from source on Fedora 30, running in Gnome Terminal.

I just noticed this while responding to #5880.

In 3.0, extra whitespace inside of braces was consumed from both sides:

$ echo .{  foo  bar  }.
.foo  bar.

In 3.1b1, (perhaps due to the changes from #5869?) we trim whitespace but only from the front:

$ echo .{  foo  bar  }.
.{foo  bar  }.

Honestly, I'm not sure what should be happening. Do we want to preserve everything inside of braces that don't contain a comma/variable? Or, do we still trim spaces from the front and back like we do when there is a comma/variable? Consider that

$ echo .{  foo  ,  bar  }.
.foo. .bar.

still comes out symmetric in 3.1b1.

ridiculousfish commented 4 years ago

Well spotted, bisects to 967c1d51eec0f840273c20a326152ea72f920d3d. IMO this should block 3.1.

mqudsi commented 4 years ago

Honestly, I'm not sure what should be happening. Do we want to preserve everything inside of braces that don't contain a comma/variable?

If the braces are not being treated as special (and that was what @faho was going for, I believe), then they should be treated the same way as any other whitespace.

There is a bigger problem here: the tokenization is borked.

This is the output with brace expansion as of 3.0 (and even now):

mqudsi@Blitzkrieg /m/d/G/fmath> printf "\"%s\"\n" { line 1, line 2 }
"line 1"
"line 2"

Both leading and trailing unescaped whitespace is removed, unescaped intra-word whitespace is coalesced, and tokens are split by literal , rather than whitespace.

(EDIT: it seems that intraword whitespace is not being coalesced. I'm not sure if this is how 3.0 shipped or if this is a regression?)

In 967c1d5, I believe @faho intended to remove all special treatment of braces containing a single, non-variable item. It's not (just) a matter of whitespace only being trimmed from the front, the tokenization is completely wrong as (based off the original issue that was trying to be fixed and everything that I can see) the output should be as if the brace were escaped, i.e. no special casing whatsoever.

mqudsi@Blitzkrieg /m/d/G/fmath> printf "\"%s\"\n" { hello world }
"{hello world }"
mqudsi@Blitzkrieg /m/d/G/fmath> printf "\"%s\"\n" \{ hello world \}
"{"
"hello"
"world"
"}"

I would expect the two outputs to be identical, otherwise we have introduced considerable complexity into the language (which is one of the reasons I was against special-casing a single- or no-item brace bair, as while it may make the language more friendly, it increases the cognitive cost of understanding the language itself), and instead of having two possible states (brace expansion and no brace expansion) there are now three (brace expansion, no brace expansion, and "brace expansion but printing the output with the braces preserved").