Eliminate expand_result_t::wildcard_match

This was an internal implementation detail that all callers had to deal
with. Just get rid of it.

src/expand.cpp

@@ -1007,8 +1007,7 @@ expand_result_t expander_t::stage_wildcards(wcstring path_to_expand, completion_
                 path_to_expand, effective_working_dir, flags, ctx.cancel_checker, &expanded);
             switch (expand_res) {
                 case wildcard_expand_result_t::match:
-                    // Something matched,so overall we matched.
-                    result = expand_result_t::wildcard_match;
+                    result = expand_result_t::ok;
                     break;
                 case wildcard_expand_result_t::no_match:
                     break;
@@ -1058,12 +1057,7 @@ expand_result_t expander_t::expand_string(wcstring input, completion_list_t *out
         for (completion_t &comp : completions) {
             expand_result_t this_result =
                 (expand.*stage)(std::move(comp.completion), &output_storage);
-            // If this_result was no match, but total_result is that we have a match, then don't
-            // change it.
-            if (!(this_result == expand_result_t::wildcard_no_match &&
-                  total_result == expand_result_t::wildcard_match)) {
-                total_result = this_result;
-            }
+            total_result = this_result;
             if (total_result == expand_result_t::error) {
                 break;
             }
@@ -1077,6 +1071,16 @@ expand_result_t expander_t::expand_string(wcstring input, completion_list_t *out
         }
     }
 
+    // This is a little tricky: if one wildcard failed to match but we still got output, it
+    // means that a previous expansion resulted in multiple strings. For example:
+    //   set dirs ./a ./b
+    //   echo $dirs/*.txt
+    // Here if ./a/*.txt matches and ./b/*.txt does not, then we don't want to report a failed
+    // wildcard. So swallow failed-wildcard errors if we got any output.
+    if (total_result == expand_result_t::wildcard_no_match && !completions.empty()) {
+        total_result = expand_result_t::ok;
+    }
+
     if (total_result != expand_result_t::error) {
         // Hack to un-expand tildes (see #647).
         if (!(flags & expand_flag::skip_home_directories)) {
@@ -1127,7 +1131,7 @@ expand_result_t expand_to_command_and_args(const wcstring &instr, const operatio
         instr, &completions,
         {expand_flag::skip_cmdsubst, expand_flag::no_descriptions, expand_flag::skip_jobs}, ctx,
         errors);
-    if (expand_err == expand_result_t::ok || expand_err == expand_result_t::wildcard_match) {
+    if (expand_err == expand_result_t::ok) {
         // The first completion is the command, any remaning are arguments.
         bool first = true;
         for (auto &comp : completions) {

src/expand.h

@@ -107,8 +107,6 @@ enum class expand_result_t {
     ok,
     /// Ok, a wildcard in the string matched no files.
     wildcard_no_match,
-    /// Ok, a wildcard in the string matched a file.
-    wildcard_match,
 };
 
 /// The string represented by PROCESS_EXPAND_SELF

src/highlight.cpp

@@ -347,7 +347,7 @@ static bool plain_statement_get_expanded_command(const wcstring &src,
     maybe_t<wcstring> cmd = command_for_plain_statement(stmt, src);
     if (!cmd) return false;
     expand_result_t err = expand_to_command_and_args(*cmd, ctx, out_cmd, nullptr);
-    return err == expand_result_t::ok || err == expand_result_t::wildcard_match;
+    return err == expand_result_t::ok;
 }
 
 rgb_color_t highlight_get_color(const highlight_spec_t &highlight, bool is_background) {

src/parse_execution.cpp

@@ -449,7 +449,6 @@ eval_result_t parse_execution_context_t::run_for_statement(
 
 eval_result_t parse_execution_context_t::run_switch_statement(
     tnode_t<grammar::switch_statement> statement) {
-    eval_result_t result = eval_result_t::ok;
 
     // Get the switch variable.
     tnode_t<grammar::argument> switch_value_n = statement.child<1>();
@@ -463,37 +462,29 @@ eval_result_t parse_execution_context_t::run_switch_statement(
     parse_error_offset_source_start(&errors, switch_value_n.source_range()->start);
 
     switch (expand_ret) {
-        case expand_result_t::error: {
-            result = report_errors(errors);
+        case expand_result_t::error:
+            return report_errors(errors);
+
+        case expand_result_t::wildcard_no_match:
+            return report_unmatched_wildcard_error(switch_value_n);
+
+        case expand_result_t::ok:
+            if (switch_values_expanded.size() > 1) {
+                return report_error(switch_value_n,
+                                    _(L"switch: Expected at most one argument, got %lu\n"),
+                                    switch_values_expanded.size());
+            }
             break;
-        }
-        case expand_result_t::wildcard_no_match: {
-            result = report_unmatched_wildcard_error(switch_value_n);
-            break;
-        }
-        case expand_result_t::wildcard_match:
-        case expand_result_t::ok: {
-            break;
-        }
-        default: {
-            DIE("unexpected expand_string() return value");
-            break;
-        }
     }
 
-    if (result == eval_result_t::ok && switch_values_expanded.size() > 1) {
-        result =
-            report_error(switch_value_n, _(L"switch: Expected at most one argument, got %lu\n"),
-                         switch_values_expanded.size());
+    // If we expanded to nothing, match the empty string.
+    assert(switch_values_expanded.size() <= 1 && "Should have at most one expansion");
+    wcstring switch_value_expanded = L"";
+    if (!switch_values_expanded.empty()) {
+        switch_value_expanded = std::move(switch_values_expanded.front().completion);
     }
 
-    if (result != eval_result_t::ok) {
-        return result;
-    }
-
-    const wcstring &switch_value_expanded =
-        switch_values_expanded.size() == 1 ? switch_values_expanded.at(0).completion : L"";
-
+    eval_result_t result = eval_result_t::ok;
     block_t *sb = parser->push_block(block_t::switch_block());
 
     // Expand case statements.
@@ -731,7 +722,7 @@ eval_result_t parse_execution_context_t::expand_command(tnode_t<grammar::plain_s
     } else if (expand_err == expand_result_t::wildcard_no_match) {
         return report_unmatched_wildcard_error(statement);
     }
-    assert(expand_err == expand_result_t::ok || expand_err == expand_result_t::wildcard_match);
+    assert(expand_err == expand_result_t::ok);
 
     // Complain if the resulting expansion was empty, or expanded to an empty string.
     if (out_cmd->empty()) {
@@ -891,7 +882,6 @@ eval_result_t parse_execution_context_t::expand_arguments_from_nodes(
                 }
                 break;
             }
-            case expand_result_t::wildcard_match:
             case expand_result_t::ok: {
                 break;
             }
@@ -1034,7 +1024,6 @@ eval_result_t parse_execution_context_t::apply_variable_assignments(
                 return eval_result_t::error;
             }
             case expand_result_t::wildcard_no_match:  // nullglob (equivalent to set)
-            case expand_result_t::wildcard_match:
             case expand_result_t::ok: {
                 break;
             }

tests/checks/wildcard.fish

@@ -0,0 +1,15 @@
+# RUN: %fish %s
+
+# Ensure that, if variable expansion results in multiple strings
+# and one of them fails a glob, that we don't fail the entire expansion.
+set dir (mktemp -d)
+cd $dir
+mkdir a
+mkdir b
+touch ./b/file.txt
+
+set dirs ./a ./b
+echo $dirs/*.txt
+# CHECK: ./b/file.txt
+
+rm -Rf $dir