Bring back caret redirections under a feature flag

This partially reverts 5b489ca30f, with
carets acting as redirections unless the stderr-nocaret flag is set.
This flag is off by default but may be enabled on the command line:

fish --features stderr-nocaret

CHANGELOG.md

@@ -15,8 +15,6 @@ This section is for changes merged to the `major` branch that are not also merge
 - Successive commas in brace expansions are handled in less surprising manner (`{,,,}` expands to four empty strings rather than an empty string, a comma and an empty string again). (#3002, #4632).
 - `%` is no longer used for process and job expansion. `$fish_pid` and `$last_pid` have taken the place of `%self` and `%last` respectively. (#4230, #1202)
 - The new `math` builtin (see below) does not support logical expressions; `test` should be used instead (#4777).
-- The `?` wildcard has been removed (#4520).
-- The `^` caret redirection for stderr has been removed (#4394). To redirect stderr, `2>/some/path` may be used, or `2>|` as a pipe.
 
 ## Notable fixes and improvements
 - `wait` builtin is added for waiting on processes (#4498).
@@ -56,6 +54,7 @@ This section is for changes merged to the `major` branch that are not also merge
 - Variables set in `if` and `while` conditions are available outside the block (#4820).
 - The universal variables file no longer contains the MAC address. It is now at the fixed location `.config/fish/fish_universal_variables` (#1912).
 - `alias` now has a `-s` and `--save` option to save the function generated by the alias using `funcsave` (#4878).
+- The `?` wildcard has been removed (#4520).
 
 ## Other significant changes
 - Command substitution output is now limited to 10 MB by default (#3822).

src/common.cpp

@@ -38,6 +38,7 @@
 #include "env.h"
 #include "expand.h"
 #include "fallback.h"  // IWYU pragma: keep
+#include "future_feature_flags.h"
 #include "proc.h"
 #include "wildcard.h"
 #include "wutil.h"  // IWYU pragma: keep
@@ -928,9 +929,10 @@ static bool unescape_string_var(const wchar_t *in, wcstring *out) {
 static void escape_string_script(const wchar_t *orig_in, size_t in_len, wcstring &out,
                                  escape_flags_t flags) {
     const wchar_t *in = orig_in;
-    bool escape_all = static_cast<bool>(flags & ESCAPE_ALL);
-    bool no_quoted = static_cast<bool>(flags & ESCAPE_NO_QUOTED);
-    bool no_tilde = static_cast<bool>(flags & ESCAPE_NO_TILDE);
+    const bool escape_all = static_cast<bool>(flags & ESCAPE_ALL);
+    const bool no_quoted = static_cast<bool>(flags & ESCAPE_NO_QUOTED);
+    const bool no_tilde = static_cast<bool>(flags & ESCAPE_NO_TILDE);
+    const bool no_caret = fish_features().test(features_t::stderr_nocaret);
 
     int need_escape = 0;
     int need_complex_escape = 0;
@@ -1003,10 +1005,12 @@ static void escape_string_script(const wchar_t *orig_in, size_t in_len, wcstring
                     out += L"**";
                     break;
                 }
+
                 case L'&':
                 case L'$':
                 case L' ':
                 case L'#':
+                case L'^':
                 case L'<':
                 case L'>':
                 case L'(':
@@ -1020,7 +1024,8 @@ static void escape_string_script(const wchar_t *orig_in, size_t in_len, wcstring
                 case L';':
                 case L'"':
                 case L'~': {
-                    if (!no_tilde || c != L'~') {
+                    bool char_is_normal = (c == L'~' && no_tilde) || (c == L'^' && no_caret);
+                    if (!char_is_normal) {
                         need_escape = 1;
                         if (escape_all) out += L'\\';
                     }

src/fish_tests.cpp

@@ -535,7 +535,7 @@ static void test_tokenizer() {
 
     const wchar_t *str =
         L"string <redirection  2>&1 'nested \"quoted\" '(string containing subshells "
-        L"){and,brackets}$as[$well (as variable arrays)] not_a_redirect^ 2> 2>^is_a_redirect "
+        L"){and,brackets}$as[$well (as variable arrays)] not_a_redirect^ ^ ^^is_a_redirect "
         L"&&& ||| "
         L"&& || & |"
         L"Compress_Newlines\n  \n\t\n   \nInto_Just_One";
@@ -609,6 +609,8 @@ static void test_tokenizer() {
     // Test redirection_type_for_string.
     if (redirection_type_for_string(L"<") != redirection_type_t::input)
         err(L"redirection_type_for_string failed on line %ld", (long)__LINE__);
+    if (redirection_type_for_string(L"^") != redirection_type_t::overwrite)
+        err(L"redirection_type_for_string failed on line %ld", (long)__LINE__);
     if (redirection_type_for_string(L">") != redirection_type_t::overwrite)
         err(L"redirection_type_for_string failed on line %ld", (long)__LINE__);
     if (redirection_type_for_string(L"2>") != redirection_type_t::overwrite)
@@ -625,6 +627,16 @@ static void test_tokenizer() {
         err(L"redirection_type_for_string failed on line %ld", (long)__LINE__);
     if (redirection_type_for_string(L"2>|"))
         err(L"redirection_type_for_string failed on line %ld", (long)__LINE__);
+
+    // Test ^ with our feature flag on and off.
+    auto saved_flags = fish_features();
+    mutable_fish_features().set(features_t::stderr_nocaret, false);
+    if (redirection_type_for_string(L"^") != redirection_type_t::overwrite)
+        err(L"redirection_type_for_string failed on line %ld", (long)__LINE__);
+    mutable_fish_features().set(features_t::stderr_nocaret, true);
+    if (redirection_type_for_string(L"^") != none())
+        err(L"redirection_type_for_string failed on line %ld", (long)__LINE__);
+    mutable_fish_features() = saved_flags;
 }
 
 // Little function that runs in a background thread, bouncing to the main.

src/tokenizer.cpp

@@ -13,6 +13,7 @@
 
 #include "common.h"
 #include "fallback.h"  // IWYU pragma: keep
+#include "future_feature_flags.h"
 #include "tokenizer.h"
 #include "wutil.h"  // IWYU pragma: keep
 
@@ -34,6 +35,9 @@ const wchar_t *tokenizer_error::Message() const {
     return _(_message);
 }
 
+// Whether carets redirect stderr.
+static bool caret_redirs() { return !fish_features().test(features_t::stderr_nocaret); }
+
 /// Return an error token and mark that we no longer have a next token.
 tok_t tokenizer_t::call_error(tokenizer_error *error_type, const wchar_t *token_start,
                               const wchar_t *error_loc) {
@@ -70,8 +74,10 @@ bool tokenizer_t::next(struct tok_t *result) {
     return true;
 }
 
-/// Tests if this character can be a part of a string.
-static bool tok_is_string_character(wchar_t c) {
+/// 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) {
     switch (c) {
         case L'\0':
         case L' ':
@@ -82,9 +88,15 @@ static bool tok_is_string_character(wchar_t c) {
         case L'\r':
         case L'<':
         case L'>':
-        case L'&':
+        case L'&': {
+            // Unconditional separators.
             return false;
-        default: return true;
+        }
+        case L'^': {
+            // Conditional separator.
+            return !caret_redirs() || !is_first;
+        }
+        default: { return true; }
     }
 }
 
@@ -109,6 +121,7 @@ tok_t tokenizer_t::read_string() {
     std::vector<char> expecting;
     int slice_offset = 0;
     const wchar_t *const buff_start = this->buff;
+    bool is_first = true;
 
     while (true) {
         wchar_t c = *this->buff;
@@ -209,8 +222,7 @@ tok_t tokenizer_t::read_string() {
                 }
                 break;
             }
-        }
-        else if (mode == tok_mode::regular_text && !tok_is_string_character(c)) {
+        } else if (mode == tok_mode::regular_text && !tok_is_string_character(c, is_first)) {
             break;
         }
 
@@ -224,6 +236,7 @@ tok_t tokenizer_t::read_string() {
 #endif
 
         this->buff++;
+        is_first = false;
     }
 
     if ((!this->accept_unfinished) && (mode != tok_mode::regular_text)) {
@@ -282,7 +295,7 @@ static maybe_t<parsed_redir_or_pipe_t> read_redirection_or_fd_pipe(const wchar_t
     size_t idx = 0;
 
     // Determine the fd. This may be specified as a prefix like '2>...' or it may be implicit like
-    // '>'. Try parsing out a number; if we did not get any digits then infer it from the
+    // '>' or '^'. Try parsing out a number; if we did not get any digits then infer it from the
     // first character. Watch out for overflow.
     long long big_fd = 0;
     for (; iswdigit(buff[idx]); idx++) {
@@ -303,6 +316,14 @@ static maybe_t<parsed_redir_or_pipe_t> read_redirection_or_fd_pipe(const wchar_t
                 result.fd = STDIN_FILENO;
                 break;
             }
+            case L'^': {
+                if (caret_redirs()) {
+                    result.fd = STDERR_FILENO;
+                } else {
+                    errored = true;
+                }
+                break;
+            }
             default: {
                 errored = true;
                 break;
@@ -311,11 +332,12 @@ static maybe_t<parsed_redir_or_pipe_t> read_redirection_or_fd_pipe(const wchar_t
     }
 
     // Either way we should have ended on the redirection character itself like '>'.
+    // Don't allow an fd with a caret redirection - see #1873
     wchar_t redirect_char = buff[idx++];  // note increment of idx
-    if (redirect_char == L'>') {
+    if (redirect_char == L'>' || (redirect_char == L'^' && idx == 1 && caret_redirs())) {
         result.redirection_mode = redirection_type_t::overwrite;
         if (buff[idx] == redirect_char) {
-            // Doubled up like >>. That means append.
+            // Doubled up like ^^ or >>. That means append.
             result.redirection_mode = redirection_type_t::append;
             idx++;
         }
@@ -507,7 +529,7 @@ maybe_t<tok_t> tokenizer_t::tok_next() {
             // Maybe a redirection like '2>&1', maybe a pipe like 2>|, maybe just a string.
             const wchar_t *error_location = this->buff;
             maybe_t<parsed_redir_or_pipe_t> redir_or_pipe;
-            if (iswdigit(*this->buff)) {
+            if (iswdigit(*this->buff) || (*this->buff == L'^' && caret_redirs())) {
                 redir_or_pipe = read_redirection_or_fd_pipe(this->buff);
             }
 
@@ -599,7 +621,9 @@ bool move_word_state_machine_t::consume_char_punctuation(wchar_t c) {
 }
 
 bool move_word_state_machine_t::is_path_component_character(wchar_t c) {
-    return tok_is_string_character(c) && !wcschr(L"/={,}'\"", 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) && !wcschr(L"/={,}'\"", c);
 }
 
 bool move_word_state_machine_t::consume_char_path_components(wchar_t c) {

tests/invocation/features-nocaret3.invoke

@@ -0,0 +1 @@
+--features 'no-stderr-nocaret' -c 'echo -n careton:; echo ^/dev/null'

tests/invocation/features-nocaret3.out

@@ -0,0 +1 @@
+careton:

tests/invocation/features-nocaret4.invoke

@@ -0,0 +1 @@
+--features '   stderr-nocaret' -c 'echo -n "caretoff: "; echo ^/dev/null'

tests/invocation/features-nocaret4.out

@@ -0,0 +1 @@
+caretoff: ^/dev/null