From 2087a3ca63281c7a45959b2fb5bb5405d2145b81 Mon Sep 17 00:00:00 2001
From: Fabian Homborg <FHomborg@gmail.com>
Date: Thu, 29 Jul 2021 20:49:14 +0200
Subject: [PATCH] Let visible length work with CR and LF

Because we are, ultimately, interested in how many cells a string
occupies, we *have* to handle carriage return (`\r`) and line
feed (`\n`).

A carriage return sets the current tally to 0, and only the longest
tally is kept. The idea here is that the last position is the same as
the last position of the longest string. So:

abcdef\r123

ends up looking like

123def

which is the same width as abcdef, 6.

A line feed meanwhile means we flush the current tally and start a new
one. Every line is printed separately, even if it's given as one.

That's because, well, counting the width over multiple lines
doesn't *help*.

As a sidenote: This is necessarily imperfect, because, while we may
know the width of the terminal ($COLUMNS), we don't know the current
cursor position. So we can only give the width, and the user can then
figure something out on their own.

But for the common case of figuring out how wide the prompt is, this
should do.
---
 src/builtin_string.cpp   | 40 +++++++++++++++++++++++++++++++---------
 tests/checks/string.fish | 25 +++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 9 deletions(-)

diff --git a/src/builtin_string.cpp b/src/builtin_string.cpp
index 8aaeabf61..d61e109f8 100644
--- a/src/builtin_string.cpp
+++ b/src/builtin_string.cpp
@@ -768,15 +768,37 @@ static int string_length(parser_t &parser, io_streams_t &streams, int argc, cons
     int nnonempty = 0;
     arg_iterator_t aiter(argv, optind, streams);
     while (const wcstring *arg = aiter.nextstr()) {
-        size_t n = opts.visible ? width_without_escapes(*arg) : arg->length();
-        if (n > 0) {
-            nnonempty++;
-        }
-        if (!opts.quiet) {
-            streams.out.append(to_string(n));
-            streams.out.append(L'\n');
-        } else if (nnonempty > 0) {
-            return STATUS_CMD_OK;
+        if (opts.visible) {
+            // Visible length only makes sense line-wise.
+            for (auto &line : split_string(*arg, L'\n')) {
+                size_t max = 0;
+                // Carriage-return returns us to the beginning,
+                // the longest string stays.
+                for (auto &reset : split_string(line, L'\r')) {
+                    size_t n = width_without_escapes(reset);
+                    if (n > max) max = n;
+                }
+                if (max > 0) {
+                    nnonempty++;
+                }
+                if (!opts.quiet) {
+                    streams.out.append(to_string(max));
+                    streams.out.append(L'\n');
+                } else if (nnonempty > 0) {
+                    return STATUS_CMD_OK;
+                }
+            }
+        } else {
+            size_t n = arg->length();
+            if (n > 0) {
+                nnonempty++;
+            }
+            if (!opts.quiet) {
+                streams.out.append(to_string(n));
+                streams.out.append(L'\n');
+            } else if (nnonempty > 0) {
+                return STATUS_CMD_OK;
+            }
         }
     }
 
diff --git a/tests/checks/string.fish b/tests/checks/string.fish
index 8f64da1a1..8f4296b96 100644
--- a/tests/checks/string.fish
+++ b/tests/checks/string.fish
@@ -87,6 +87,31 @@ string pad -c_ --width 5 longer-than-width-param x
 string pad -c ab -w4 .
 # CHECKERR: string pad: Padding should be a character 'ab'
 
+# Visible length. Let's start off simple, colors are ignored:
+string length --visible (set_color red)abc
+# CHECK: 3
+begin
+    set -l fish_emoji_width 2
+    # This should print the emoji width
+    string length --visible . 🐟
+    # CHECK: 1
+    # CHECK: 2
+    set -l fish_emoji_width 1
+    string length --visible . 🐟
+    # CHECK: 1
+    # CHECK: 1
+end
+
+# Only the longest run between carriage returns is kept because the rest is overwritten.
+string length --visible (set_color normal)abcdef\rfooba(set_color red)raaa
+# (foobaraaa)
+# CHECK: 9
+
+# Visible length is *always* split by line
+string length --visible a(set_color blue)b\ncde
+# CHECK: 2
+# CHECK: 3
+
 string sub --length 2 abcde
 # CHECK: ab