From eaecb817ca9cffd5bdf91710d0363efb9d5d4791 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 2 Feb 2020 17:51:04 -0800 Subject: [PATCH] Retain leading spaces in non-expanding braces This makes two changes: 1. Remove the 'brace_text_start' idea. The idea of 'brace_text_start' was to prevent emitting `BRACE_SPACE` at the beginning or end of an item. But we later strip these off anyways, so there is no apparent benefit. If we are not doing brace expansion, this prevented emitting whitespace at the beginning or end of an item, leading to #6564. 2. When performing brace expansion, only stomp the space character with `BRACE_SPACE`; do not stomp newlines and tabs. This is because the fix in came from a newline or tab literal, then we would have effectively replaced a newline or tab with a space, so this is important for #6564 as well. Moreover, it is not easy to place a literal newline or tab inside a brace expansion, and users who do probably do not mean for it to be stripped, so I believe this is a good change in general. Fixes #6564 --- src/common.cpp | 11 +------- tests/checks/braces.fish | 50 +++++++++++++++++++++++++++++++++++ tests/parameter_expansion.in | 6 ----- tests/parameter_expansion.out | 2 -- tests/test1.err | 3 --- tests/test1.in | 15 ----------- tests/test1.out | 14 ---------- 7 files changed, 51 insertions(+), 50 deletions(-) create mode 100644 tests/checks/braces.fish diff --git a/src/common.cpp b/src/common.cpp index 3fd2da590..f76445fc0 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -1435,7 +1435,6 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in // 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 vars_or_seps; - bool brace_text_start = false; int brace_count = 0; bool errored = false; @@ -1531,7 +1530,6 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in // assert(brace_count > 0 && "imbalanced brackets are a tokenizer error, we // shouldn't be able to get here"); brace_count--; - brace_text_start = brace_text_start && brace_count > 0; to_append_or_none = BRACE_END; if (!braces.empty()) { // If we didn't have a var or separator since the last '{', @@ -1559,17 +1557,13 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in case L',': { if (unescape_special && brace_count > 0) { to_append_or_none = BRACE_SEP; - brace_text_start = false; vars_or_seps.push_back(input_position); } break; } - case L'\n': - case L'\t': case L' ': { if (unescape_special && brace_count > 0) { - to_append_or_none = - brace_text_start ? maybe_t(BRACE_SPACE) : none(); + to_append_or_none = BRACE_SPACE; } break; } @@ -1586,9 +1580,6 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in break; } default: { - if (unescape_special && brace_count > 0) { - brace_text_start = true; - } break; } } diff --git a/tests/checks/braces.fish b/tests/checks/braces.fish new file mode 100644 index 000000000..c42fd8e76 --- /dev/null +++ b/tests/checks/braces.fish @@ -0,0 +1,50 @@ +#RUN: %fish %s + +echo x-{1} +#CHECK: x-{1} + +echo x-{1,2} +#CHECK: x-1 x-2 + +echo foo-{1,2{3,4}} +#CHECK: foo-1 foo-23 foo-24 + +echo foo-{} # literal "{}" expands to itself +#CHECK: foo-{} + +echo foo-{{},{}} # the inner "{}" expand to themselves, the outer pair expands normally. +#CHECK: foo-{} foo-{} + +echo foo-{{a},{}} # also works with something in the braces. +#CHECK: foo-{a} foo-{} + +echo foo-{""} # still expands to foo-{} +#CHECK: foo-{} + +echo foo-{$undefinedvar} # still expands to nothing +#CHECK: + +echo foo-{,,,} # four empty items in the braces. +#CHECK: foo- foo- foo- foo- + +echo foo-{,\,,} # an empty item, a "," and an empty item. +#CHECK: foo- foo-, foo- + +echo .{ foo bar }. # see 6564 +#CHECK: .{ foo bar }. + +# whitespace within entries is retained +for foo in {a, hello +wo rld } + echo \'$foo\' +end +# CHECK: 'a' +# CHECK: 'hello +# CHECK: wo rld' + +for foo in {hello +world} + echo \'$foo\' +end +#CHECK: '{hello +#CHECK: world}' diff --git a/tests/parameter_expansion.in b/tests/parameter_expansion.in index 7804adf5d..31c89a690 100644 --- a/tests/parameter_expansion.in +++ b/tests/parameter_expansion.in @@ -14,12 +14,6 @@ echo \'{ hello , world }\' for phrase in {good\,, beautiful ,morning}; echo -n "$phrase "; end | string trim; for phrase in {goodbye\,,\ cruel\ ,world\n}; echo -n $phrase; end; -# whitespace within entries converted to spaces in a single entry -for foo in {a, hello -world } - echo \'$foo\' -end - # dual expansion cartesian product echo { alpha, beta }\ {lambda, gamma }, | string replace -r ',$' '' diff --git a/tests/parameter_expansion.out b/tests/parameter_expansion.out index d541621e0..4717848c7 100644 --- a/tests/parameter_expansion.out +++ b/tests/parameter_expansion.out @@ -6,8 +6,6 @@ apple orange banana 'hello' 'world' good, beautiful morning goodbye, cruel world -'a' -'hello world' alpha lambda, beta lambda, alpha gamma, beta gamma Meg Jo diff --git a/tests/test1.err b/tests/test1.err index c425878af..ee269e5a2 100644 --- a/tests/test1.err +++ b/tests/test1.err @@ -2,9 +2,6 @@ #################### # Comments in odd places don't cause problems -#################### -# Brace expansion - #################### # Escaped newlines diff --git a/tests/test1.in b/tests/test1.in index 6657a19eb..2d1b81009 100644 --- a/tests/test1.in +++ b/tests/test1.in @@ -11,21 +11,6 @@ for i in 1 2 # Comment on same line as command end; end -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-{{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. -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 5868a7a02..7e01503c8 100644 --- a/tests/test1.out +++ b/tests/test1.out @@ -6,20 +6,6 @@ 2a 2b -#################### -# Brace expansion -x-{1} -x-1 x-2 -foo-1 foo-23 foo-24 -foo-{} -foo-{} foo-{} -foo-{a} foo-{} -foo-{} -banana - -foo- foo- foo- foo- -foo- foo-, foo- - #################### # Escaped newlines foo bar