fish-shell icon indicating copy to clipboard operation
fish-shell copied to clipboard

Literal brace expansion is buggy

Open mqudsi opened this issue 3 years ago • 5 comments
trafficstars

A while ago, a patch was added when it was observed that the majority of cases involving {} were accidental and intended the literal braces to be printed/evaluated rather than invoking cartesian expansion. This was then also expanded to consider {x} where x is anything that doesn't contain an unescaped , to also be taken as a literal expression and not enter a special parsing mode.

I've recently discovered that this isn't done quite right and that whitespace within {} braces is emitted literally as if escaped rather than coalesced:

> printf "'%s'\n" {      }
'{      }'

I don't believe this is intentional, and even if it were, I believe it shouldn't be parsed as such. Treating { and } as literal tokens at the parser level is fine, but we don't have any concepts of literal whitespace except when individually prefixed with \ or when in a quoted context. It also goes against how brace expansion handles whitespace in general:

> printf "'%s'\n" { hello , world }
'hello'
'world'

It's ok to treat {one} as a single, literal token but IMHO { one } should be treated as three separate tokens.

mqudsi avatar Nov 13 '22 22:11 mqudsi

I'm going to close this in favor of #5880, because that's the underlying issue - braces are still treated specially until at some point we figure out they shouldn't be, and at that point we only check for , or any other expansion.

faho avatar Nov 14 '22 07:11 faho

I respectfully disagree; this is completely separate. Here, you've already encountered a fully-formed bracket pair and have decided at the parser level to insert a hack to support certain user cases. You already have the legal, fully-formed token; you just need to re-tokenize it correctly.

More fundamentally, I don't see how this can be a duplicate of #5880 since I disagree with #5880 altogether and have lodged my objections there. This can be closed-as-fixed without closing #5880 as-fixed so they can't be duplicates.

mqudsi avatar Nov 15 '22 17:11 mqudsi

The behavior was introduced in #6565, which was a patch that we knew broke some things but punted on fixing them until after the 3.1 release. For what it is worth, it seems that at least some of the behavior we all agreed is buggy or out-of-spec was intentional at the time the PR was drafted, though in the comments everyone seems to mostly agree that it's wrong and should be changed.

mqudsi avatar Nov 16 '22 19:11 mqudsi

Thanks for linking the previous discussion. The gist is

Braces are now equivalent to a new form of quoting, except the quote is preserved and emitted in the output.

so at least for matching braces without an enclosed comma, we should probably remove the special treatment

krobelus avatar Nov 16 '22 20:11 krobelus

After reading the previous issues, I no longer know what is the desired behavior here. (I know what I would like, but that's not the same question.)

mqudsi avatar Nov 17 '22 17:11 mqudsi

Thanks for linking the previous discussion. The gist is

Braces are now equivalent to a new form of quoting, except the quote is preserved and emitted in the output.

so at least for matching braces without an enclosed comma, we should probably remove the special treatment

This feature—unintended or not—is very useful and, personally, I make heavy use of it.

ChristoferK avatar Feb 09 '23 15:02 ChristoferK

You could always just use quotes.

faho avatar Feb 09 '23 15:02 faho