diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1c0656349..95a8b210f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -20,6 +20,7 @@ Scripting improvements - ``echo`` no longer writes its output one byte at a time, improving performance and allowing use with linux' special API files (``/proc``, ``/sys`` and such) (:issue:`7836`). - fish should now better handle ``cd`` on filesystems with broken ``stat(3)`` responses (:issue:`7577`). - Builtins now properly report a ``$status`` of 1 upon unsuccessful writes (:issue:`7857`). +- `string match` with unmatched capture groups and without the `--all` flag now sets an empty variable instead of a variable containing the empty string, matching the documentation. Interactive improvements ------------------------- diff --git a/src/builtin_string.cpp b/src/builtin_string.cpp index 02ce71402..4e15b8aec 100644 --- a/src/builtin_string.cpp +++ b/src/builtin_string.cpp @@ -897,11 +897,13 @@ class pcre2_matcher_t final : public string_matcher_t { parser_t &parser_; const wcstring &haystack_; const compiled_regex_t ®ex_; + const bool all_flag_; bool regex_valid_ = false; public: - regex_importer_t(parser_t &parser, const wcstring &haystack, const compiled_regex_t ®ex) - : parser_(parser), haystack_(haystack), regex_(regex) {} + regex_importer_t(parser_t &parser, const wcstring &haystack, const compiled_regex_t ®ex, + bool all_flag) + : parser_(parser), haystack_(haystack), regex_(regex), all_flag_(all_flag) {} /// Enumerates the named groups in the compiled PCRE2 expression, validates the names of /// the groups as variable names, and initializes their value (overriding any previous @@ -997,7 +999,7 @@ class pcre2_matcher_t final : public string_matcher_t { // didn't match but its brethren did, we need to make sure to put *something* in the // resulting array, and unfortunately fish doesn't support empty/null members so // we're going to have to use an empty string as the sentinel value. - if (!value_found) { + if (!value_found && all_flag_) { vals.push_back(wcstring{}); } } @@ -1035,7 +1037,7 @@ class pcre2_matcher_t final : public string_matcher_t { return false; } - regex_importer_t var_importer(this->parser, arg, this->regex); + regex_importer_t var_importer(this->parser, arg, this->regex, opts.all); // We must manually init the importer rather than relegating this to the constructor // because it will validate the names it is importing to make sure they're all legal and diff --git a/tests/checks/regex-import.fish b/tests/checks/regex-import.fish index 4441e0c11..527951fa4 100644 --- a/tests/checks/regex-import.fish +++ b/tests/checks/regex-import.fish @@ -20,7 +20,7 @@ printf "%s\n" $word1 $word2 # Clear variables on no match set foo foo -echo "foo" | string match -rq -- '^(?bar)$' +echo foo | string match -rq -- '^(?bar)$' echo $foo # CHECK: @@ -55,6 +55,21 @@ string match -rq '(?b.*)' -- aaa ba ccc be echo $bee # CHECK: ba +# Verify the following regarding capture groups which are not matched: +# 1. Set no values if --all is not provided +# 2. Set an empty string value if --all is provided +set -e nums +set -e text + +string match -r '(?\d+)|(?[a-z]+)' -- xyz +# CHECK: xyz +# CHECK: xyz +set --show text +# CHECK: $text: set in global scope, unexported, with 1 elements +# CHECK: $text[1]: |xyz| +set --show nums +# CHECK: $nums: set in global scope, unexported, with 0 elements + string match -r --all '(?\d+)|(?[a-z]+)' -- '111 aaa 222 bbb' # CHECK: 111 # CHECK: 111