Format: fixes skipping nested conditionals

When a `{?arg}` / `{/arg}` conditional is skipped, the end of the skipped part
was the next `{?}` / `{/}` in the format string, even if it closed a
conditional nested in it. The text after the nested conditional was printed
and the real closing `{?}` / `{/}` was reported as an invalid placeholder.
A literal `{{?}}` inside the skipped part also ended it.

Skip to the `{?}` / `{/}` that matches the opening one, like the printing path
already does.

CHANGELOG.md

@@ -10,6 +10,7 @@ Bugfixes:
 * Fixed AMD GPU name detection in amdgpu.ids when PCI device ID or revision ID has leading zeros. (GPU, Linux)
 * Improved uptime detection on SunOS via kstat. (#2674, Uptime, SunOS)
 * Fixed substring formatting returning the remaining string when a negative end index is before the start of the string. (General)
+* Fixed a skipped `{?}` / `{/}` conditional in a format string ending at the `{?}` / `{/}` of a conditional nested in it. (General)
 
 Logos:
 * Added Tiny Core Linux (#2655)

src/common/impl/format.c

@@ -343,6 +343,51 @@ static bool skipAnsiEscape(FFstrbuf* in, FFstrbuf* out, FFstrbuf* trailingEscape
     return true;
 }
 
+// Returns the index of the `}` of the `{<marker>}` that closes the conditional we are in, or the
+// length of the format string if there is none. `i` is the index of the `}` that ends the opening
+// placeholder. The part to skip may hold conditionals of its own, which have to be passed over.
+// Like in `parseFormatString`, only a conditional with a valid argument opens a block.
+static uint32_t findConditionalEnd(const FFstrbuf* formatstr, uint32_t i, char marker, uint32_t numArgs, const FFformatarg* arguments) {
+    FF_STRBUF_AUTO_DESTROY placeholderValue = ffStrbufCreate();
+    uint32_t depth = 0;
+
+    for (++i; i < formatstr->length; ++i) {
+        if (formatstr->chars[i] != '{') {
+            continue;
+        }
+
+        ++i;
+        if (i >= formatstr->length) {
+            break;
+        }
+
+        // double {{ evaluates to a single { and doesn't start a placeholder
+        if (formatstr->chars[i] == '{') {
+            continue;
+        }
+
+        uint32_t iEnd = ffStrbufNextIndexC(formatstr, i, '}');
+        if (formatstr->chars[i] == marker) {
+            if (iEnd - i == 1) {
+                if (depth == 0) {
+                    return iEnd;
+                }
+                --depth;
+            } else {
+                ffStrbufClear(&placeholderValue);
+                ffStrbufAppendNS(&placeholderValue, iEnd - i - 1, &formatstr->chars[i + 1]);
+                uint32_t index = getArgumentIndex(placeholderValue.chars, numArgs, arguments);
+                if (index >= 1 && index <= numArgs) {
+                    ++depth;
+                }
+            }
+        }
+        i = iEnd;
+    }
+
+    return formatstr->length;
+}
+
 static bool parseFormatString(FFstrbuf* buffer, const FFstrbuf* formatstr, uint32_t numArgs, const FFformatarg* arguments) {
     uint32_t argCounter = 0;
 
@@ -440,7 +485,7 @@ static bool parseFormatString(FFstrbuf* buffer, const FFstrbuf* formatstr, uint3
             }
 
             // fastforward to the end of the if without printing the in between
-            i = ffStrbufNextIndexS(formatstr, i, "{?}") + 2; // 2 is the length of "{?}" - 1 because the loop will increment it again directly after continue
+            i = findConditionalEnd(formatstr, i, '?', numArgs, arguments);
             continue;
         }
 
@@ -463,7 +508,7 @@ static bool parseFormatString(FFstrbuf* buffer, const FFstrbuf* formatstr, uint3
             }
 
             // fastforward to the end of the if without printing the in between
-            i = ffStrbufNextIndexS(formatstr, i, "{/}") + 2; // 2 is the length of "{/}" - 1 because the loop will increment it again directly after continue
+            i = findConditionalEnd(formatstr, i, '/', numArgs, arguments);
             continue;
         }
 

tests/format.c

@@ -17,6 +17,19 @@ static void verify(const char* format, const char* arg, const char* expected, in
 
 #define VERIFY(format, argument, expected) verify((format), (argument), (expected), __LINE__)
 
+static void verify2(const char* format, const char* arg1, const char* arg2, const char* expected, int lineNo) {
+    FF_STRBUF_AUTO_DESTROY result = ffStrbufCreate();
+    FF_STRBUF_AUTO_DESTROY formatter = ffStrbufCreateStatic(format);
+    const FFformatarg arguments[] = { { .type = FF_ARG_TYPE_STRING, arg1 }, { .type = FF_ARG_TYPE_STRING, arg2 } };
+    ffParseFormatString(&result, &formatter, 2, arguments);
+    if (!ffStrbufEqualS(&result, expected)) {
+        fprintf(stderr, FASTFETCH_TEXT_MODIFIER_ERROR "[%d] %s: expected \"%s\", got \"%s\"\n" FASTFETCH_TEXT_MODIFIER_RESET, lineNo, format, expected, result.chars);
+        exit(1);
+    }
+}
+
+#define VERIFY2(format, argument1, argument2, expected) verify2((format), (argument1), (argument2), (expected), __LINE__)
+
 int main(void) {
     instance.config.display.pipe = true;
 
@@ -123,6 +136,28 @@ int main(void) {
         VERIFY("output({?1}OK{?}{/1}NOT OK{/})", "", "output(NOT OK)");
     }
 
+    {
+        // a skipped conditional may hold conditionals of its own
+        VERIFY2("{?1}A{?2}B{?}C{?}D", "x", "x", "ABCD");
+        VERIFY2("{?1}A{?2}B{?}C{?}D", "x", "", "ACD");
+        VERIFY2("{?1}A{?2}B{?}C{?}D", "", "x", "D");
+        VERIFY2("{?1}A{?2}B{?}C{?}D", "", "", "D");
+        VERIFY2("{/1}A{/2}B{/}C{/}D", "", "", "ABCD");
+        VERIFY2("{/1}A{/2}B{/}C{/}D", "", "x", "ACD");
+        VERIFY2("{/1}A{/2}B{/}C{/}D", "x", "", "D");
+        VERIFY2("{/1}A{/2}B{/}C{/}D", "x", "x", "D");
+        VERIFY2("{?1}A{?1}B{?1}C{?}D{?}E{?}F", "", "", "F");
+        VERIFY2("{?1}{/2}A{/}B{?}C", "", "", "C");
+
+        // a conditional with an invalid argument does not open a block
+        VERIFY2("{?1}A{?3}B{?}C", "", "x", "C");
+        VERIFY2("{?1}A{?3}B{?}C{?}D", "", "x", "C{?}D");
+
+        // an escaped brace does not start a placeholder
+        VERIFY2("{?1}{{?}}A{?}B", "", "", "B");
+        VERIFY2("{?1}{{?2}A{?}B", "", "x", "B");
+    }
+
     {
         VERIFY("output({1:3})", "\e[1m12345", "output(\e[1m123)");
         VERIFY("output({1:-3})", "\e[1m12345", "output(\e[1m123…)");