From 554ee240b31cebb2e170a89f15c7059209065dc9 Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Sun, 21 Jul 2019 13:53:05 -0700
Subject: [PATCH] Correct handling of explicitly separated output when all
 elements are empty

Previously when propagating explicitly separated output, we would early-out
if the buffer was empty, where empty meant contains no characters. However
it may contain one or more empty strings, in which case we should propagate
those strings.

Remove this footgun "empty" function and handle this properly.

Fixes #5987
---
 CHANGELOG.md             | 1 +
 src/io.cpp               | 7 ++++---
 src/io.h                 | 2 --
 src/parse_execution.cpp  | 5 +++--
 tests/checks/string.fish | 6 ++++++
 5 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5e884a7d2..35a86ae93 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,7 @@
 - More of the documentation, including the tutorial, is now available as man pages as well.
 - Local values for `fish_complete_path` and `fish_function_path` are now ignored; only their global values are respected.
 - Empty universal variables may now be exported (#5992).
+- `string split` had a bug where empty strings would be dropped if the output was only empty strings; this has been fixed (#5987).
 
 ### Syntax changes and new commands
 - Brace expansion now only takes place if the braces include a "," or a variable expansion, so things like `git reset HEAD@{0}` now work (#5869).
diff --git a/src/io.cpp b/src/io.cpp
index edbf06e9b..54ead1392 100644
--- a/src/io.cpp
+++ b/src/io.cpp
@@ -31,14 +31,15 @@ void io_pipe_t::print() const {
 void io_bufferfill_t::print() const { std::fwprintf(stderr, L"bufferfill {%d}\n", write_fd_.fd()); }
 
 void io_buffer_t::append_from_stream(const output_stream_t &stream) {
-    if (stream.empty()) return;
+    const separated_buffer_t<wcstring> &input = stream.buffer();
+    if (input.elements().empty()) return;
     scoped_lock locker(append_lock_);
     if (buffer_.discarded()) return;
-    if (stream.buffer().discarded()) {
+    if (input.discarded()) {
         buffer_.set_discard();
         return;
     }
-    buffer_.append_wide_buffer(stream.buffer());
+    buffer_.append_wide_buffer(input);
 }
 
 void io_buffer_t::run_background_fillthread(autoclose_fd_t readfd) {
diff --git a/src/io.h b/src/io.h
index 89fcdc810..79ea595de 100644
--- a/src/io.h
+++ b/src/io.h
@@ -403,8 +403,6 @@ class output_stream_t {
 
     void append_formatv(const wchar_t *format, va_list va) { append(vformat_string(format, va)); }
 
-    bool empty() const { return buffer_.size() == 0; }
-
     wcstring contents() const { return buffer_.newline_serialized(); }
 };
 
diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp
index eff62a4e6..75d7cc7d0 100644
--- a/src/parse_execution.cpp
+++ b/src/parse_execution.cpp
@@ -337,8 +337,9 @@ parse_execution_result_t parse_execution_context_t::run_function_statement(
     int err = builtin_function(*parser, streams, arguments, pstree, body);
     parser->set_last_statuses(statuses_t::just(err));
 
-    if (!streams.err.empty()) {
-        this->report_error(header, L"%ls", streams.err.contents().c_str());
+    wcstring errtext = streams.err.contents();
+    if (!errtext.empty()) {
+        this->report_error(header, L"%ls", errtext.c_str());
         result = parse_execution_errored;
     }
 
diff --git a/tests/checks/string.fish b/tests/checks/string.fish
index c27b6b6c1..6e2ee8ba9 100644
--- a/tests/checks/string.fish
+++ b/tests/checks/string.fish
@@ -498,6 +498,12 @@ end
 count (dualsplit)
 # CHECK: 4
 
+# Ensure we handle empty outputs correctly (#5987)
+count (string split / /)
+# CHECK: 2
+count (echo -ne '\x00\x00\x00' | string split0)
+# CHECK: 3
+
 # string collect
 count (echo one\ntwo\nthree\nfour | string collect)
 count (echo one | string collect)