From 2a92e66902cdc15af74c03d25378262a256ec181 Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Mon, 14 Oct 2019 15:45:40 -0700
Subject: [PATCH] Support for &> and &| as convenience redirections

This adds support for &> and &| syntax, which both redirect stdout, and
also apply a redirection of stderr to stdout.
---
 CHANGELOG.md               |  5 +++--
 sphinx_doc_src/index.rst   |  7 ++++---
 src/fish_tests.cpp         | 22 ++++++++++++++++++----
 src/parse_execution.cpp    | 18 ++++++++++++++++++
 src/proc.h                 |  2 +-
 src/tokenizer.cpp          | 33 ++++++++++++++++++++++++++++++++-
 src/tokenizer.h            |  4 ++++
 tests/checks/redirect.fish | 27 +++++++++++++++++++++++++++
 8 files changed, 107 insertions(+), 11 deletions(-)
 create mode 100644 tests/checks/redirect.fish

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ae361417b..12754e5ff 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,8 +19,9 @@
 - `switch` now allows arguments that expand to nothing, like empty variables (#5677).
 - The null command (`:`) now always exits successfully, rather than passing through the previous exit status (#6022).
 - `jobs --last` returns 0 to indicate success when a job is found (#6104).
-- `commandline -p` and `commandline -j` now split on `&&` and `||` in addition to `;` and `&` (#6214)
-- `fish` now correctly handles CDPATH entries that starts with `..` (#6220)
+- `commandline -p` and `commandline -j` now split on `&&` and `||` in addition to `;` and `&` (#6214).
+- `fish` now correctly handles CDPATH entries that starts with `..` (#6220).
+- New redirections `&>` and `&|` may be used to redirect or pipe stdout, and also redirect stderr to stdout (#6192).
 
 ### Syntax changes and new commands
 - Brace expansion now only takes place if the braces include a "," or a variable expansion, meaning common commands such as `git reset HEAD@{0}` do not require escaping (#5869).
diff --git a/sphinx_doc_src/index.rst b/sphinx_doc_src/index.rst
index fefb73736..0a25e42d7 100644
--- a/sphinx_doc_src/index.rst
+++ b/sphinx_doc_src/index.rst
@@ -337,8 +337,6 @@ Most programs use three input/output (IO) streams, each represented by a number
 
 - Standard error, FD 2, for writing errors and warnings, defaults to writing to the screen.
 
-The reason for providing for two output file descriptors is to allow separation of errors and warnings from regular program output.
-
 Any file descriptor can be directed to a different output than its default through a simple mechanism called a redirection.
 
 An example of a file redirection is ``echo hello > output.txt``, which directs the output of the echo command to the file output.txt.
@@ -358,9 +356,11 @@ An example of a file redirection is ``echo hello > output.txt``, which directs t
 
 - An ampersand followed by a minus sign (``&-``). The file descriptor will be closed.
 
+As a convenience, the redirection ``&>`` can be used to direct both stdout and stderr to the same file.
+
 Example:
 
-To redirect both standard output and standard error to the file 'all_output.txt', you can write ``echo Hello > all_output.txt 2>&1``.
+To redirect both standard output and standard error to the file 'all_output.txt', you can write ``echo Hello &> all_output.txt``, which is a convenience for ``echo Hello > all_output.txt 2>&1``.
 
 Any file descriptor can be redirected in an arbitrary way by prefixing the redirection with the file descriptor.
 
@@ -388,6 +388,7 @@ Pipes usually connect file descriptor 1 (standard output) of the first process t
 
 will attempt to build the fish program, and any errors will be shown using the less pager.
 
+As a convenience, the pipe ``&|`` may be used to redirect both stdout and stderr to the same process. (Note this is different from bash, which uses ``|&``).
 
 .. _syntax-background:
 
diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp
index 739144cd1..4476c6019 100644
--- a/src/fish_tests.cpp
+++ b/src/fish_tests.cpp
@@ -590,15 +590,16 @@ 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^ ^ ^^is_a_redirect "
+        L"&| &> "
         L"&&& ||| "
         L"&& || & |"
         L"Compress_Newlines\n  \n\t\n   \nInto_Just_One";
     using tt = token_type_t;
     const token_type_t types[] = {
-        tt::string, tt::redirect, tt::string,   tt::redirect, tt::string,     tt::string,
-        tt::string, tt::redirect, tt::redirect, tt::string,   tt::andand,     tt::background,
-        tt::oror,   tt::pipe,     tt::andand,   tt::oror,     tt::background, tt::pipe,
-        tt::string, tt::end,      tt::string};
+        tt::string,     tt::redirect,   tt::string,   tt::redirect, tt::string, tt::string,
+        tt::string,     tt::redirect,   tt::redirect, tt::string,   tt::pipe,   tt::redirect,
+        tt::andand,     tt::background, tt::oror,     tt::pipe,     tt::andand, tt::oror,
+        tt::background, tt::pipe,       tt::string,   tt::end,      tt::string};
 
     say(L"Test correct tokenization");
 
@@ -686,6 +687,13 @@ static void test_tokenizer() {
     do_test(pipe_or_redir(L"9999999999999>&2")->is_valid() == false);
     do_test(pipe_or_redir(L"9999999999999>&2")->is_valid() == false);
 
+    do_test(pipe_or_redir(L"&|")->is_pipe);
+    do_test(pipe_or_redir(L"&|")->stderr_merge);
+    do_test(!pipe_or_redir(L"&>")->is_pipe);
+    do_test(pipe_or_redir(L"&>")->stderr_merge);
+    do_test(pipe_or_redir(L"&>>")->stderr_merge);
+    do_test(pipe_or_redir(L"&>?")->stderr_merge);
+
     auto get_redir_mode = [](const wchar_t *s) -> maybe_t<redirection_mode_t> {
         if (auto redir = pipe_or_redir_t::from_string(s)) {
             return redir->mode;
@@ -4609,6 +4617,12 @@ static void test_highlighting() {
         {L"self%not", highlight_role_t::param},
     });
 
+    highlight_tests.push_back({
+        {L"false", highlight_role_t::command},
+        {L"&|", highlight_role_t::statement_terminator},
+        {L"true", highlight_role_t::command},
+    });
+
     auto &vars = parser_t::principal_parser().vars();
     // Verify variables and wildcards in commands using /bin/cat.
     vars.set(L"VARIABLE_IN_COMMAND", ENV_LOCAL, {L"a"});
diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp
index 80e38ef08..2a0f6c7bf 100644
--- a/src/parse_execution.cpp
+++ b/src/parse_execution.cpp
@@ -80,6 +80,11 @@ static wcstring profiling_cmd_name_for_redirectable_block(const parse_node_t &no
     return result;
 }
 
+/// Get a redirection from stderr to stdout (i.e. 2>&1).
+static std::shared_ptr<io_data_t> get_stderr_merge() {
+    return std::make_shared<io_fd_t>(STDERR_FILENO, STDOUT_FILENO, true /* user_supplied */);
+}
+
 parse_execution_context_t::parse_execution_context_t(parsed_source_ref_t pstree, parser_t *p,
                                                      std::shared_ptr<job_t> parent)
     : pstree(std::move(pstree)), parser(p), parent_job(std::move(parent)) {}
@@ -1019,6 +1024,12 @@ bool parse_execution_context_t::determine_io_chain(tnode_t<g::arguments_or_redir
         if (new_io.get() != NULL) {
             result.push_back(new_io);
         }
+
+        if (redirect->stderr_merge) {
+            // This was a redirect like &> which also modifies stderr.
+            // Also redirect stderr to stdout.
+            result.push_back(get_stderr_merge());
+        }
     }
 
     if (out_chain && !errored) {
@@ -1141,6 +1152,13 @@ parse_execution_result_t parse_execution_context_t::populate_job_from_job_node(
             break;
         }
         processes.back()->pipe_write_fd = parsed_pipe->fd;
+        if (parsed_pipe->stderr_merge) {
+            // This was a pipe like &| which redirects both stdout and stderr.
+            // Also redirect stderr to stdout.
+            auto ios = processes.back()->io_chain();
+            ios.push_back(get_stderr_merge());
+            processes.back()->set_io_chain(std::move(ios));
+        }
 
         // Store the new process (and maybe with an error).
         processes.emplace_back(new process_t());
diff --git a/src/proc.h b/src/proc.h
index c093e5c1d..4b086011e 100644
--- a/src/proc.h
+++ b/src/proc.h
@@ -215,7 +215,7 @@ class process_t {
     /// IO chain getter and setter.
     const io_chain_t &io_chain() const { return process_io_chain; }
 
-    void set_io_chain(const io_chain_t &chain) { this->process_io_chain = chain; }
+    void set_io_chain(io_chain_t chain) { this->process_io_chain = std::move(chain); }
 
     /// Store the current topic generations. That is, right before the process is launched, record
     /// the generations of all topics; then we can tell which generation values have changed after
diff --git a/src/tokenizer.cpp b/src/tokenizer.cpp
index 7fe4708a5..037c4bbd8 100644
--- a/src/tokenizer.cpp
+++ b/src/tokenizer.cpp
@@ -296,6 +296,7 @@ maybe_t<pipe_or_redir_t> pipe_or_redir_t::from_string(const wchar_t *buff) {
        Note we are only responsible for parsing the redirection part, not 'cmd' or 'file'.
 
         cmd | cmd        normal pipe
+        cmd &| cmd       normal pipe plus stderr-merge
         cmd >| cmd       pipe with explicit fd
         cmd 2>| cmd      pipe with explicit fd
         cmd < file       stdin redirection
@@ -308,6 +309,7 @@ maybe_t<pipe_or_redir_t> pipe_or_redir_t::from_string(const wchar_t *buff) {
         cmd 1>&2 file    fd redirection with an explicit src fd
         cmd <&2 file     fd redirection with no explicit src fd (stdin is used)
         cmd 3<&0 file    fd redirection with an explicit src fd
+        cmd &> file      redirection with stderr merge
         cmd ^ file       caret (stderr) redirection, perhaps disabled via feature flags
         cmd ^^ file      caret (stderr) redirection, perhaps disabled via feature flags
     */
@@ -404,7 +406,25 @@ maybe_t<pipe_or_redir_t> pipe_or_redir_t::from_string(const wchar_t *buff) {
                 break;
             }
         }
-
+        case L'&': {
+            consume(L'&');
+            if (try_consume(L'|')) {
+                // &| is pipe with stderr merge.
+                result.fd = STDOUT_FILENO;
+                result.is_pipe = true;
+                result.stderr_merge = true;
+            } else if (try_consume(L'>')) {
+                result.fd = STDOUT_FILENO;
+                result.stderr_merge = true;
+                result.mode = redirection_mode_t::overwrite;
+                if (try_consume(L'>')) result.mode = redirection_mode_t::append;  // like &>>
+                if (try_consume(L'?'))
+                    result.mode = redirection_mode_t::noclob;  // like &>? or &>>?
+            } else {
+                return none();
+            }
+            break;
+        }
         default: {
             // Not a redirection.
             return none();
@@ -521,10 +541,20 @@ maybe_t<tok_t> tokenizer_t::next() {
         }
         case L'&': {
             if (this->buff[1] == L'&') {
+                // && is and.
                 result.emplace(token_type_t::andand);
                 result->offset = start_pos;
                 result->length = 2;
                 this->buff += 2;
+            } else if (this->buff[1] == L'>' || this->buff[1] == L'|') {
+                // &> and &| redirect both stdout and stderr.
+                auto redir = pipe_or_redir_t::from_string(buff);
+                assert(redir.has_value() &&
+                       "Should always succeed to parse a &> or &| redirection");
+                result.emplace(redir->token_type());
+                result->offset = start_pos;
+                result->length = redir->consumed;
+                this->buff += redir->consumed;
             } else {
                 result.emplace(token_type_t::background);
                 result->offset = start_pos;
@@ -535,6 +565,7 @@ maybe_t<tok_t> tokenizer_t::next() {
         }
         case L'|': {
             if (this->buff[1] == L'|') {
+                // || is or.
                 result.emplace(token_type_t::oror);
                 result->offset = start_pos;
                 result->length = 2;
diff --git a/src/tokenizer.h b/src/tokenizer.h
index 91fd55993..a24a26d38 100644
--- a/src/tokenizer.h
+++ b/src/tokenizer.h
@@ -152,6 +152,10 @@ struct pipe_or_redir_t {
     // Ignored for pipes.
     redirection_mode_t mode{redirection_mode_t::overwrite};
 
+    // Whether, in addition to this redirection, stderr should also be dup'd to stdout
+    // For example &| or &>
+    bool stderr_merge{false};
+
     // Number of characters consumed when parsing the string.
     size_t consumed{0};
 
diff --git a/tests/checks/redirect.fish b/tests/checks/redirect.fish
new file mode 100644
index 000000000..d7768c70a
--- /dev/null
+++ b/tests/checks/redirect.fish
@@ -0,0 +1,27 @@
+#RUN: %fish %s
+
+function outnerr
+    command echo out $argv
+    command echo err $argv 1>&2
+end
+
+outnerr 0 &| count
+#CHECK: 2
+
+set -l tmpdir (mktemp -d)
+outnerr overwrite &> $tmpdir/file.txt
+cat $tmpdir/file.txt
+#CHECK: out overwrite
+#CHECK: err overwrite
+
+outnerr append &>> $tmpdir/file.txt
+cat $tmpdir/file.txt
+#CHECK: out overwrite
+#CHECK: err overwrite
+#CHECK: out append
+#CHECK: err append
+
+echo noclobber &>>? $tmpdir/file.txt
+#CHECKERR: {{.*}} The file {{.*}} already exists
+
+rm -Rf $tmpdir