From cc32b4f2a78ad1d3ecfab6c6c1250a3d09857a3d Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Tue, 29 Jun 2021 19:30:27 +0200 Subject: [PATCH] Make '&' only background if followed by a separating character This is opt-in through a new feature flag "ampersand-nobg-in-token". When this flag and "qmark-noglob" are enabled, this command no longer needs quoting: curl https://example.com/thing?foo=bar&duran=duran Compared to the previous approach e1570a4 ("Let '&' only separate as the first char of a word"), this has some advantages: 1. "&&" and "&>" are no longer affected. They are still special, even if used between tokens without spaces, like "echo bar&>foo". Maybe this is not really *better*, but it avoids risking to annoy users by breaking the old variant. 2. "&" is still special if at the end of a token, like in "sleep 1&". Word movement is not affected by the semantics change, so Alt-F and friends still stop at every "&". --- CHANGELOG.rst | 3 ++- doc_src/language.rst | 10 +++++++--- src/builtin_status.cpp | 8 ++++++-- src/fish_tests.cpp | 13 +++++++++++++ src/future_feature_flags.cpp | 2 ++ src/future_feature_flags.h | 3 +++ src/tokenizer.cpp | 16 +++++++++++----- .../features-ampersand-nobg-in-token1.fish | 12 ++++++++++++ tests/checks/status.fish | 7 ++++--- 9 files changed, 60 insertions(+), 14 deletions(-) create mode 100644 tests/checks/features-ampersand-nobg-in-token1.fish diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f7bcee017..e8e6d9adf 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -4,10 +4,11 @@ fish 3.4.0 (released ???) Notable improvements and fixes ------------------------------ - Complimenting the ``prompt`` command in 3.3.0, ``fish_config`` gained a ``theme`` subcommand to show and pick from the sample themes (meaning color schemes) directly in the terminal, instead of having to open a webbrowser. For example ``fish_config theme choose Nord`` loads the Nord theme in the current session (:issue:`8132`). The current theme can be saved with ``fish_config theme dump`` and custom themes can be added by saving them in ``~/.config/fish/themes/``. -- fish's command substitution syntax has been extended: ``$(cmd)`` now has the same meaning as ``(cmd)`` but it can be used inside double quotes, to prevent line splitting of the results. (:issue:`159`). +- fish's command substitution syntax has been extended: ``$(cmd)`` now has the same meaning as ``(cmd)`` but it can be used inside double quotes, to prevent line splitting of the results (:issue:`159`). Deprecations and removed features --------------------------------- +- A new feature flag ``ampersand-nobg-in-token`` makes ``&`` only act as background operator if followed by a separator. In combination with ``qmark-noglob`` this allows to write some URLs without quoting or escaping (:issue:`7991`) Scripting improvements ---------------------- diff --git a/doc_src/language.rst b/doc_src/language.rst index c486b77cf..5c51edbfa 100644 --- a/doc_src/language.rst +++ b/doc_src/language.rst @@ -233,6 +233,8 @@ These listed jobs can be removed with the :ref:`disown ` command. At the moment, functions cannot be started in the background. Functions that are stopped and then restarted in the background using the :ref:`bg ` command will not execute correctly. +If the ``&`` character is followed by a non-separating character, it is not interpreted as background operator. Separating characters are whitespace and the characters ``;<>&|``. + .. _syntax-function: Functions @@ -1393,14 +1395,16 @@ Feature flags are how fish stages changes that might break scripts. Breaking cha You can see the current list of features via ``status features``:: > status features - stderr-nocaret on 3.0 ^ no longer redirects stderr - qmark-noglob off 3.0 ? no longer globs - regex-easyesc off 3.1 string replace -r needs fewer \\'s + stderr-nocaret on 3.0 ^ no longer redirects stderr + qmark-noglob off 3.0 ? no longer globs + regex-easyesc off 3.1 string replace -r needs fewer \\'s + ampersand-nobg-in-token off 3.4 & only backgrounds if followed by a separating character There are two breaking changes in fish 3.0: caret ``^`` no longer redirects stderr, and question mark ``?`` is no longer a glob. There is one breaking change in fish 3.1: ``string replace -r`` does a superfluous round of escaping for the replacement, so escaping backslashes would look like ``string replace -ra '([ab])' '\\\\\\\$1' a``. This flag removes that if turned on, so ``'\\\\$1'`` is enough. +There is one breaking change in fish 3.4: in ``echo https://example.com/?q=hello&qq=goodbye`` the ``&`` is no longer interpreted as backgrounding operator. These changes are off by default. They can be enabled on a per session basis:: diff --git a/src/builtin_status.cpp b/src/builtin_status.cpp index db87ef454..62693143b 100644 --- a/src/builtin_status.cpp +++ b/src/builtin_status.cpp @@ -148,10 +148,14 @@ static bool set_status_cmd(const wchar_t *cmd, status_cmd_opts_t &opts, status_c /// Print the features and their values. static void print_features(io_streams_t &streams) { + size_t max_len = std::numeric_limits::min(); + for (const auto &md : features_t::metadata) { + max_len = std::max(max_len, wcslen(md.name)); + } for (const auto &md : features_t::metadata) { int set = feature_test(md.flag); - streams.out.append_format(L"%ls\t%s\t%ls\t%ls\n", md.name, set ? "on" : "off", md.groups, - md.description); + streams.out.append_format(L"%-*ls%-3s %ls %ls\n", max_len + 1, md.name, set ? "on" : "off", + md.groups, md.description); } } diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index cf7f3da32..331a5c2f8 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2773,6 +2773,7 @@ static void test_word_motion() { test_1_word_motion(word_motion_right, move_word_style_punctuation, L"^a^ bcd^"); test_1_word_motion(word_motion_right, move_word_style_punctuation, L"a^b^ cde^"); test_1_word_motion(word_motion_right, move_word_style_punctuation, L"^ab^ cde^"); + test_1_word_motion(word_motion_right, move_word_style_punctuation, L"^ab^&cd^ ^& ^e^ f^&"); test_1_word_motion(word_motion_right, move_word_style_whitespace, L"^^a-b-c^ d-e-f"); test_1_word_motion(word_motion_right, move_word_style_whitespace, L"^a-b-c^\n d-e-f^ "); @@ -5196,6 +5197,15 @@ static void test_highlighting() { {L"&", highlight_role_t::statement_terminator}, }); + highlight_tests.push_back({ + {L"echo", highlight_role_t::command}, + {L"foo&bar", highlight_role_t::param}, + {L"foo", highlight_role_t::param, /*nospace=*/true}, + {L"&", highlight_role_t::statement_terminator}, + {L"echo", highlight_role_t::command}, + {L"&>", highlight_role_t::redirection}, + }); + highlight_tests.push_back({ {L"if command", highlight_role_t::keyword}, {L"ls", highlight_role_t::command}, @@ -5489,6 +5499,8 @@ static void test_highlighting() { highlight_tests.push_back({{L"$EMPTY_VARIABLE", highlight_role_t::error}}); highlight_tests.push_back({{L"\"$EMPTY_VARIABLE\"", highlight_role_t::error}}); + const auto saved_flags = fish_features(); + mutable_fish_features().set(features_t::ampersand_nobg_in_token, true); for (const highlight_component_list_t &components : highlight_tests) { // Generate the text. wcstring text; @@ -5523,6 +5535,7 @@ static void test_highlighting() { } } } + mutable_fish_features() = saved_flags; vars.remove(L"VARIABLE_IN_COMMAND", ENV_DEFAULT); vars.remove(L"VARIABLE_IN_COMMAND2", ENV_DEFAULT); } diff --git a/src/future_feature_flags.cpp b/src/future_feature_flags.cpp index b539a9a93..15a5b6b92 100644 --- a/src/future_feature_flags.cpp +++ b/src/future_feature_flags.cpp @@ -20,6 +20,8 @@ const features_t::metadata_t features_t::metadata[features_t::flag_count] = { {qmark_noglob, L"qmark-noglob", L"3.0", L"? no longer globs", false}, {string_replace_backslash, L"regex-easyesc", L"3.1", L"string replace -r needs fewer \\'s", false}, + {ampersand_nobg_in_token, L"ampersand-nobg-in-token", L"3.4", + L"& only backgrounds if followed by a separating character", false}, }; const struct features_t::metadata_t *features_t::metadata_for(const wchar_t *name) { diff --git a/src/future_feature_flags.h b/src/future_feature_flags.h index 0ed5d5d95..9ddb5efc4 100644 --- a/src/future_feature_flags.h +++ b/src/future_feature_flags.h @@ -22,6 +22,9 @@ class features_t { /// Whether string replace -r double-unescapes the replacement. string_replace_backslash, + /// Whether "&" is not-special if followed by a word character. + ampersand_nobg_in_token, + /// The number of flags. flag_count }; diff --git a/src/tokenizer.cpp b/src/tokenizer.cpp index d9c179d6f..e43065714 100644 --- a/src/tokenizer.cpp +++ b/src/tokenizer.cpp @@ -98,7 +98,7 @@ tok_t::tok_t(token_type_t type) : type(type) {} /// Tests if this character can be a part of a string. The redirect ^ is allowed unless it's the /// first character. Hash (#) starts a comment if it's the first character in a token; otherwise it /// is considered a string character. See issue #953. -static bool tok_is_string_character(wchar_t c, bool is_first) { +static bool tok_is_string_character(wchar_t c, bool is_first, maybe_t next) { switch (c) { case L'\0': case L' ': @@ -108,11 +108,16 @@ static bool tok_is_string_character(wchar_t c, bool is_first) { case L';': case L'\r': case L'<': - case L'>': - case L'&': { + case L'>': { // Unconditional separators. return false; } + case L'&': { + if (!feature_test(features_t::ampersand_nobg_in_token)) return false; + bool next_is_string = next && tok_is_string_character(*next, false, none()); + // Unlike in other shells, '&' is not special if followed by a string character. + return next_is_string; + } case L'^': { // Conditional separator. return !caret_redirs() || !is_first; @@ -259,7 +264,8 @@ tok_t tokenizer_t::read_string() { } break; } - } else if (mode == tok_modes::regular_text && !tok_is_string_character(c, is_first)) { + } else if (mode == tok_modes::regular_text && + !tok_is_string_character(c, is_first, this->token_cursor[1])) { break; } @@ -764,7 +770,7 @@ bool move_word_state_machine_t::is_path_component_character(wchar_t c) { // Always treat separators as first. All this does is ensure that we treat ^ as a string // character instead of as stderr redirection, which I hypothesize is usually what is // desired. - return tok_is_string_character(c, true) && !std::wcschr(L"/={,}'\":@", c); + return tok_is_string_character(c, true, none()) && !std::wcschr(L"/={,}'\":@", c); } bool move_word_state_machine_t::consume_char_path_components(wchar_t c) { diff --git a/tests/checks/features-ampersand-nobg-in-token1.fish b/tests/checks/features-ampersand-nobg-in-token1.fish new file mode 100644 index 000000000..f5f929beb --- /dev/null +++ b/tests/checks/features-ampersand-nobg-in-token1.fish @@ -0,0 +1,12 @@ +#RUN: %fish --features=ampersand-nobg-in-token %s + +echo no&background +# CHECK: no&background + +echo background& +# CHECK: background + +echo background & +# CHECK: background + +wait diff --git a/tests/checks/status.fish b/tests/checks/status.fish index ccee6e764..ffe383e77 100644 --- a/tests/checks/status.fish +++ b/tests/checks/status.fish @@ -54,9 +54,10 @@ eval test_function # Future Feature Flags status features -#CHECK: stderr-nocaret on 3.0 ^ no longer redirects stderr -#CHECK: qmark-noglob off 3.0 ? no longer globs -#CHECK: regex-easyesc off 3.1 string replace -r needs fewer \'s +#CHECK: stderr-nocaret on 3.0 ^ no longer redirects stderr +#CHECK: qmark-noglob off 3.0 ? no longer globs +#CHECK: regex-easyesc off 3.1 string replace -r needs fewer \'s +#CHECK: ampersand-nobg-in-token off 3.4 & only backgrounds if followed by a separating character status test-feature stderr-nocaret echo $status #CHECK: 0