From 60d4024d6448fb53ea34c7f7e24d001871a8d157 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Sat, 24 Jun 2023 15:16:54 -0700
Subject: [PATCH] terminal: reset CSI param separator in parser on clear

---
 src/terminal/Parser.zig  | 67 ++++++++++++++++++++++++++++++++++------
 src/terminfo/ghostty.zig |  4 +--
 2 files changed, 58 insertions(+), 13 deletions(-)

diff --git a/src/terminal/Parser.zig b/src/terminal/Parser.zig
index abfbc0efb..07466e19e 100644
--- a/src/terminal/Parser.zig
+++ b/src/terminal/Parser.zig
@@ -382,15 +382,7 @@ fn doAction(self: *Parser, action: TransitionAction, c: u8) ?Action {
                 self.params_idx += 1;
             }
 
-            // We only allow the colon separator for the 'm' command.
-            switch (self.params_sep) {
-                .none => {},
-                .semicolon => {},
-                .colon => if (c != 'm') break :csi_dispatch null,
-                .mixed => break :csi_dispatch null,
-            }
-
-            break :csi_dispatch Action{
+            const result: Action = .{
                 .csi_dispatch = .{
                     .intermediates = self.intermediates[0..self.intermediates_idx],
                     .params = self.params[0..self.params_idx],
@@ -398,10 +390,35 @@ fn doAction(self: *Parser, action: TransitionAction, c: u8) ?Action {
                     .sep = switch (self.params_sep) {
                         .none, .semicolon => .semicolon,
                         .colon => .colon,
-                        .mixed => unreachable,
+
+                        // This should never happen because of the checks below
+                        // but we have to exhaustively handle the switch.
+                        .mixed => .semicolon,
                     },
                 },
             };
+
+            // We only allow the colon separator for the 'm' command.
+            switch (self.params_sep) {
+                .none => {},
+                .semicolon => {},
+                .colon => if (c != 'm') {
+                    log.warn(
+                        "CSI colon separator only allowed for 'm' command, got: {}",
+                        .{result},
+                    );
+                    break :csi_dispatch null;
+                },
+                .mixed => {
+                    log.warn(
+                        "CSI command had mixed colons and semicolons, got: {}",
+                        .{result},
+                    );
+                    break :csi_dispatch null;
+                },
+            }
+
+            break :csi_dispatch result;
         },
         .esc_dispatch => Action{
             .esc_dispatch = .{
@@ -418,6 +435,7 @@ fn doAction(self: *Parser, action: TransitionAction, c: u8) ?Action {
 fn clear(self: *Parser) void {
     self.intermediates_idx = 0;
     self.params_idx = 0;
+    self.params_sep = .none;
     self.param_acc = 0;
     self.param_acc_idx = 0;
 }
@@ -531,6 +549,35 @@ test "csi: SGR ESC [ 38 : 2 m" {
     }
 }
 
+test "csi: SGR colon followed by semicolon" {
+    var p = init();
+    _ = p.next(0x1B);
+    for ("[48:2") |c| {
+        const a = p.next(c);
+        try testing.expect(a[0] == null);
+        try testing.expect(a[1] == null);
+        try testing.expect(a[2] == null);
+    }
+
+    {
+        const a = p.next('m');
+        try testing.expect(p.state == .ground);
+        try testing.expect(a[0] == null);
+        try testing.expect(a[1].? == .csi_dispatch);
+        try testing.expect(a[2] == null);
+    }
+
+    _ = p.next(0x1B);
+    _ = p.next('[');
+    {
+        const a = p.next('H');
+        try testing.expect(p.state == .ground);
+        try testing.expect(a[0] == null);
+        try testing.expect(a[1].? == .csi_dispatch);
+        try testing.expect(a[2] == null);
+    }
+}
+
 test "csi: SGR ESC [ 48 : 2 m" {
     var p = init();
     _ = p.next(0x1B);
diff --git a/src/terminfo/ghostty.zig b/src/terminfo/ghostty.zig
index 865210155..2e37bfc64 100644
--- a/src/terminfo/ghostty.zig
+++ b/src/terminfo/ghostty.zig
@@ -155,9 +155,7 @@ pub const ghostty: Source = .{
         .{ .name = "rmxx", .value = .{ .string = "\\E[29m" } },
         .{ .name = "setab", .value = .{ .string = "\\E[%?%p1%{8}%<%t4%p1%d%e%p1%{16}%<%t10%p1%{8}%-%d%e48;5;%p1%d%;m" } },
         .{ .name = "setaf", .value = .{ .string = "\\E[%?%p1%{8}%<%t3%p1%d%e%p1%{16}%<%t9%p1%{8}%-%d%e38;5;%p1%d%;m" } },
-        .{ .name = "setrgbb", .value = .{ .string = "\\E[48;2;%p1%d;%p2%d;%p3%dm" } },
-        // This causes weird rendering issues, why?
-        //.{ .name = "setrgbb", .value = .{ .string = "\\E[48:2:%p1%d:%p2%d:%p3%dm" } },
+        .{ .name = "setrgbb", .value = .{ .string = "\\E[48:2:%p1%d:%p2%d:%p3%dm" } },
         .{ .name = "setrgbf", .value = .{ .string = "\\E[38:2:%p1%d:%p2%d:%p3%dm" } },
         .{ .name = "sgr", .value = .{ .string = "%?%p9%t\\E(0%e\\E(B%;\\E[0%?%p6%t;1%;%?%p2%t;4%;%?%p1%p3%|%t;7%;%?%p4%t;5%;%?%p7%t;8%;m" } },
         .{ .name = "sgr0", .value = .{ .string = "\\E(B\\E[m" } },