diff --git a/src/common.cpp b/src/common.cpp index e4e284848..67f60f502 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -1390,8 +1390,12 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in } case L'$': { if (unescape_special) { - to_append_or_none = VARIABLE_EXPAND; - vars_or_seps.push_back(input_position); + bool is_cmdsub = + input_position + 1 < input_len && input[input_position + 1] == L'('; + if (!is_cmdsub) { + to_append_or_none = VARIABLE_EXPAND; + vars_or_seps.push_back(input_position); + } } break; } diff --git a/src/expand.cpp b/src/expand.cpp index b33f4131e..17bcad32f 100644 --- a/src/expand.cpp +++ b/src/expand.cpp @@ -761,7 +761,7 @@ static expand_result_t expand_cmdsubst(wcstring input, const operation_context_t wcstring whole_item; whole_item.reserve(paren_begin + 1 + sub_item2.size() + 1 + tail_item.completion.size()); - whole_item.append(input, 0, paren_begin); + whole_item.append(input, 0, paren_begin - have_dollar); whole_item.push_back(INTERNAL_SEPARATOR); whole_item.append(sub_item2); whole_item.push_back(INTERNAL_SEPARATOR); diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 805509a2f..f26f8abde 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -4654,6 +4654,8 @@ void history_tests_t::test_history_formats() { // We don't expect whitespace to be elided (#4908: except for leading/trailing whitespace) const wchar_t *expected[] = {L"EOF", L"sleep 123", + L"posix_cmd_sub $(is supported but only splits on newlines)", + L"posix_cmd_sub \"$(is supported)\"", L"a && echo valid construct", L"final line", L"echo supsup", @@ -5102,8 +5104,7 @@ static void test_error_messages() { {L"echo $", ERROR_NO_VAR_NAME}, {L"echo foo\"$\"bar", ERROR_NO_VAR_NAME}, {L"echo \"foo\"$\"bar\"", ERROR_NO_VAR_NAME}, - {L"echo foo $ bar", ERROR_NO_VAR_NAME}, - {L"echo foo$(foo)bar", ERROR_BAD_VAR_SUBCOMMAND1}}; + {L"echo foo $ bar", ERROR_NO_VAR_NAME}}; parse_error_list_t errors; for (const auto &test : error_tests) { @@ -5193,6 +5194,12 @@ static void test_highlighting() { {L"|", highlight_role_t::statement_terminator}, {L"cat", highlight_role_t::command}, }); + highlight_tests.push_back({ + {L"true", highlight_role_t::command}, + {L"$(", highlight_role_t::operat}, + {L"true", highlight_role_t::command}, + {L")", highlight_role_t::operat}, + }); highlight_tests.push_back({ {L"true", highlight_role_t::command}, {L"\"before", highlight_role_t::quote}, diff --git a/src/parse_constants.h b/src/parse_constants.h index e0e4c52f9..65031e8f1 100644 --- a/src/parse_constants.h +++ b/src/parse_constants.h @@ -262,9 +262,6 @@ enum class pipeline_position_t { /// Error issued on $@. #define ERROR_NOT_ARGV_AT _(L"$@ is not supported. In fish, please use $argv.") -/// Error issued on $(...). -#define ERROR_BAD_VAR_SUBCOMMAND1 _(L"$(...) is not supported. In fish, please use '(%ls)'.") - /// Error issued on $*. #define ERROR_NOT_ARGV_STAR _(L"$* is not supported. In fish, please use $argv.") diff --git a/src/parse_util.cpp b/src/parse_util.cpp index 827ad79a7..692ccc8bc 100644 --- a/src/parse_util.cpp +++ b/src/parse_util.cpp @@ -915,26 +915,6 @@ void parse_util_expand_variable_error(const wcstring &token, size_t global_token append_syntax_error(errors, global_dollar_pos, ERROR_NO_VAR_NAME); break; } - case '(': { - // e.g.: 'echo "foo$(bar)baz" - // Try to determine what's in the parens. - wcstring token_after_parens; - wcstring paren_text; - size_t open_parens = dollar_pos + 1, cmdsub_start = 0, cmdsub_end = 0; - if (parse_util_locate_cmdsubst_range(token, &open_parens, &paren_text, &cmdsub_start, - &cmdsub_end, true) > 0) { - token_after_parens = tok_first(paren_text); - } - - // Make sure we always show something. - if (token_after_parens.empty()) { - token_after_parens = get_ellipsis_str(); - } - - append_syntax_error(errors, global_dollar_pos, ERROR_BAD_VAR_SUBCOMMAND1, - truncate(token_after_parens, var_err_len).c_str()); - break; - } case L'\0': { append_syntax_error(errors, global_dollar_pos, ERROR_NO_VAR_NAME); break; @@ -960,39 +940,6 @@ void parse_util_expand_variable_error(const wcstring &token, size_t global_token assert(errors->size() == start_error_count + 1); } -/// Detect cases like $(abc). Given an arg like foo(bar), let arg_src be foo and cmdsubst_src be -/// bar. If arg ends with VARIABLE_EXPAND, then report an error. -static parser_test_error_bits_t detect_dollar_cmdsub_errors(size_t arg_src_offset, - const wcstring &arg_src, - const wcstring &cmdsubst_src, - parse_error_list_t *out_errors) { - parser_test_error_bits_t result_bits = 0; - wcstring unescaped_arg_src; - - if (!unescape_string(arg_src, &unescaped_arg_src, UNESCAPE_SPECIAL) || - unescaped_arg_src.empty()) { - return result_bits; - } - - wchar_t last = unescaped_arg_src.at(unescaped_arg_src.size() - 1); - if (last == VARIABLE_EXPAND) { - result_bits |= PARSER_TEST_ERROR; - if (out_errors != nullptr) { - wcstring subcommand_first_token = tok_first(cmdsubst_src); - if (subcommand_first_token.empty()) { - // e.g. $(). Report somthing. - subcommand_first_token = get_ellipsis_str(); - } - append_syntax_error( - out_errors, - arg_src_offset + arg_src.size() - 1, // global position of the dollar - ERROR_BAD_VAR_SUBCOMMAND1, truncate(subcommand_first_token, var_err_len).c_str()); - } - } - - return result_bits; -} - /// Test if this argument contains any errors. Detected errors include syntax errors in command /// substitutions, improperly escaped characters and improper use of the variable expansion /// operator. @@ -1038,15 +985,6 @@ parser_test_error_bits_t parse_util_detect_errors_in_argument(const ast::argumen if (out_errors != nullptr) { out_errors->insert(out_errors->end(), subst_errors.begin(), subst_errors.end()); - - // Hackish. Take this opportunity to report $(...) errors. We do this because - // after we've replaced with internal separators, we can't distinguish between - // "" and (), and also we no longer have the source of the command substitution. - // As an optimization, this is only necessary if the last character is a $. - if (paren_begin > 0 && arg_src.at(paren_begin - 1) == L'$') { - err |= detect_dollar_cmdsub_errors( - source_start, arg_src.substr(0, paren_begin), subst, out_errors); - } } break; } diff --git a/tests/checks/cmdsub.fish b/tests/checks/cmdsub.fish index bfc9f60af..9c833e2b2 100644 --- a/tests/checks/cmdsub.fish +++ b/tests/checks/cmdsub.fish @@ -1,5 +1,8 @@ #RUN: %fish %s +echo $(echo 1\n2) +# CHECK: 1 2 + # Command substitution inside double quotes strips trailing newline. echo "a$(echo b)c" # CHECK: abc diff --git a/tests/history_sample_bash b/tests/history_sample_bash index 3094c9a39..8ecd55b63 100644 --- a/tests/history_sample_bash +++ b/tests/history_sample_bash @@ -15,7 +15,8 @@ backticks `are not allowed` a && echo valid construct [[ x = y ]] && echo double brackets not allowed (( 1 = 2 )) && echo double parens not allowed -posix_cmd_sub $(is not supported) +posix_cmd_sub "$(is supported)" +posix_cmd_sub $(is supported but only splits on newlines) sleep 123 /** # see issue 7407 jq <<"EOF"