From 709e91c1e6f71ce1e4858ddf7e8542ea93aec1c0 Mon Sep 17 00:00:00 2001
From: Fabian Homborg <FHomborg@gmail.com>
Date: Wed, 16 Sep 2020 18:22:12 +0200
Subject: [PATCH] builtin test: Let `-t` work for the standard streams

Since builtins don't actually have the streams connected, but instead
read input via the io_streams_t objects, this would just always say
what *fish's* fds were.

Instead, pass along some of the stream data to check those
specifically - nobody cares that `test`s fd 0 *technically* is stdin.
What they want to know is that, if they used another program in that
place, it would connect to the TTY.

This is pretty hacky - I abused static variables for this, but
since it's two bools and an int it's probably okay.

See #1228.

Fixes #4766.
---
 src/builtin_test.cpp     | 17 +++++++++++-
 tests/pexpects/isatty.py | 59 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 1 deletion(-)
 create mode 100644 tests/pexpects/isatty.py

diff --git a/src/builtin_test.cpp b/src/builtin_test.cpp
index 717e9817f..e258b9225 100644
--- a/src/builtin_test.cpp
+++ b/src/builtin_test.cpp
@@ -78,6 +78,10 @@ enum token_t {
     test_paren_close,  // ")", close paren
 };
 
+static int stdin_fd{-1};
+static bool out_is_redirected;
+static bool err_is_redirected;
+
 /// Our number type. We support both doubles and long longs. We have to support these separately
 /// because some integers are not representable as doubles; these may come up in practice (e.g.
 /// inodes).
@@ -104,7 +108,11 @@ class number_t {
     // Return true if the number is a tty()/
     bool isatty() const {
         if (delta != 0.0 || base > INT_MAX || base < INT_MIN) return false;
-        return ::isatty(static_cast<int>(base));
+        int bint = static_cast<int>(base);
+        if (bint == 0) return ::isatty(stdin_fd);
+        if (bint == 1) return !out_is_redirected && ::isatty(STDOUT_FILENO);
+        if (bint == 2) return !err_is_redirected && ::isatty(STDERR_FILENO);
+        return ::isatty(bint);
     }
 };
 
@@ -882,6 +890,13 @@ maybe_t<int> builtin_test(parser_t &parser, io_streams_t &streams, wchar_t **arg
         return args.at(0).empty() ? STATUS_CMD_ERROR : STATUS_CMD_OK;
     }
 
+    // HACK: We have static variables describing the stream state.
+    // This is supremely cheesy, but the alternative is threading them through
+    // *every single evaluation function*, even the ones that would never use them.
+    stdin_fd = streams.stdin_fd;
+    out_is_redirected = streams.out_is_redirected;
+    out_is_redirected = streams.out_is_redirected;
+
     // Try parsing
     wcstring err;
     unique_ptr<expression> expr = test_parser::parse_args(args, err, program_name);
diff --git a/tests/pexpects/isatty.py b/tests/pexpects/isatty.py
new file mode 100644
index 000000000..743e87c61
--- /dev/null
+++ b/tests/pexpects/isatty.py
@@ -0,0 +1,59 @@
+#!/usr/bin/env python3
+from pexpect_helper import SpawnedProc
+import subprocess
+import sys
+import time
+
+sp = SpawnedProc()
+send, sendline, sleep, expect_prompt, expect_re, expect_str = (
+    sp.send,
+    sp.sendline,
+    sp.sleep,
+    sp.expect_prompt,
+    sp.expect_re,
+    sp.expect_str,
+)
+expect_prompt()
+
+sendline("test -t 0; echo $status")
+expect_prompt("0")
+
+sendline("""function t
+test -t 0 && echo stdin
+test -t 1 && echo stdout
+test -t 2 && echo stderr
+end""")
+expect_prompt()
+
+sendline("t")
+expect_str("stdin")
+expect_str("stdout")
+expect_str("stderr")
+expect_prompt()
+
+sendline("cat </dev/null | t")
+expect_str("stdout")
+expect_str("stderr")
+expect_prompt()
+
+sendline("t | cat")
+expect_str("stdin")
+expect_str("stderr")
+expect_prompt()
+
+sendline("t 2>| cat")
+expect_str("stdin")
+expect_str("stdout")
+expect_prompt()
+
+sendline("cat </dev/null | t | cat")
+expect_str("stderr")
+expect_prompt()
+sendline("cat </dev/null | t 2>| cat")
+expect_str("stdout")
+expect_prompt()
+
+sendline("t </dev/null")
+expect_str("stdout")
+expect_str("stderr")
+expect_prompt()