Only do brace expansion if they contain a variable or ","

Brace expansion with single words in it is quite useless - `HEAD@{0}`
expanding to `HEAD@0` breaks git.

So we complicate the rule slightly - if there is no variable expansion
or "," inside of braces, they are just treated as literal braces.

Note that this is technically backwards-incompatible, because

    echo foo{0}

will now print `foo{0}` instead of `foo0`. However that's a
technicality because the braces were literally useless in that case.

Our tests needed to be adjusted, but that's because they are meant to
exercise this in weird ways.

I don't believe this will break any code in practice.

Fixes #5869.

CHANGELOG.md

@@ -16,7 +16,7 @@
 - fish now underlines every valid entered path instead of just the last one.
 
 ### Syntax changes and new commands
-- None yet.
+- Brace expansion now only takes place if the braces include a "," or a variable expansion, so things like `git reset HEAD@{0}` now work (#5869).
 
 ### Scripting improvements
 - `string split0` now returns 0 if it split something (#5701).

sphinx_doc_src/index.rst

@@ -764,31 +764,41 @@ Examples::
 Brace expansion
 ---------------
 
-A comma separated list of characters enclosed in curly braces will be expanded so each element of the list becomes a new parameter.
+A comma separated list of characters enclosed in curly braces will be expanded so each element of the list becomes a new parameter. This is useful to save on typing, and to separate a variable name from surrounding text.
 
 Examples::
 
-  echo input.{c,h,txt}
+  > echo input.{c,h,txt}
-  # Outputs 'input.c input.h input.txt'
+  input.c input.h input.txt
 
-  mv *.{c,h} src/
+  > mv *.{c,h} src/
   # Moves all files with the suffix '.c' or '.h' to the subdirectory src.
 
-A literal "{}" will not be used as a brace expansion, but if after expansion there is nothing between the braces, the argument will be removed::
+  > cp file{,.bak}
+  # Make a copy of `file` at `file.bak`.
 
-    echo foo-{}
+  > set -l dogs hot cool cute
-    # Outputs foo-{}
+  > echo {$dogs}dog
+  hotdog cooldog cutedog
 
-    echo foo-{$undefinedvar}
+If two braces do not contain a "," or a variable expansion, they will not be expanded in this manner::
-    # Output is an empty line - see :ref:`the cartesian product section <cartesian-product>`
 
+    > echo foo-{}
+    foo-{}
+    > git reset --hard HEAD@{2}
+    # passes "HEAD@{2}" to git
+    > echo {{a,b}}
+    {a} {b} # because the inner brace pair is expanded, but the outer isn't.
 
-If there is nothing between a brace and a comma or two commas, it's interpreted as an empty element.
+If after expansion there is nothing between the braces, the argument will be removed (see :ref:`the cartesian product section <cartesian-product>`)::
 
-So::
+    > echo foo-{$undefinedvar}
+    # Output is an empty line, just like a bare `echo`.
 
-    echo {,,/usr}/bin
+If there is nothing between a brace and a comma or two commas, it's interpreted as an empty element::
-    # Output /bin /bin /usr/bin
+
+    > echo {,,/usr}/bin
+    /bin /bin /usr/bin
 
 To use a "," as an element, `quote <#quotes>`_ or `escape <#escapes>`_ it.
 

src/common.cpp

@@ -1448,6 +1448,11 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in
     const bool unescape_special = static_cast<bool>(flags & UNESCAPE_SPECIAL);
     const bool allow_incomplete = static_cast<bool>(flags & UNESCAPE_INCOMPLETE);
 
+    // The positions of open braces.
+    std::vector<size_t> braces;
+    // The positions of variable expansions or brace ","s.
+    // We only read braces as expanders if there's a variable expansion or "," in them.
+    std::vector<size_t> vars_or_seps;
     bool brace_text_start = false;
     int brace_count = 0;
 
@@ -1456,7 +1461,6 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in
         mode_unquoted,
         mode_single_quotes,
         mode_double_quotes,
-        mode_braces
     } mode = mode_unquoted;
 
     for (size_t input_position = 0; input_position < input_len && !errored; input_position++) {
@@ -1523,6 +1527,7 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in
                 case L'$': {
                     if (unescape_special) {
                         to_append_or_none = VARIABLE_EXPAND;
+                        vars_or_seps.push_back(input_position);
                     }
                     break;
                 }
@@ -1530,6 +1535,8 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in
                     if (unescape_special) {
                         brace_count++;
                         to_append_or_none = BRACE_BEGIN;
+                        // We need to store where the brace *ends up* in the output because of NOT_A_WCHAR.
+                        braces.push_back(result.size());
                     }
                     break;
                 }
@@ -1544,6 +1551,25 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in
                         brace_count--;
                         brace_text_start = brace_text_start && brace_count > 0;
                         to_append_or_none = BRACE_END;
+                        if (braces.size()) {
+                            // If we didn't have a var or separator since the last '{',
+                            // put the literal back.
+                            if (!vars_or_seps.size() || vars_or_seps.back() < braces.back()) {
+                                result[braces.back()] = L'{';
+                                // We also need to turn all spaces back.
+                                for (size_t i = braces.back() + 1; i < result.size(); i++) {
+                                    if (result[i] == BRACE_SPACE) result[i] = L' ';
+                                }
+                                to_append_or_none = L'}';
+                            }
+
+                            // Remove all seps inside the current brace pair, so if we have a surrounding pair
+                            // we only get seps inside *that*.
+                            if (vars_or_seps.size()) {
+                                while(vars_or_seps.size() && vars_or_seps.back() > braces.back()) vars_or_seps.pop_back();
+                            }
+                            braces.pop_back();
+                        }
                     }
                     break;
                 }
@@ -1551,6 +1577,7 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in
                     if (unescape_special && brace_count > 0) {
                         to_append_or_none = BRACE_SEP;
                         brace_text_start = false;
+                        vars_or_seps.push_back(input_position);
                     }
                     break;
                 }
@@ -1652,6 +1679,7 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in
                 case '$': {
                     if (unescape_special) {
                         to_append_or_none = VARIABLE_EXPAND_SINGLE;
+                        vars_or_seps.push_back(input_position);
                     }
                     break;
                 }

tests/parameter_expansion.in

@@ -15,7 +15,7 @@ for phrase in {good\,,   beautiful ,morning}; echo -n "$phrase "; end | string t
 for phrase in {goodbye\,,\ cruel\ ,world\n}; echo -n $phrase; end;
 
 # whitespace within entries converted to spaces in a single entry
-for foo in { hello
+for foo in {a, hello
 world }
 	echo \'$foo\'
 end
@@ -31,4 +31,5 @@ end
 # subshells with expansion
 for name in (for name in {Beth, Amy}; printf "$name\n"; end); printf "$name\n"; end
 
+echo {{a,b}}
 # vim: set ft=fish:

tests/parameter_expansion.out

@@ -1,14 +1,16 @@
 {}
-apple
+{apple}
 apple orange
 apple orange
 apple orange banana
 'hello' 'world'
 good, beautiful morning
 goodbye, cruel world
+'a'
 'hello world'
 alpha lambda, beta lambda, alpha gamma, beta gamma
 Meg
 Jo
 Beth
 Amy
+{a} {b}

tests/test1.err

@@ -3,7 +3,7 @@
 # Comments in odd places don't cause problems
 
 ####################
-# Bracket expansion
+# Brace expansion
 
 ####################
 # Escaped newlines

tests/test1.in

@@ -11,14 +11,16 @@ for i in 1 2 # Comment on same line as command
 	end;
 end
 
-logmsg Bracket expansion
+logmsg Brace expansion
 echo x-{1}
 echo x-{1,2}
 echo foo-{1,2{3,4}}
 
 echo foo-{} # literal "{}" expands to itself
 echo foo-{{},{}} # the inner "{}" expand to themselves, the outer pair expands normally.
-echo foo-{""} # still expands to foo-
+echo foo-{{a},{}} # also works with something in the braces.
+echo foo-{""} # still expands to foo-{}
+echo banana # just as a marker
 echo foo-{$undefinedvar} # still expands to nothing
 
 echo foo-{,,,} # four empty items in the braces.

tests/test1.out

@@ -7,13 +7,15 @@
 2b
 
 ####################
-# Bracket expansion
+# Brace expansion
-x-1
+x-{1}
 x-1 x-2
 foo-1 foo-23 foo-24
 foo-{}
 foo-{} foo-{}
-foo-
+foo-{a} foo-{}
+foo-{}
+banana
 
 foo- foo- foo- foo-
 foo- foo-, foo-