From aa58cae6014582e19a6b52db8d0865ae81771b3b Mon Sep 17 00:00:00 2001
From: Fabian Homborg <FHomborg@gmail.com>
Date: Tue, 2 Jan 2018 15:24:48 +0100
Subject: [PATCH] Don't count successive "," as literal in brace expansion

This was highly surprising.

Fixes #3002.
---
 CHANGELOG.md         |  1 +
 doc_src/index.hdr.in | 11 +++++++++++
 src/common.cpp       |  4 +---
 tests/test1.in       |  4 ++++
 tests/test1.out      |  2 ++
 5 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8a28a887e..460fd62e5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@ This section is for changes merged to the `major` branch that are not also merge
 - `set x[1] x[2] a b` is no longer valid syntax (#4236).
 - For loop control variables are no longer local to the for block (#1935).
 - A literal `{}` now expands to itself, rather than nothing. This makes working with `find -exec` easier. (#1109, #4632)
+- Successive commas in brace expansions are handled in less surprising manner (`{,,,}` expands to four empty strings rather than an empty string, a comma and an empty string again). (#3002, #4632).
 
 ## Notable fixes and improvements
 - `wait` builtin is added for waiting on processes (#4498).
diff --git a/doc_src/index.hdr.in b/doc_src/index.hdr.in
index 5a398cc00..cf11ccba2 100644
--- a/doc_src/index.hdr.in
+++ b/doc_src/index.hdr.in
@@ -514,6 +514,17 @@ echo foo-{}
 
 echo foo-{$undefinedvar}
 # Output is an empty line - see <a href="#cartesian-product">the cartesian product section</a>
+\endfish
+
+If there is nothing between a brace and a comma or two commas, it's interpreted as an empty element.
+
+So:
+\fish
+echo {,,/usr}/bin
+# Output /bin /bin /usr/bin
+\endfish
+
+To use a "," as an element, <a href="#quotes">quote</a> or <a href="#escapes">escape</a> it.
 
 \subsection expand-variable Variable expansion
 
diff --git a/src/common.cpp b/src/common.cpp
index 4c225c0fc..4896c2d7a 100644
--- a/src/common.cpp
+++ b/src/common.cpp
@@ -1354,9 +1354,7 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in
                     break;
                 }
                 case L',': {
-                    // If the last character was a separator, then treat this as a literal comma.
-                    if (unescape_special && bracket_count > 0 &&
-                        string_last_char(result) != BRACKET_SEP) {
+                    if (unescape_special && bracket_count > 0) {
                         to_append_or_none = BRACKET_SEP;
                     }
                     break;
diff --git a/tests/test1.in b/tests/test1.in
index 95d3ea8be..980b3d889 100644
--- a/tests/test1.in
+++ b/tests/test1.in
@@ -15,11 +15,15 @@ logmsg Bracket 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-{$undefinedvar} # still expands to nothing
 
+echo foo-{,,,} # four empty items in the braces.
+echo foo-{,\,,} # an empty item, a "," and an empty item.
+
 logmsg Escaped newlines
 echo foo\ bar
 echo foo\
diff --git a/tests/test1.out b/tests/test1.out
index 14b22a690..ee7dcfcda 100644
--- a/tests/test1.out
+++ b/tests/test1.out
@@ -14,8 +14,10 @@ foo-1 foo-23 foo-24
 foo-{}
 foo-{} foo-{}
 foo-
+foo- foo- foo- foo-
 
 foo- foo- foo- foo-
+foo- foo-, foo-
 
 ####################
 # Escaped newlines