From 5b489ca30f3205b03b23dff6bedbc59df499e92b Mon Sep 17 00:00:00 2001 From: Peter Ammon Date: Sun, 1 Apr 2018 13:43:05 -0700 Subject: [PATCH] Remove caret redirection This removes the caret as a shorthand for redirecting stderr. Note that stderr may be redirected to a file via 2>/some/path... and may be redirected with a pipe via 2>|. Fixes #4394 --- CHANGELOG.md | 3 ++- src/common.cpp | 1 - src/fish_tests.cpp | 4 +--- src/tokenizer.cpp | 38 ++++++++++---------------------------- tests/gen_output.fish | 10 +++++----- tests/interactive.fish | 7 ++++--- tests/jobs.in | 2 +- tests/printf.in | 2 +- tests/read.in | 4 ++-- tests/test.fish | 2 +- tests/test6.in | 2 +- 11 files changed, 28 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b1741ff32..de7c11d61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ 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). @@ -51,7 +53,6 @@ This section is for changes merged to the `major` branch that are not also merge - The machine hostname, where available, is now exposed as `$hostname` which is now a reserved variable. This drops the dependency on the `hostname` executable (#4422). - `functions --handlers` can be used to show event handlers (#4694). - Variables set in `if` and `while` conditions are available outside the block (#4820). -- The `?` wildcard has been removed (#4520). ## Other significant changes - Command substitution output is now limited to 10 MB by default (#3822). diff --git a/src/common.cpp b/src/common.cpp index a1612a06e..f5fcf65c3 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -1007,7 +1007,6 @@ static void escape_string_script(const wchar_t *orig_in, size_t in_len, wcstring case L'$': case L' ': case L'#': - case L'^': case L'<': case L'>': case L'(': diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index fb080f632..d748e6e27 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -534,7 +534,7 @@ static void test_tokenizer() { const wchar_t *str = L"string &1 'nested \"quoted\" '(string containing subshells " - L"){and,brackets}$as[$well (as variable arrays)] not_a_redirect^ ^ ^^is_a_redirect " + L"){and,brackets}$as[$well (as variable arrays)] not_a_redirect^ 2> 2>^is_a_redirect " L"&&& ||| " L"&& || & |" L"Compress_Newlines\n \n\t\n \nInto_Just_One"; @@ -608,8 +608,6 @@ 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) diff --git a/src/tokenizer.cpp b/src/tokenizer.cpp index afd0903da..75d4ce71f 100644 --- a/src/tokenizer.cpp +++ b/src/tokenizer.cpp @@ -70,10 +70,8 @@ bool tokenizer_t::next(struct tok_t *result) { return true; } -/// 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) { +/// Tests if this character can be a part of a string. +static bool tok_is_string_character(wchar_t c) { switch (c) { case L'\0': case L' ': @@ -84,15 +82,9 @@ static bool tok_is_string_character(wchar_t c, bool is_first) { case L'\r': case L'<': case L'>': - case L'&': { - // Unconditional separators. + case L'&': return false; - } - case L'^': { - // Conditional separator. - return !is_first; - } - default: { return true; } + default: return true; } } @@ -117,7 +109,6 @@ tok_t tokenizer_t::read_string() { std::vector expecting; int slice_offset = 0; const wchar_t *const buff_start = this->buff; - bool is_first = true; while (true) { wchar_t c = *this->buff; @@ -219,7 +210,7 @@ tok_t tokenizer_t::read_string() { break; } } - else if (mode == tok_mode::regular_text && !tok_is_string_character(c, is_first)) { + else if (mode == tok_mode::regular_text && !tok_is_string_character(c)) { break; } @@ -233,7 +224,6 @@ tok_t tokenizer_t::read_string() { #endif this->buff++; - is_first = false; } if ((!this->accept_unfinished) && (mode != tok_mode::regular_text)) { @@ -292,7 +282,7 @@ static maybe_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 - // '>' or '^'. Try parsing out a number; if we did not get any digits then infer it from the + // '>'. 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++) { @@ -313,10 +303,6 @@ static maybe_t read_redirection_or_fd_pipe(const wchar_t result.fd = STDIN_FILENO; break; } - case L'^': { - result.fd = STDERR_FILENO; - break; - } default: { errored = true; break; @@ -325,12 +311,11 @@ static maybe_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'>' || (redirect_char == L'^' && idx == 1)) { + if (redirect_char == L'>') { result.redirection_mode = redirection_type_t::overwrite; if (buff[idx] == redirect_char) { - // Doubled up like ^^ or >>. That means append. + // Doubled up like >>. That means append. result.redirection_mode = redirection_type_t::append; idx++; } @@ -505,8 +490,7 @@ maybe_t tokenizer_t::tok_next() { break; } case L'>': - case L'<': - case L'^': { + case L'<': { // There's some duplication with the code in the default case below. The key difference // here is that we must never parse these as a string; a failed redirection is an error! auto redir_or_pipe = read_redirection_or_fd_pipe(this->buff); @@ -615,9 +599,7 @@ 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) { - // 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); + return tok_is_string_character(c) && !wcschr(L"/={,}'\"", c); } bool move_word_state_machine_t::consume_char_path_components(wchar_t c) { diff --git a/tests/gen_output.fish b/tests/gen_output.fish index 7bf7f3ec3..788e1a421 100755 --- a/tests/gen_output.fish +++ b/tests/gen_output.fish @@ -8,10 +8,10 @@ # for i in $argv - set template_out (basename $i .in).out - set template_err (basename $i .in).err - set template_status (basename $i .in).status + set template_out (basename $i .in).out + set template_err (basename $i .in).err + set template_status (basename $i .in).status - fish <$i >$template_out ^$template_err - echo $status >$template_status + fish <$i >$template_out 2>$template_err + echo $status >$template_status end diff --git a/tests/interactive.fish b/tests/interactive.fish index 41552f615..ad10cbf54 100644 --- a/tests/interactive.fish +++ b/tests/interactive.fish @@ -32,8 +32,9 @@ else set files_to_test *.expect end -source test_util.fish (status -f) $argv; or exit -cat interactive.config >> $XDG_CONFIG_HOME/fish/config.fish +source test_util.fish (status -f) $argv +or exit +cat interactive.config >>$XDG_CONFIG_HOME/fish/config.fish say -o cyan "Testing interactive functionality" if not type -q expect @@ -46,7 +47,7 @@ function test_file echo -n "Testing file $file ... " begin set -lx TERM dumb - expect -n -c 'source interactive.expect.rc' -f $file >$file.tmp.out ^$file.tmp.err + expect -n -c 'source interactive.expect.rc' -f $file >$file.tmp.out 2>$file.tmp.err end set -l exit_status $status set -l res ok diff --git a/tests/jobs.in b/tests/jobs.in index 9b2ba6c22..326f20e59 100644 --- a/tests/jobs.in +++ b/tests/jobs.in @@ -3,7 +3,7 @@ sleep 5 & sleep 5 & jobs -c jobs -q; echo $status -bg -23 1 ^/dev/null +bg -23 1 2>/dev/null or echo bg: invalid option -23 >&2 fg 3 bg 3 diff --git a/tests/printf.in b/tests/printf.in index 366b34921..4ee7e95cd 100644 --- a/tests/printf.in +++ b/tests/printf.in @@ -28,7 +28,7 @@ printf 'a\cb' echo # Bogus printf specifier, should produce no stdout -printf "%5" 10 ^/dev/null +printf "%5" 10 2>/dev/null # Octal escapes produce literal bytes, not characters # \376 is 0xFE diff --git a/tests/read.in b/tests/read.in index fcd2e071d..7230e4cfc 100644 --- a/tests/read.in +++ b/tests/read.in @@ -141,7 +141,7 @@ set line abcdefghijklmnopqrstuvwxyz # Ensure the `read` command terminates if asked to read too much data. The var # should be empty. We throw away any data we read if it exceeds the limit on # what we consider reasonable. -yes $line | dd bs=1024 count=(math "1 + $fish_read_limit / 1024") ^/dev/null | read --null x +yes $line | dd bs=1024 count=(math "1 + $fish_read_limit / 1024") 2>/dev/null | read --null x if test $status -ne 122 echo reading too much data did not terminate with failure status end @@ -188,7 +188,7 @@ end # Same as previous test but limit the amount of data fed to `read` rather than # using the `--nchars` flag. -yes $line | dd bs=1024 count=(math "$fish_read_limit / 1024") ^/dev/null | read --null x +yes $line | dd bs=1024 count=(math "$fish_read_limit / 1024") 2>/dev/null | read --null x if test $status -ne 0 echo the read of the max amount of data failed unexpectedly end diff --git a/tests/test.fish b/tests/test.fish index c42e4a08e..0a0dd5605 100644 --- a/tests/test.fish +++ b/tests/test.fish @@ -35,7 +35,7 @@ function test_file echo -n "Testing file $file ... " - ../test/root/bin/fish <$file >$base.tmp.out ^$base.tmp.err + ../test/root/bin/fish <$file >$base.tmp.out 2>$base.tmp.err set -l exit_status $status set -l res ok diff --git a/tests/test6.in b/tests/test6.in index 0ea772bb9..b64d2048b 100644 --- a/tests/test6.in +++ b/tests/test6.in @@ -88,7 +88,7 @@ if begin; rm -rf test6.tmp.dir; and mkdir test6.tmp.dir; end end popd if begin - set -l PATH $PWD/test6.tmp.dir $PATH ^/dev/null + set -l PATH $PWD/test6.tmp.dir $PATH 2>/dev/null complete -C$dir | grep "^$dir/.*Directory" >/dev/null end echo "incorrect implicit cd from PATH"