From 812cc1dbafba5ec20d1846b93d0f535b3d0ee8aa Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Sun, 7 Jun 2020 13:53:41 -0700
Subject: [PATCH] Clean up line_t

Use a single allocation instead of two for text and colors.
Comment and tighten up its methods.
---
 src/fish_tests.cpp |  5 ++++-
 src/screen.cpp     | 15 +++++++++++----
 src/screen.h       | 40 +++++++++++++++++++++++-----------------
 3 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp
index b08346c50..e1ce29851 100644
--- a/src/fish_tests.cpp
+++ b/src/fish_tests.cpp
@@ -2294,7 +2294,10 @@ struct pager_layout_testcase_t {
                 std::replace(expected.begin(), expected.end(), L'\x2026', ellipsis_char);
             }
 
-            wcstring text = sd.line(0).to_string();
+            wcstring text;
+            for (const auto &p : sd.line(0).text) {
+                text.push_back(p.first);
+            }
             if (text != expected) {
                 std::fwprintf(stderr, L"width %zu got %zu<%ls>, expected %zu<%ls>\n", this->width,
                               text.length(), text.c_str(), expected.length(), expected.c_str());
diff --git a/src/screen.cpp b/src/screen.cpp
index 57be553e2..39678b63e 100644
--- a/src/screen.cpp
+++ b/src/screen.cpp
@@ -84,6 +84,14 @@ static size_t next_tab_stop(size_t current_line_width) {
 /// Like fish_wcwidth, but returns 0 for control characters instead of -1.
 static int fish_wcwidth_min_0(wchar_t widechar) { return std::max(0, fish_wcwidth(widechar)); }
 
+int line_t::wcswidth_min_0(size_t max) const {
+    int result = 0;
+    for (size_t idx = 0, end = std::min(max, text.size()); idx < end; idx++) {
+        result += fish_wcwidth_min_0(text[idx].first);
+    }
+    return result;
+}
+
 /// Whether we permit soft wrapping. If so, in some cases we don't explicitly move to the second
 /// physical line on a wrapped logical line; instead we just output it.
 static bool allow_soft_wrap() {
@@ -767,9 +775,8 @@ static void s_update(screen_t *scr, const wcstring &left_prompt, const wcstring
         // over the shared prefix of what we want to output now, and what we output before, to
         // avoid repeatedly outputting it.
         if (skip_prefix > 0) {
-            size_t skip_width = shared_prefix < skip_prefix
-                                    ? skip_prefix
-                                    : fish_wcswidth(&o_line.text.at(0), shared_prefix);
+            size_t skip_width =
+                shared_prefix < skip_prefix ? skip_prefix : o_line.wcswidth_min_0(shared_prefix);
             if (skip_width > skip_remaining) skip_remaining = skip_width;
         }
 
@@ -846,7 +853,7 @@ static void s_update(screen_t *scr, const wcstring &left_prompt, const wcstring
             // Only do it if the previous line could conceivably be wider.
             // That means if it is a prefix of the current one we can skip it.
             if (s_line.text.size() != shared_prefix) {
-                int prev_width = fish_wcswidth(&s_line.text.at(0), s_line.text.size());
+                int prev_width = s_line.wcswidth_min_0();
                 clear_remainder = prev_width > current_width;
             }
         }
diff --git a/src/screen.h b/src/screen.h
index 6a33ecfb2..177f17e01 100644
--- a/src/screen.h
+++ b/src/screen.h
@@ -20,6 +20,7 @@
 #include <memory>
 #include <unordered_map>
 #include <unordered_set>
+#include <utility>
 #include <vector>
 
 #include "common.h"
@@ -30,42 +31,47 @@ class page_rendering_t;
 
 /// A class representing a single line of a screen.
 struct line_t {
-    std::vector<wchar_t> text;
-    std::vector<highlight_spec_t> colors;
-    bool is_soft_wrapped;
-    size_t indentation;
+    /// A pair of a character, and the color with which to draw it.
+    using highlighted_char_t = std::pair<wchar_t, highlight_spec_t>;
+    std::vector<highlighted_char_t> text{};
+    bool is_soft_wrapped{false};
+    size_t indentation{0};
 
-    line_t() : text(), colors(), is_soft_wrapped(false), indentation(0) {}
+    line_t() = default;
 
+    /// Clear the line's contents.
     void clear(void) {
         text.clear();
-        colors.clear();
     }
 
-    void append(wchar_t txt, highlight_spec_t color) {
-        text.push_back(txt);
-        colors.push_back(color);
-    }
+    /// Append a single character \p txt to the line with color \p c.
+    void append(wchar_t c, highlight_spec_t color) { text.push_back({c, color}); }
 
+    /// Append a nul-terminated string \p txt to the line, giving each character \p color.
     void append(const wchar_t *txt, highlight_spec_t color) {
         for (size_t i = 0; txt[i]; i++) {
-            text.push_back(txt[i]);
-            colors.push_back(color);
+            text.push_back({txt[i], color});
         }
     }
 
-    size_t size(void) const { return text.size(); }
+    /// \return the number of characters.
+    size_t size() const { return text.size(); }
 
-    wchar_t char_at(size_t idx) const { return text.at(idx); }
+    /// \return the character at a char index.
+    wchar_t char_at(size_t idx) const { return text.at(idx).first; }
 
-    highlight_spec_t color_at(size_t idx) const { return colors.at(idx); }
+    /// \return the color at a char index.
+    highlight_spec_t color_at(size_t idx) const { return text.at(idx).second; }
 
+    /// Append the contents of \p line to this line.
     void append_line(const line_t &line) {
         text.insert(text.end(), line.text.begin(), line.text.end());
-        colors.insert(colors.end(), line.colors.begin(), line.colors.end());
     }
 
-    wcstring to_string() const { return wcstring(this->text.begin(), this->text.end()); }
+    /// \return the width of this line, counting up to no more than \p max characters.
+    /// This follows fish_wcswidth() semantics, except that characters whose width would be -1 are
+    /// treated as 0.
+    int wcswidth_min_0(size_t max = std::numeric_limits<size_t>::max()) const;
 };
 
 /// A class representing screen contents.