diff --git a/src/terminal/Parser.zig b/src/terminal/Parser.zig index a779c3350..bc5859ede 100644 --- a/src/terminal/Parser.zig +++ b/src/terminal/Parser.zig @@ -86,7 +86,9 @@ pub const Action = union(enum) { final: u8, /// The list of separators used for CSI params. The value of the - /// bit can be mapped to Sep. + /// bit can be mapped to Sep. The index of this bit set specifies + /// the separator AFTER that param. For example: 0;4:3 would have + /// index 1 set. pub const SepList = std.StaticBitSet(MAX_PARAMS); /// The separator used for CSI params. @@ -192,7 +194,19 @@ pub const Action = union(enum) { /// 4 because we also use the intermediates array for UTF8 decoding which /// can be at most 4 bytes. const MAX_INTERMEDIATE = 4; -const MAX_PARAMS = 16; + +/// Maximum number of CSI parameters. This is arbitrary. Practically, the +/// only CSI command that uses more than 3 parameters is the SGR command +/// which can be infinitely long. 24 is a reasonable limit based on empirical +/// data. This used to be 16 but Kakoune has a SGR command that uses 17 +/// parameters. +/// +/// We could in the future make this the static limit and then allocate after +/// but that's a lot more work and practically its so rare to exceed this +/// number. I implore TUI authors to not use more than this number of CSI +/// params, but I suspect we'll introduce a slow path with heap allocation +/// one day. +const MAX_PARAMS = 24; /// Current state of the state machine state: State = .ground, @@ -689,6 +703,64 @@ test "csi: SGR mixed colon and semicolon with blank" { } } +// This is from a Kakoune actual SGR sequence also. +test "csi: SGR mixed colon and semicolon setting underline, bg, fg" { + var p = init(); + _ = p.next(0x1B); + for ("[4:3;38;2;51;51;51;48;2;170;170;170;58;2;255;97;136") |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); + + const d = a[1].?.csi_dispatch; + try testing.expect(d.final == 'm'); + try testing.expectEqual(17, d.params.len); + try testing.expectEqual(@as(u16, 4), d.params[0]); + try testing.expect(d.params_sep.isSet(0)); + try testing.expectEqual(@as(u16, 3), d.params[1]); + try testing.expect(!d.params_sep.isSet(1)); + try testing.expectEqual(@as(u16, 38), d.params[2]); + try testing.expect(!d.params_sep.isSet(2)); + try testing.expectEqual(@as(u16, 2), d.params[3]); + try testing.expect(!d.params_sep.isSet(3)); + try testing.expectEqual(@as(u16, 51), d.params[4]); + try testing.expect(!d.params_sep.isSet(4)); + try testing.expectEqual(@as(u16, 51), d.params[5]); + try testing.expect(!d.params_sep.isSet(5)); + try testing.expectEqual(@as(u16, 51), d.params[6]); + try testing.expect(!d.params_sep.isSet(6)); + try testing.expectEqual(@as(u16, 48), d.params[7]); + try testing.expect(!d.params_sep.isSet(7)); + try testing.expectEqual(@as(u16, 2), d.params[8]); + try testing.expect(!d.params_sep.isSet(8)); + try testing.expectEqual(@as(u16, 170), d.params[9]); + try testing.expect(!d.params_sep.isSet(9)); + try testing.expectEqual(@as(u16, 170), d.params[10]); + try testing.expect(!d.params_sep.isSet(10)); + try testing.expectEqual(@as(u16, 170), d.params[11]); + try testing.expect(!d.params_sep.isSet(11)); + try testing.expectEqual(@as(u16, 58), d.params[12]); + try testing.expect(!d.params_sep.isSet(12)); + try testing.expectEqual(@as(u16, 2), d.params[13]); + try testing.expect(!d.params_sep.isSet(13)); + try testing.expectEqual(@as(u16, 255), d.params[14]); + try testing.expect(!d.params_sep.isSet(14)); + try testing.expectEqual(@as(u16, 97), d.params[15]); + try testing.expect(!d.params_sep.isSet(15)); + try testing.expectEqual(@as(u16, 136), d.params[16]); + try testing.expect(!d.params_sep.isSet(16)); + } +} + test "csi: colon for non-m final" { var p = init(); _ = p.next(0x1B); diff --git a/src/terminal/sgr.zig b/src/terminal/sgr.zig index 52bfb2c31..2bc32c5f9 100644 --- a/src/terminal/sgr.zig +++ b/src/terminal/sgr.zig @@ -103,12 +103,16 @@ pub const Parser = struct { /// Next returns the next attribute or null if there are no more attributes. pub fn next(self: *Parser) ?Attribute { - if (self.idx > self.params.len) return null; + if (self.idx >= self.params.len) { + // If we're at index zero it means we must have an empty + // list and an empty list implicitly means unset. + if (self.idx == 0) { + // Add one to ensure we don't loop on unset + self.idx += 1; + return .unset; + } - // Implicitly means unset - if (self.params.len == 0) { - self.idx += 1; - return .unset; + return null; } const slice = self.params[self.idx..self.params.len]; @@ -788,7 +792,6 @@ test "sgr: direct fg colon with colorspace and extra param" { { const v = p.next().?; - std.log.warn("WHAT={}", .{v}); try testing.expect(v == .direct_color_fg); try testing.expectEqual(@as(u8, 1), v.direct_color_fg.r); try testing.expectEqual(@as(u8, 2), v.direct_color_fg.g); @@ -864,3 +867,50 @@ test "sgr: kakoune input" { //try testing.expect(p.next() == null); } + +// Discussion #5930, another input sent by kakoune +test "sgr: kakoune input issue underline, fg, and bg" { + // echo -e "\033[4:3;38;2;51;51;51;48;2;170;170;170;58;2;255;97;136mset everything in one sequence, broken\033[m" + + // This used to crash + var p: Parser = .{ + .params = &[_]u16{ 4, 3, 38, 2, 51, 51, 51, 48, 2, 170, 170, 170, 58, 2, 255, 97, 136 }, + .params_sep = sep: { + var list = SepList.initEmpty(); + list.set(0); + break :sep list; + }, + }; + + { + const v = p.next().?; + try testing.expect(v == .underline); + try testing.expectEqual(Attribute.Underline.curly, v.underline); + } + + { + const v = p.next().?; + try testing.expect(v == .direct_color_fg); + try testing.expectEqual(@as(u8, 51), v.direct_color_fg.r); + try testing.expectEqual(@as(u8, 51), v.direct_color_fg.g); + try testing.expectEqual(@as(u8, 51), v.direct_color_fg.b); + } + + { + const v = p.next().?; + try testing.expect(v == .direct_color_bg); + try testing.expectEqual(@as(u8, 170), v.direct_color_bg.r); + try testing.expectEqual(@as(u8, 170), v.direct_color_bg.g); + try testing.expectEqual(@as(u8, 170), v.direct_color_bg.b); + } + + { + const v = p.next().?; + try testing.expect(v == .underline_color); + try testing.expectEqual(@as(u8, 255), v.underline_color.r); + try testing.expectEqual(@as(u8, 97), v.underline_color.g); + try testing.expectEqual(@as(u8, 136), v.underline_color.b); + } + + try testing.expect(p.next() == null); +} diff --git a/src/terminal/stream.zig b/src/terminal/stream.zig index eb5ab2c65..3d9ed72fb 100644 --- a/src/terminal/stream.zig +++ b/src/terminal/stream.zig @@ -932,7 +932,7 @@ pub fn Stream(comptime Handler: type) type { // SGR - Select Graphic Rendition 'm' => switch (input.intermediates.len) { 0 => if (@hasDecl(T, "setAttribute")) { - // log.info("parse SGR params={any}", .{action.params}); + // log.info("parse SGR params={any}", .{input.params}); var p: sgr.Parser = .{ .params = input.params, .params_sep = input.params_sep,