diff --git a/src/expand.cpp b/src/expand.cpp index e76227e10..3cb203e6e 100644 --- a/src/expand.cpp +++ b/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) { diff --git a/src/expand.h b/src/expand.h index 221bffec9..678afd1af 100644 --- a/src/expand.h +++ b/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 diff --git a/src/highlight.cpp b/src/highlight.cpp index a15fb732d..177bfffb1 100644 --- a/src/highlight.cpp +++ b/src/highlight.cpp @@ -347,7 +347,7 @@ static bool plain_statement_get_expanded_command(const wcstring &src, maybe_t 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) { diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 0a1a0aa64..5f0de0765 100644 --- a/src/parse_execution.cpp +++ b/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 statement) { - eval_result_t result = eval_result_t::ok; // Get the switch variable. tnode_t 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_tempty()) { @@ -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; } diff --git a/tests/checks/wildcard.fish b/tests/checks/wildcard.fish new file mode 100644 index 000000000..2758d6b2e --- /dev/null +++ b/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