Common (Format): fixes module format strings mishandling a value that carries more than one leading ANSI escape sequence

CHANGELOG.md

@@ -84,6 +84,7 @@ Bugfixes:
     * This fixes PKG package count detection on FreeBSD.
 * Fixed `{#keys}` and `{#title}` in module format strings not honoring the `brightColor` display option. (Format)
 * Fixed `paddingTop` and `paddingLeft` being ignored by the `kitty-icat` image logo type. (Logo)
+* Fixed module format strings mishandling a value that carries more than one leading ANSI escape sequence: the second escape was taken for the start of the trailing reset, so `:`, `<`, `>` and `|` left the value untruncated instead of truncating it.
 * Fixed issues when running on big-endian platforms.
 * Fixed building on FreeBSD 16.0-CURRENT, where `<string.h>` defines `memrchr` as a qualifier-preserving function-like macro. (General, FreeBSD)
 * Some internal cleanups and optimizations.

src/common/impl/format.c

@@ -438,31 +438,67 @@ static bool parseQuickJSString(FFstrbuf* buffer, const char* script, uint32_t sc
 }
 #endif
 
-static bool skipAnsiEscape(FFstrbuf* in, FFstrbuf* out, FFstrbuf* trailingEscape) {
+// Returns the length of the ANSI CSI sequence at `str`, or 0 if there is none.
-    if (__builtin_expect(in->chars[0] == '\e' && in->chars[1] == '[', false)) {
+// A CSI sequence is `ESC [` + parameter bytes (0x30-0x3F) + intermediate bytes (0x20-0x2F) + final byte (0x40-0x7E)
-        // skip ANSI escape codes at the start of the string for truncation
+static uint32_t ansiEscapeLength(const char* str) {
-        const char* p = in->chars + 2;
+    if (str[0] != '\e' || str[1] != '[') {
-        while (*p && (ffCharIsDigit(*p) || *p == ';')) {
+        return 0;
-            ++p;
-        }
-        if (*p && isascii(*p)) {
-            ++p;
-        }
-        uint32_t prefixLen = (uint32_t) (p - in->chars);
-        ffStrbufAppendNS(out, prefixLen, in->chars);
-        ffStrbufSubstrAfter(in, prefixLen - 1);
-
-        if (trailingEscape) {
-            // likely have a `CSI m` reset at the end of the string
-            uint32_t iLastEscape = ffStrbufLastIndexC(in, '\e');
-            if (iLastEscape != in->length) {
-                ffStrbufSetNS(trailingEscape, in->length - iLastEscape, in->chars + iLastEscape);
-                ffStrbufSubstrBefore(in, iLastEscape);
-            }
-        }
-        return true;
     }
-    return false;
+
+    const char* p = str + 2;
+    while (*p >= '0' && *p <= '?') {
+        ++p;
+    }
+    while (*p >= ' ' && *p <= '/') {
+        ++p;
+    }
+    if (*p < '@' || *p > '~') {
+        return 0; // not a complete CSI sequence
+    }
+    return (uint32_t) (p - str) + 1;
+}
+
+static bool skipAnsiEscape(FFstrbuf* in, FFstrbuf* out, FFstrbuf* trailingEscape) {
+    // Skip all ANSI escape codes at the start of the string, so that they don't count
+    // towards the truncation / padding length. A value may carry more than one of them,
+    // e.g. `\e[31m\e[1m`.
+    uint32_t prefixLen = 0;
+    while (in->length - prefixLen >= 2) {
+        uint32_t len = ansiEscapeLength(in->chars + prefixLen);
+        if (len == 0) {
+            break;
+        }
+        prefixLen += len;
+    }
+
+    if (prefixLen == 0) {
+        return false;
+    }
+
+    ffStrbufAppendNS(out, prefixLen, in->chars);
+    ffStrbufSubstrAfter(in, prefixLen - 1);
+
+    if (trailingEscape) {
+        // likely have one or more `CSI m` resets at the end of the string
+        uint32_t end = in->length;
+        while (end >= 3) {
+            uint32_t start = end - 1;
+            while (start > 0 && in->chars[start] != '\e') {
+                --start;
+            }
+            uint32_t len = ansiEscapeLength(in->chars + start);
+            if (len == 0 || start + len != end) {
+                break;
+            }
+            end = start;
+        }
+
+        if (end < in->length) {
+            ffStrbufSetNS(trailingEscape, in->length - end, in->chars + end);
+            ffStrbufSubstrBefore(in, end);
+        }
+    }
+    return true;
 }
 
 static bool parseFormatString(FFstrbuf* buffer, const FFstrbuf* formatstr, uint32_t numArgs, const FFformatarg* arguments) {

tests/format.c

@@ -147,6 +147,23 @@ int main(void) {
         VERIFY("output({1~2})", "\e[30;47m12345\e[m", "output(\e[30;47m345\e[m)");
     }
 
+    {
+        // All leading escape codes must be skipped, not just the first one
+        VERIFY("output({1:3})", "\e[31m\e[1m12345", "output(\e[31m\e[1m123)");
+        VERIFY("output({1:-3})", "\e[31m\e[1m12345", "output(\e[31m\e[1m123…)");
+        VERIFY("output({1<8})", "\e[31m\e[1m12345", "output(\e[31m\e[1m12345   )");
+        VERIFY("output({1~2})", "\e[31m\e[1m12345", "output(\e[31m\e[1m345)");
+
+        // ... and so must all trailing ones
+        VERIFY("output({1:3})", "\e[31m\e[1m12345\e[m\e[39m", "output(\e[31m\e[1m123\e[m\e[39m)");
+        VERIFY("output({1~2})", "\e[31m\e[1m12345\e[m\e[39m", "output(\e[31m\e[1m345\e[m\e[39m)");
+        VERIFY("output({1<8})", "\e[31m\e[1m12345\e[m\e[39m", "output(\e[31m\e[1m12345   \e[m\e[39m)");
+
+        // `\e[?25l` is a complete CSI sequence, not just `\e[?`
+        VERIFY("output({1:3})", "\e[?25l12345", "output(\e[?25l123)");
+        VERIFY("output({1~4})", "\e[?25l12345", "output(\e[?25l5)");
+    }
+
 #ifndef _WIN32 // Windows doesn't have setenv
     {
         ffListInit(&instance.config.display.constants);