Correct behavior of string match and unmatched capture groups

string match is documented as setting an unset variable if a capture group
is unmatched in an otherwise matched regex, and if the `--all` flag is not
provided. However prior to this fix, it instead set a variable containing
the empty string as a single value. Correct the implementation to match
the documentation.

Note that if the `--all` flag is provided we continue to set empty
strings, which is documented.

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
 -------------------------

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 &regex_;
+        const bool all_flag_;
         bool regex_valid_ = false;
 
        public:
-        regex_importer_t(parser_t &parser, const wcstring &haystack, const compiled_regex_t &regex)
-            : parser_(parser), haystack_(haystack), regex_(regex) {}
+        regex_importer_t(parser_t &parser, const wcstring &haystack, const compiled_regex_t &regex,
+                         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

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 -- '^(?<foo>bar)$'
+echo foo | string match -rq -- '^(?<foo>bar)$'
 echo $foo
 # CHECK:
 
@@ -55,6 +55,21 @@ string match -rq '(?<bee>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 '(?<nums>\d+)|(?<text>[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 '(?<nums>\d+)|(?<text>[a-z]+)' -- '111 aaa 222 bbb'
 # CHECK: 111
 # CHECK: 111