Support multiple read --line variable outputs

ref #4861. Also closes #4917.

src/builtin_read.cpp

@@ -12,6 +12,7 @@
 
 #include <algorithm>
 #include <memory>
+#include <numeric>
 #include <string>
 #include <vector>
 
@@ -461,19 +462,33 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         opts.shell = false;
     }
     else if (opts.one_line) {
-        // --line is the same as read -d \n
+        // --line is the same as read -d \n repeated N times
         opts.have_delimiter = true;
         opts.delimiter = L"\n";
         opts.split_null = false;
         opts.shell = false;
     }
 
+    wchar_t * const *var_ptr = argv;
+    auto vars_left = [&] () { return argv + argc - var_ptr; };
+    auto clear_remaining_vars = [&] () {
+        while (vars_left()) {
+            env_set_empty(*var_ptr, opts.place);
+            // env_set_one(*var_ptr, opts.place, L"");
+            ++var_ptr;
+        }
+    };
+
+    // Normally, we either consume a line of input or all available input. But if we are reading a line at
+    // a time, we need a middle ground where we only consume as many lines as we need to fill the given vars.
+    do {
+        buff.clear();
+
         // TODO: Determine if the original set of conditions for interactive reads should be reinstated:
         // if (isatty(0) && streams.stdin_fd == STDIN_FILENO && !split_null) {
         int stream_stdin_is_a_tty = isatty(streams.stdin_fd);
         if (stream_stdin_is_a_tty && !opts.split_null) {
-        // We should read interactively using reader_readline(). This does not support splitting on
+            // Read interactively using reader_readline(). This does not support splitting on null.
-        // null.
             exit_res = read_interactive(buff, opts.nchars, opts.shell, opts.silent, opts.prompt,
                                         opts.right_prompt, opts.commandline);
         } else if (!opts.nchars && !stream_stdin_is_a_tty &&
@@ -484,9 +499,7 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         }
 
         if (exit_res != STATUS_CMD_OK) {
-        // Define the var(s) without any data. We do this because when this happens we want the user
+            clear_remaining_vars();
-        // to be able to use the var but have it expand to nothing.
-        for (int i = 0; i < argc; i++) env_set_empty(argv[i], opts.place);
             return exit_res;
         }
 
@@ -504,45 +517,29 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
             // Every character is a separate token with one wrinkle involving non-array mode where the
             // final var gets the remaining characters as a single string.
             size_t x = std::max(static_cast<size_t>(1), buff.size());
-        size_t n_splits = (opts.array || static_cast<size_t>(argc) > x) ? x : argc;
+            size_t n_splits = (opts.array || static_cast<size_t>(vars_left()) > x) ? x : vars_left();
             wcstring_list_t chars;
             chars.reserve(n_splits);
             x = x - n_splits + 1;
             int i = 0;
             for (auto it = buff.begin(), end = buff.end(); it != end; ++i, ++it) {
-            if (opts.array || i < argc) {
+                if (opts.array || i + 1 < vars_left()) {
-                chars.emplace_back(wcstring(1, *it));
+                    chars.emplace_back(1, *it);
                 } else {
-                if (x) {
+                    chars.emplace_back(it, buff.end());
-                    chars.back().reserve(x);
+                    break;
-                    x = 0;
-                }
-                chars.back().push_back(*it);
                 }
             }
 
             if (opts.array) {
                 // Array mode: assign each char as a separate element of the sole var.
-            env_set(argv[0], opts.place, chars);
+                env_set(*var_ptr++, opts.place, chars);
             } else {
                 // Not array mode: assign each char to a separate var with the remainder being assigned
                 // to the last var.
-            int i = 0;
+                auto c = chars.begin();
-            size_t j = 0;
+                for (; c != chars.end() && vars_left(); ++c) {
-            for (; i + 1 < argc; ++i) {
+                    env_set_one(*var_ptr++, opts.place, *c);
-                if (j < chars.size()) {
-                    env_set_one(argv[i], opts.place, chars[j]);
-                    j++;
-                } else {
-                    env_set_one(argv[i], opts.place, L"");
-                }
-            }
-
-            if (i < argc) {
-                wcstring val = chars.size() == static_cast<size_t>(argc) ? chars[i] : L"";
-                env_set_one(argv[i], opts.place, val);
-            } else {
-                env_set_empty(argv[i], opts.place);
                 }
             }
         } else if (opts.array) {
@@ -558,14 +555,14 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
                      loc.first != wcstring::npos; loc = wcstring_tok(buff, opts.delimiter, loc)) {
                     tokens.emplace_back(wcstring(buff, loc.first, loc.second));
                 }
-            env_set(argv[0], opts.place, tokens);
+                env_set(*var_ptr++, opts.place, tokens);
             } else {
                 // We're using a delimiter provided by the user so use the `string split` behavior.
                 wcstring_list_t splits;
                 split_about(buff.begin(), buff.end(), opts.delimiter.begin(), opts.delimiter.end(),
                             &splits);
 
-            env_set(argv[0], opts.place, splits);
+                env_set(*var_ptr++, opts.place, splits);
             }
         } else {
             // Not array mode. Split the input into tokens and assign each to the vars in sequence.
@@ -573,13 +570,13 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
                 // We're using IFS, so tokenize the buffer using each IFS char. This is for backward
                 // compatibility with old versions of fish.
                 wcstring_range loc = wcstring_range(0, 0);
-            for (int i = 0; i < argc; i++) {
+                while (vars_left()) {
                     wcstring substr;
-                loc = wcstring_tok(buff, (i + 1 < argc) ? opts.delimiter : wcstring(), loc);
+                    loc = wcstring_tok(buff, (vars_left() > 1) ? opts.delimiter : wcstring(), loc);
                     if (loc.first != wcstring::npos) {
                         substr = wcstring(buff, loc.first, loc.second);
                     }
-                env_set_one(argv[i], opts.place, substr);
+                    env_set_one(*var_ptr++, opts.place, substr);
                 }
             } else {
                 // We're using a delimiter provided by the user so use the `string split` behavior.
@@ -588,11 +585,18 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
                 // is set to the remaining string.
                 split_about(buff.begin(), buff.end(), opts.delimiter.begin(), opts.delimiter.end(),
                             &splits, argc - 1);
-            for (size_t i = 0; i < (size_t)argc && i < splits.size(); i++) {
+                assert(splits.size() <= (size_t) vars_left());
-                env_set_one(argv[i], opts.place, splits[i]);
+                for (const auto &split : splits) {
+                    env_set_one(*var_ptr++, opts.place, split);
                 }
             }
         }
+    } while (opts.one_line && vars_left());
+
+    if (!opts.array) {
+        // In case there were more args than splits
+        clear_remaining_vars();
+    }
 
     return exit_res;
 }

src/wcstringutil.cpp

@@ -31,7 +31,7 @@ wcstring_range wcstring_tok(wcstring& str, const wcstring& needle, wcstring_rang
 }
 
 wcstring truncate(const wcstring &input, int max_len, ellipsis_type etype) {
-    if (input.size() <= max_len) {
+    if (input.size() <= (size_t) max_len) {
         return input;
     }