From 3df2f78c9e85c964a2175a870ac4d31218914776 Mon Sep 17 00:00:00 2001
From: Tako Schotanus <tako@codejive.org>
Date: Wed, 18 Mar 2026 12:32:56 +0100
Subject: [PATCH] fix: minor Markup fixes and refactorings

---
 .../org/codejive/twinkle/fluent/Fluent.java   |  4 +-
 .../org/codejive/twinkle/fluent/Markup.java   | 13 ----
 .../codejive/twinkle/fluent/MarkupParser.java |  7 ++
 .../fluent/commands/TextualCommands.java      |  8 +++
 ...ltMarkup.java => DefaultMarkupParser.java} | 64 ++++++++-----------
 .../twinkle/fluent/impl/FluentImpl.java       | 21 ++++--
 ...rkup.java => TestDefaultMarkupParser.java} | 58 +++++++++--------
 .../codejive/twinkle/fluent/TestFluent.java   | 43 +++++++++++++
 8 files changed, 133 insertions(+), 85 deletions(-)
 delete mode 100644 twinkle-text/src/main/java/org/codejive/twinkle/fluent/Markup.java
 create mode 100644 twinkle-text/src/main/java/org/codejive/twinkle/fluent/MarkupParser.java
 rename twinkle-text/src/main/java/org/codejive/twinkle/fluent/impl/{DefaultMarkup.java => DefaultMarkupParser.java} (83%)
 rename twinkle-text/src/test/java/org/codejive/twinkle/fluent/{TestDefaultMarkup.java => TestDefaultMarkupParser.java} (89%)

diff --git a/twinkle-text/src/main/java/org/codejive/twinkle/fluent/Fluent.java b/twinkle-text/src/main/java/org/codejive/twinkle/fluent/Fluent.java
index a9defa7..74bfa4b 100644
--- a/twinkle-text/src/main/java/org/codejive/twinkle/fluent/Fluent.java
+++ b/twinkle-text/src/main/java/org/codejive/twinkle/fluent/Fluent.java
@@ -58,10 +58,10 @@ static Fluent of(Appendable appendable, Style startingStyle) {
      * Sets the Markup parser to use for parsing markup patterns in text. This is useful for
      * allowing users to define their own custom markup syntax.
      *
-     * @param markup the Markup parser to use
+     * @param markupParser the Markup parser to use
      * @return this Fluent instance for chaining
      */
-    Fluent markupParser(Markup markup);
+    Fluent markupParser(MarkupParser markupParser);
 
     /**
      * Returns the current style combining the styles that have been applied since the creation of
diff --git a/twinkle-text/src/main/java/org/codejive/twinkle/fluent/Markup.java b/twinkle-text/src/main/java/org/codejive/twinkle/fluent/Markup.java
deleted file mode 100644
index edae91f..0000000
--- a/twinkle-text/src/main/java/org/codejive/twinkle/fluent/Markup.java
+++ /dev/null
@@ -1,13 +0,0 @@
-package org.codejive.twinkle.fluent;
-
-/** An interface for possible markup parser implementations that can be used for the `Fluent` */
-public interface Markup {
-
-    default String parse(String textWithMarkup) {
-        StringBuilder sb = new StringBuilder();
-        parse(sb, textWithMarkup);
-        return sb.toString();
-    }
-
-    void parse(Appendable appendable, String textWithMarkup);
-}
diff --git a/twinkle-text/src/main/java/org/codejive/twinkle/fluent/MarkupParser.java b/twinkle-text/src/main/java/org/codejive/twinkle/fluent/MarkupParser.java
new file mode 100644
index 0000000..ac64ed3
--- /dev/null
+++ b/twinkle-text/src/main/java/org/codejive/twinkle/fluent/MarkupParser.java
@@ -0,0 +1,7 @@
+package org.codejive.twinkle.fluent;
+
+/** An interface for possible markup parser implementations that can be used for the `Fluent` */
+public interface MarkupParser {
+
+    void parse(Fluent fluent, String textWithMarkup);
+}
diff --git a/twinkle-text/src/main/java/org/codejive/twinkle/fluent/commands/TextualCommands.java b/twinkle-text/src/main/java/org/codejive/twinkle/fluent/commands/TextualCommands.java
index 04a49e1..caf9024 100644
--- a/twinkle-text/src/main/java/org/codejive/twinkle/fluent/commands/TextualCommands.java
+++ b/twinkle-text/src/main/java/org/codejive/twinkle/fluent/commands/TextualCommands.java
@@ -27,6 +27,14 @@ default Fluent text(@NonNull Printable prt) {
      */
     Fluent text(@NonNull Object obj, Object... args);
 
+    /**
+     * Writes the given string
+     *
+     * @param text the text to output
+     * @return this Fluent instance for chaining
+     */
+    Fluent plain(@NonNull String text);
+
     Fluent markup(@NonNull Object obj, Object... args);
 
     /**
diff --git a/twinkle-text/src/main/java/org/codejive/twinkle/fluent/impl/DefaultMarkup.java b/twinkle-text/src/main/java/org/codejive/twinkle/fluent/impl/DefaultMarkupParser.java
similarity index 83%
rename from twinkle-text/src/main/java/org/codejive/twinkle/fluent/impl/DefaultMarkup.java
rename to twinkle-text/src/main/java/org/codejive/twinkle/fluent/impl/DefaultMarkupParser.java
index 759bb75..32e0cda 100644
--- a/twinkle-text/src/main/java/org/codejive/twinkle/fluent/impl/DefaultMarkup.java
+++ b/twinkle-text/src/main/java/org/codejive/twinkle/fluent/impl/DefaultMarkupParser.java
@@ -6,7 +6,8 @@
 import java.util.regex.Pattern;
 import org.codejive.twinkle.ansi.Color;
 import org.codejive.twinkle.ansi.Hyperlink;
-import org.codejive.twinkle.fluent.Markup;
+import org.codejive.twinkle.fluent.Fluent;
+import org.codejive.twinkle.fluent.MarkupParser;
 import org.codejive.twinkle.fluent.commands.ColorCommands;
 import org.codejive.twinkle.fluent.commands.NegatableCommands;
 
@@ -16,46 +17,40 @@
  * like `{red}` applying a red color. While others can have parameters, like `{~10,5}` moving the
  * cursor to column 10 and row 5.
  */
-public class DefaultMarkup implements Markup {
-    protected final FluentImpl fluent;
-
+public class DefaultMarkupParser implements MarkupParser {
     private static Map<String, Color.BasicColor> colors;
 
     private static final Pattern markupPattern = Pattern.compile("(?<!\\{)\\{([^{}]*)}");
 
-    public DefaultMarkup(FluentImpl fluent) {
-        this.fluent = fluent;
-    }
-
     @Override
-    public void parse(Appendable appendable, String textWithMarkup) {
+    public void parse(Fluent fluent, String textWithMarkup) {
         // Call handleMarkup() for each markup pattern found in the text
         int lastIndex = 0;
         Matcher matcher = markupPattern.matcher(textWithMarkup);
         while (matcher.find()) {
             // Append text before the markup
             if (matcher.start() > lastIndex) {
-                append(appendable, textWithMarkup.substring(lastIndex, matcher.start()));
+                append(fluent, textWithMarkup.substring(lastIndex, matcher.start()));
             }
             // Handle the markup content
             String markupContent = matcher.group(1);
-            handleMarkup(appendable, markupContent);
+            handleMarkup(fluent, markupContent);
             lastIndex = matcher.end();
         }
         // Append any remaining text after the last markup
         if (lastIndex < textWithMarkup.length()) {
-            append(appendable, textWithMarkup.substring(lastIndex, textWithMarkup.length()));
+            append(fluent, textWithMarkup.substring(lastIndex, textWithMarkup.length()));
         }
     }
 
-    protected void handleMarkup(Appendable appendable, String markup) {
-        if (tryStyles(markup, fluent)) {
+    protected void handleMarkup(Fluent fluent, String markup) {
+        if (tryStyles(fluent, markup)) {
             return;
         }
 
         if (markup.startsWith("/") && markup.length() > 1) {
             String closeMarkup = markup.substring(1);
-            if (tryStyles(closeMarkup, fluent.not())) {
+            if (tryStyles(fluent.not(), closeMarkup)) {
                 return;
             }
         }
@@ -81,26 +76,20 @@ protected void handleMarkup(Appendable appendable, String markup) {
                 return;
         }
 
-        if (tryPosition(markup)) {
+        if (tryPosition(fluent, markup)) {
             return;
         }
 
-        Hyperlink link = tryHyperlink(markup);
-        if (link != null) {
-            if (link == Hyperlink.END) {
-                fluent.lru();
-            } else {
-                fluent.url(link.url, link.id);
-            }
+        if (tryHyperlink(fluent, markup)) {
             return;
         }
 
-        if (tryColors(markup)) {
+        if (tryColors(fluent, markup)) {
             return;
         }
     }
 
-    private boolean tryStyles(String markup, NegatableCommands fluentNeg) {
+    private boolean tryStyles(NegatableCommands fluentNeg, String markup) {
         switch (markup.toLowerCase()) {
             case "bold":
             case "b":
@@ -135,7 +124,7 @@ private boolean tryStyles(String markup, NegatableCommands fluentNeg) {
         return false;
     }
 
-    private boolean tryPosition(String markup) {
+    private boolean tryPosition(Fluent fluent, String markup) {
         switch (markup.toLowerCase()) {
             case "mark":
             case "@":
@@ -217,24 +206,27 @@ private int parsePos(String pos, int max) throws NumberFormatException {
         return Integer.parseInt(pos.trim());
     }
 
-    private Hyperlink tryHyperlink(String markup) {
+    private boolean tryHyperlink(Fluent fluent, String markup) {
         if (markup.equals("/")) {
-            return Hyperlink.END;
+            fluent.lru();
+            return true;
         } else if (markup.startsWith("http://") || markup.startsWith("https://")) {
-            return Hyperlink.of(markup);
+            Hyperlink link = Hyperlink.of(markup);
+            fluent.url(link.url, link.id);
+            return true;
         }
-        return null;
+        return false;
     }
 
-    private boolean tryColors(String markup) {
+    private boolean tryColors(Fluent fluent, String markup) {
         if (markup.startsWith("bg:")) {
-            return tryColors(markup.substring(3), fluent.bg());
+            return tryColorsFgBg(fluent.bg(), markup.substring(3));
         } else {
-            return tryColors(markup, fluent);
+            return tryColorsFgBg(fluent, markup);
         }
     }
 
-    private boolean tryColors(String markup, ColorCommands fluentColors) {
+    private boolean tryColorsFgBg(ColorCommands fluentColors, String markup) {
         // Try #RRGGBB format first
         Color color = tryRgbColor(markup);
         if (color == null) {
@@ -289,9 +281,9 @@ private Color tryColorByName(String markup) {
         return colors.get(lmarkup);
     }
 
-    protected void append(Appendable appendable, String text) {
+    protected void append(Fluent fluent, String text) {
         try {
-            appendable.append(text);
+            fluent.plain(text);
         } catch (Exception e) {
             // We simply ignore errors
         }
diff --git a/twinkle-text/src/main/java/org/codejive/twinkle/fluent/impl/FluentImpl.java b/twinkle-text/src/main/java/org/codejive/twinkle/fluent/impl/FluentImpl.java
index 9113d35..72935bc 100644
--- a/twinkle-text/src/main/java/org/codejive/twinkle/fluent/impl/FluentImpl.java
+++ b/twinkle-text/src/main/java/org/codejive/twinkle/fluent/impl/FluentImpl.java
@@ -13,7 +13,7 @@
 import org.codejive.twinkle.ansi.util.AnsiTricks;
 import org.codejive.twinkle.ansi.util.Printable;
 import org.codejive.twinkle.fluent.Fluent;
-import org.codejive.twinkle.fluent.Markup;
+import org.codejive.twinkle.fluent.MarkupParser;
 import org.codejive.twinkle.fluent.commands.ColorCommands;
 import org.codejive.twinkle.fluent.commands.LineCommands;
 import org.codejive.twinkle.fluent.commands.NegatableCommands;
@@ -33,7 +33,7 @@ public class FluentImpl implements Fluent {
     protected final Style startingStyle;
     protected Style currentStyle;
     protected Deque<Style> styleStack;
-    protected Markup markup;
+    protected MarkupParser markupParser;
 
     public static FluentImpl of(Appendable appendable, Style startingStyle) {
         return new FluentImpl(appendable, startingStyle);
@@ -48,12 +48,12 @@ protected FluentImpl(Appendable appendable, Style startingStyle) {
         this.startingStyle = startingStyle;
         this.currentStyle = startingStyle;
         this.styleStack = new ArrayDeque<>();
-        this.markup = new DefaultMarkup(this);
+        this.markupParser = new DefaultMarkupParser();
     }
 
     @Override
-    public Fluent markupParser(Markup markup) {
-        this.markup = markup;
+    public Fluent markupParser(MarkupParser markupParser) {
+        this.markupParser = markupParser;
         return this;
     }
 
@@ -100,10 +100,19 @@ public FluentImpl text(@NonNull Object obj, Object... args) {
         return append(String.format(String.valueOf(obj), args));
     }
 
+    @Override
+    public FluentImpl plain(@NonNull String text) {
+        return append(text);
+    }
+
     @Override
     public Fluent markup(@NonNull Object obj, Object... args) {
+        StringBuilder sb = new StringBuilder();
+        FluentImpl f = new FluentImpl(sb, currentStyle);
+        f.markupParser.parse(f, obj.toString());
+
         Formatter fmt = new Formatter(appendable);
-        fmt.format(markup.parse(obj.toString()), args);
+        fmt.format(sb.toString(), args);
         return this;
     }
 
diff --git a/twinkle-text/src/test/java/org/codejive/twinkle/fluent/TestDefaultMarkup.java b/twinkle-text/src/test/java/org/codejive/twinkle/fluent/TestDefaultMarkupParser.java
similarity index 89%
rename from twinkle-text/src/test/java/org/codejive/twinkle/fluent/TestDefaultMarkup.java
rename to twinkle-text/src/test/java/org/codejive/twinkle/fluent/TestDefaultMarkupParser.java
index 788004a..224c3c4 100644
--- a/twinkle-text/src/test/java/org/codejive/twinkle/fluent/TestDefaultMarkup.java
+++ b/twinkle-text/src/test/java/org/codejive/twinkle/fluent/TestDefaultMarkupParser.java
@@ -5,19 +5,19 @@
 import org.codejive.twinkle.ansi.Ansi;
 import org.codejive.twinkle.ansi.Color;
 import org.codejive.twinkle.ansi.Style;
-import org.codejive.twinkle.fluent.impl.DefaultMarkup;
+import org.codejive.twinkle.fluent.impl.DefaultMarkupParser;
 import org.codejive.twinkle.fluent.impl.FluentImpl;
 import org.junit.jupiter.api.Test;
 
 /**
- * Tests for {@link DefaultMarkup}.
+ * Tests for {@link DefaultMarkupParser}.
  *
  * <p>Each test creates a {@link FluentImpl} backed by a {@link StringBuilder} and passes it to a
- * {@link DefaultMarkup} instance. The same {@link StringBuilder} is used as the target appendable
- * for {@code parse()}, so plain text and ANSI escape sequences land in the same buffer in the
- * correct order.
+ * {@link DefaultMarkupParser} instance. The same {@link StringBuilder} is used as the target
+ * appendable for {@code parse()}, so plain text and ANSI escape sequences land in the same buffer
+ * in the correct order.
  */
-public class TestDefaultMarkup {
+public class TestDefaultMarkupParser {
 
     /**
      * Creates a fresh FluentImpl + DefaultMarkup pair that both write to the returned
@@ -26,10 +26,10 @@ public class TestDefaultMarkup {
     private static class Setup {
         final StringBuilder sb = new StringBuilder();
         final FluentImpl fluent = FluentImpl.of(sb, Style.DEFAULT);
-        final DefaultMarkup markup = new DefaultMarkup(fluent);
+        final DefaultMarkupParser markup = new DefaultMarkupParser();
 
         void parse(String text) {
-            markup.parse(sb, text);
+            markup.parse(fluent, text);
         }
 
         String result() {
@@ -193,10 +193,6 @@ public void testItalicOffMarkup() {
         assertThat(s.result()).isEqualTo(Ansi.italic() + "on" + Ansi.italicOff() + "off");
     }
 
-    // -------------------------------------------------------------------------
-    // Color markup
-    // -------------------------------------------------------------------------
-
     @Test
     public void testRedColorMarkup() {
         Setup s = new Setup();
@@ -273,6 +269,21 @@ public void testMultipleMarkupInOneString() {
 
     @Test
     public void testMultipleMarkupInOneString2() {
+        Setup s = new Setup();
+        s.parse("A{brightmagenta}B{white}C{brightmagenta}D");
+        assertThat(s.result())
+                .isEqualTo(
+                        "A"
+                                + Color.BasicColor.MAGENTA.bright().toAnsiFg()
+                                + "B"
+                                + Color.BasicColor.WHITE.toAnsiFg()
+                                + "C"
+                                + Color.BasicColor.MAGENTA.bright().toAnsiFg()
+                                + "D");
+    }
+
+    @Test
+    public void testMarkupPushPopInOneString() {
         Setup s = new Setup();
         s.parse("A{brightmagenta}{+}B{white}C{-}D");
         assertThat(s.result())
@@ -313,10 +324,6 @@ public void testOverlappingCloseTags() {
                                 + Ansi.italicOff());
     }
 
-    // -------------------------------------------------------------------------
-    // Cursor position markup
-    // -------------------------------------------------------------------------
-
     @Test
     public void testPositionMarkup() {
         Setup s = new Setup();
@@ -373,10 +380,6 @@ public void testJumpCursorCaretShorthand() {
         assertThat(s.result()).isEqualTo(Ansi.cursorRestore());
     }
 
-    // -------------------------------------------------------------------------
-    // Hyperlink markup
-    // -------------------------------------------------------------------------
-
     @Test
     public void testHyperlinkMarkup() {
         Setup s = new Setup();
@@ -392,10 +395,6 @@ public void testHyperlinkEndOnlyMarkup() {
         assertThat(s.result()).isEqualTo(Ansi.linkEnd());
     }
 
-    // -------------------------------------------------------------------------
-    // Push / Pop markup
-    // -------------------------------------------------------------------------
-
     @Test
     public void testPushPlusShorthand() {
         Setup s = new Setup();
@@ -415,10 +414,6 @@ public void testPushPopKeywords() {
         assertThat(s.fluent.style()).isEqualTo(styleBeforePush);
     }
 
-    // -------------------------------------------------------------------------
-    // Unknown / invalid markup is silently ignored
-    // -------------------------------------------------------------------------
-
     @Test
     public void testUnknownMarkupIsIgnored() {
         Setup s = new Setup();
@@ -439,4 +434,11 @@ public void testInvalidPositionIsIgnored() {
         s.parse("{~abc,def}text");
         assertThat(s.result()).isEqualTo("text");
     }
+
+    @Test
+    public void testTextWithFormatting() {
+        Setup s = new Setup();
+        s.parse("{i}%s{/i}");
+        assertThat(s.result()).isEqualTo(Ansi.italic() + "%s" + Ansi.italicOff());
+    }
 }
diff --git a/twinkle-text/src/test/java/org/codejive/twinkle/fluent/TestFluent.java b/twinkle-text/src/test/java/org/codejive/twinkle/fluent/TestFluent.java
index ac98281..0cb3ec8 100644
--- a/twinkle-text/src/test/java/org/codejive/twinkle/fluent/TestFluent.java
+++ b/twinkle-text/src/test/java/org/codejive/twinkle/fluent/TestFluent.java
@@ -2,6 +2,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.util.ArrayList;
 import org.codejive.twinkle.ansi.Ansi;
 import org.codejive.twinkle.ansi.Color;
 import org.codejive.twinkle.ansi.Constants;
@@ -560,4 +561,46 @@ public void testStringFluent() {
         stringFluent.bold().text("Hello").not().bold();
         assertThat(stringFluent.toString()).isEqualTo(Ansi.bold() + "Hello" + Ansi.normal());
     }
+
+    @Test
+    public void testFormatMarkupOrdering() {
+        Fluent stringFluent = Fluent.string();
+        // Markup is applied first, then formatting, so markup is invalid and should be ignored
+        stringFluent.markup("{%s}", "bold");
+        assertThat(stringFluent.toString()).isEqualTo("");
+    }
+
+    @Test
+    public void testMarkupAppendable() {
+        RecordingAppendable recapp = new RecordingAppendable();
+        Fluent fluent = Fluent.of(recapp);
+        String txt = "this could be a very large string";
+        fluent.markup("{i}%s{/i}", txt);
+        assertThat(recapp.calls).hasSize(3);
+        assertThat(recapp.calls.get(0)).isEqualTo(Ansi.italic());
+        assertThat(recapp.calls.get(1)).isEqualTo(txt);
+        assertThat(recapp.calls.get(2)).isEqualTo(Ansi.italicOff());
+    }
+
+    public static class RecordingAppendable implements Appendable {
+        public ArrayList<CharSequence> calls = new ArrayList<>();
+
+        @Override
+        public Appendable append(CharSequence csq) {
+            calls.add(csq);
+            return this;
+        }
+
+        @Override
+        public Appendable append(CharSequence csq, int start, int end) {
+            calls.add(csq.subSequence(start, end));
+            return this;
+        }
+
+        @Override
+        public Appendable append(char c) {
+            calls.add("" + c);
+            return this;
+        }
+    }
 }