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

@@ -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).

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) {

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(); }
 };
 

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;
     }
 

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)