From 28a22fc07f50bc097a6bd0ad1d5f8c2e463a77bb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Jun 2023 09:24:07 -0700 Subject: [PATCH 1/5] various tests to ensure we parse curly underlines correctly --- src/terminal/Parser.zig | 55 +++++++++++++++++++++++++++++++++++++++++ src/terminal/sgr.zig | 6 +++++ src/terminal/stream.zig | 5 +++- src/termio/Exec.zig | 2 +- 4 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/terminal/Parser.zig b/src/terminal/Parser.zig index 8e55ba564..0c3d8ca73 100644 --- a/src/terminal/Parser.zig +++ b/src/terminal/Parser.zig @@ -524,12 +524,67 @@ test "csi: SGR ESC [ 38 : 2 m" { const d = a[1].?.csi_dispatch; try testing.expect(d.final == 'm'); + try testing.expect(d.sep == .colon); try testing.expect(d.params.len == 2); try testing.expectEqual(@as(u16, 38), d.params[0]); try testing.expectEqual(@as(u16, 2), d.params[1]); } } +test "csi: SGR ESC [4:3m colon" { + var p = init(); + _ = p.next(0x1B); + _ = p.next('['); + _ = p.next('4'); + _ = p.next(':'); + _ = p.next('3'); + + { + 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.expect(d.sep == .colon); + try testing.expect(d.params.len == 2); + try testing.expectEqual(@as(u16, 4), d.params[0]); + try testing.expectEqual(@as(u16, 3), d.params[1]); + } +} + +test "csi: SGR with many blank and colon" { + var p = init(); + _ = p.next(0x1B); + for ("[58:2::240:143:104") |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.expect(d.sep == .colon); + try testing.expect(d.params.len == 6); + try testing.expectEqual(@as(u16, 58), d.params[0]); + try testing.expectEqual(@as(u16, 2), d.params[1]); + try testing.expectEqual(@as(u16, 0), d.params[2]); + try testing.expectEqual(@as(u16, 240), d.params[3]); + try testing.expectEqual(@as(u16, 143), d.params[4]); + try testing.expectEqual(@as(u16, 104), d.params[5]); + } +} + test "csi: mixing semicolon/colon" { var p = init(); _ = p.next(0x1B); diff --git a/src/terminal/sgr.zig b/src/terminal/sgr.zig index eda08ac8c..5d57162e8 100644 --- a/src/terminal/sgr.zig +++ b/src/terminal/sgr.zig @@ -344,6 +344,12 @@ test "sgr: underline styles" { try testing.expect(v.underline == .single); } + { + const v = testParseColon(&[_]u16{ 4, 3 }); + try testing.expect(v == .underline); + try testing.expect(v.underline == .curly); + } + { const v = testParseColon(&[_]u16{ 4, 4 }); try testing.expect(v == .underline); diff --git a/src/terminal/stream.zig b/src/terminal/stream.zig index f36fa6315..b121f123e 100644 --- a/src/terminal/stream.zig +++ b/src/terminal/stream.zig @@ -385,7 +385,10 @@ pub fn Stream(comptime Handler: type) type { // SGR - Select Graphic Rendition 'm' => if (@hasDecl(T, "setAttribute")) { var p: sgr.Parser = .{ .params = action.params, .colon = action.sep == .colon }; - while (p.next()) |attr| try self.handler.setAttribute(attr); + while (p.next()) |attr| { + //log.info("SGR attribute: {}", .{attr}); + try self.handler.setAttribute(attr); + } } else log.warn("unimplemented CSI callback: {}", .{action}), // CPR - Request Cursor Postion Report diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index 0f513c0ba..80ea35348 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -1081,7 +1081,7 @@ const StreamHandler = struct { pub fn setAttribute(self: *StreamHandler, attr: terminal.Attribute) !void { switch (attr) { - .unknown => |unk| log.warn("unimplemented or unknown attribute: {any}", .{unk}), + .unknown => |unk| log.warn("unimplemented or unknown SGR attribute: {any}", .{unk}), else => self.terminal.setAttribute(attr) catch |err| log.warn("error setting attribute {}: {}", .{ attr, err }), From b9bc61c0a40660a93f51dc9e5ea90c7428948d17 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Jun 2023 09:34:29 -0700 Subject: [PATCH 2/5] terminal: parse underline color sequences (but do not handle yet) --- src/terminal/Terminal.zig | 3 +++ src/terminal/sgr.zig | 56 +++++++++++++++++++++++++++++++++++++-- src/terminal/stream.zig | 2 +- 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 57f95841f..5a7622bae 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -461,6 +461,9 @@ pub fn setAttribute(self: *Terminal, attr: sgr.Attribute) !void { self.screen.cursor.pen.attrs.underline = .none; }, + // TODO + .underline_color, .reset_underline_color => {}, + .blink => { log.warn("blink requested, but not implemented", .{}); self.screen.cursor.pen.attrs.blink = true; diff --git a/src/terminal/sgr.zig b/src/terminal/sgr.zig index 5d57162e8..c7fcda035 100644 --- a/src/terminal/sgr.zig +++ b/src/terminal/sgr.zig @@ -33,6 +33,8 @@ pub const Attribute = union(enum) { /// Underline the text underline: Underline, reset_underline: void, + underline_color: RGB, + reset_underline_color: void, /// Blink the text blink: void, @@ -210,8 +212,12 @@ pub const Parser = struct { 48 => if (slice.len >= 5 and slice[1] == 2) { self.idx += 4; - // In the 6-len form, ignore the 3rd param. - const rgb = slice[2..5]; + // In the 6-len form, ignore the 3rd param. Otherwise, use it. + const rgb = if (slice.len == 5) slice[2..5] else rgb: { + // Consume one more element + self.idx += 1; + break :rgb slice[3..6]; + }; // We use @truncate because the value should be 0 to 255. If // it isn't, the behavior is undefined so we just... truncate it. @@ -231,6 +237,29 @@ pub const Parser = struct { 49 => return Attribute{ .reset_bg = {} }, + 58 => if (slice.len >= 5 and slice[1] == 2) { + self.idx += 4; + + // In the 6-len form, ignore the 3rd param. Otherwise, use it. + const rgb = if (slice.len == 5) slice[2..5] else rgb: { + // Consume one more element + self.idx += 1; + break :rgb slice[3..6]; + }; + + // We use @truncate because the value should be 0 to 255. If + // it isn't, the behavior is undefined so we just... truncate it. + return Attribute{ + .underline_color = .{ + .r = @truncate(u8, rgb[0]), + .g = @truncate(u8, rgb[1]), + .b = @truncate(u8, rgb[2]), + }, + }; + }, + + 59 => return Attribute{ .reset_underline_color = {} }, + 90...97 => return Attribute{ // 82 instead of 90 to offset to "bright" colors .@"8_bright_fg" = @intToEnum(color.Name, slice[0] - 82), @@ -437,3 +466,26 @@ test "sgr: 256 color" { try testing.expect(p.next().? == .@"256_fg"); try testing.expect(p.next().? == .@"256_bg"); } + +test "sgr: underline color" { + { + const v = testParseColon(&[_]u16{ 58, 2, 1, 2, 3 }); + try testing.expect(v == .underline_color); + try testing.expectEqual(@as(u8, 1), v.underline_color.r); + try testing.expectEqual(@as(u8, 2), v.underline_color.g); + try testing.expectEqual(@as(u8, 3), v.underline_color.b); + } + + { + const v = testParseColon(&[_]u16{ 58, 2, 0, 1, 2, 3 }); + try testing.expect(v == .underline_color); + try testing.expectEqual(@as(u8, 1), v.underline_color.r); + try testing.expectEqual(@as(u8, 2), v.underline_color.g); + try testing.expectEqual(@as(u8, 3), v.underline_color.b); + } +} + +test "sgr: reset underline color" { + var p: Parser = .{ .params = &[_]u16{59} }; + try testing.expect(p.next().? == .reset_underline_color); +} diff --git a/src/terminal/stream.zig b/src/terminal/stream.zig index b121f123e..93f3f5bc8 100644 --- a/src/terminal/stream.zig +++ b/src/terminal/stream.zig @@ -386,7 +386,7 @@ pub fn Stream(comptime Handler: type) type { 'm' => if (@hasDecl(T, "setAttribute")) { var p: sgr.Parser = .{ .params = action.params, .colon = action.sep == .colon }; while (p.next()) |attr| { - //log.info("SGR attribute: {}", .{attr}); + log.info("SGR attribute: {}", .{attr}); try self.handler.setAttribute(attr); } } else log.warn("unimplemented CSI callback: {}", .{action}), From 860209e968f0a2438280dba989586b9e691ba022 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Jun 2023 09:37:58 -0700 Subject: [PATCH 3/5] terminal: track underline color on cell --- src/terminal/Screen.zig | 9 ++++++++- src/terminal/Terminal.zig | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 03dece7c6..6bacf9ded 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -182,6 +182,12 @@ pub const Cell = struct { fg: color.RGB = .{}, bg: color.RGB = .{}, + /// Underline color. + /// NOTE(mitchellh): This is very rarely set so ideally we wouldn't waste + /// cell space for this. For now its on this struct because it is convenient + /// but we should consider a lookaside table for this. + underline_fg: color.RGB = .{}, + /// On/off attributes that can be set attrs: packed struct { has_bg: bool = false, @@ -194,6 +200,7 @@ pub const Cell = struct { inverse: bool = false, strikethrough: bool = false, underline: sgr.Attribute.Underline = .none, + underline_color: bool = false, /// True if this is a wide character. This char takes up /// two cells. The following cell ALWAYS is a space. @@ -265,7 +272,7 @@ pub const Cell = struct { test { //log.warn("CELL={} bits={} {}", .{ @sizeOf(Cell), @bitSizeOf(Cell), @alignOf(Cell) }); - try std.testing.expectEqual(12, @sizeOf(Cell)); + try std.testing.expectEqual(16, @sizeOf(Cell)); } }; diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 5a7622bae..0f0fef511 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -461,8 +461,18 @@ pub fn setAttribute(self: *Terminal, attr: sgr.Attribute) !void { self.screen.cursor.pen.attrs.underline = .none; }, - // TODO - .underline_color, .reset_underline_color => {}, + .underline_color => |rgb| { + self.screen.cursor.pen.attrs.underline_color = true; + self.screen.cursor.pen.underline_fg = .{ + .r = rgb.r, + .g = rgb.g, + .b = rgb.b, + }; + }, + + .reset_underline_color => { + self.screen.cursor.pen.attrs.underline_color = false; + }, .blink => { log.warn("blink requested, but not implemented", .{}); From 1c2451b5324228b39530db819f38076bb94a7be9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Jun 2023 09:39:52 -0700 Subject: [PATCH 4/5] renderer: render underline color if set --- src/renderer/Metal.zig | 4 +++- src/renderer/OpenGL.zig | 9 ++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig index 788bcb567..5e282675e 100644 --- a/src/renderer/Metal.zig +++ b/src/renderer/Metal.zig @@ -1023,11 +1023,13 @@ pub fn updateCell( null, ); + const color = if (cell.attrs.underline_color) cell.underline_fg else colors.fg; + self.cells.appendAssumeCapacity(.{ .mode = .fg, .grid_pos = .{ @intToFloat(f32, x), @intToFloat(f32, y) }, .cell_width = cell.widthLegacy(), - .color = .{ colors.fg.r, colors.fg.g, colors.fg.b, alpha }, + .color = .{ color.r, color.g, color.b, alpha }, .glyph_pos = .{ glyph.atlas_x, glyph.atlas_y }, .glyph_size = .{ glyph.width, glyph.height }, .glyph_offset = .{ glyph.offset_x, glyph.offset_y }, diff --git a/src/renderer/OpenGL.zig b/src/renderer/OpenGL.zig index df58137a7..91c1c8346 100644 --- a/src/renderer/OpenGL.zig +++ b/src/renderer/OpenGL.zig @@ -1181,6 +1181,9 @@ pub fn updateCell( @enumToInt(sprite), null, ); + + const color = if (cell.attrs.underline_color) cell.underline_fg else colors.fg; + self.cells.appendAssumeCapacity(.{ .mode = .fg, .grid_col = @intCast(u16, x), @@ -1192,9 +1195,9 @@ pub fn updateCell( .glyph_height = underline_glyph.height, .glyph_offset_x = underline_glyph.offset_x, .glyph_offset_y = underline_glyph.offset_y, - .fg_r = colors.fg.r, - .fg_g = colors.fg.g, - .fg_b = colors.fg.b, + .fg_r = color.r, + .fg_g = color.g, + .fg_b = color.b, .fg_a = alpha, .bg_r = 0, .bg_g = 0, From 86705a181a61e7aff20bffe0d378f7f15351e2bd Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Jun 2023 09:40:50 -0700 Subject: [PATCH 5/5] finalize comments --- TODO.md | 1 - src/terminal/stream.zig | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/TODO.md b/TODO.md index 37c53e7e2..308985ed2 100644 --- a/TODO.md +++ b/TODO.md @@ -30,4 +30,3 @@ Major Features: * Sixels: https://saitoha.github.io/libsixel/ * Kitty keyboard protocol: https://sw.kovidgoyal.net/kitty/keyboard-protocol/ * Kitty graphics protocol: https://sw.kovidgoyal.net/kitty/graphics-protocol/ -* Colored underlines: https://sw.kovidgoyal.net/kitty/underlines/ diff --git a/src/terminal/stream.zig b/src/terminal/stream.zig index 93f3f5bc8..d681992e5 100644 --- a/src/terminal/stream.zig +++ b/src/terminal/stream.zig @@ -386,7 +386,7 @@ pub fn Stream(comptime Handler: type) type { 'm' => if (@hasDecl(T, "setAttribute")) { var p: sgr.Parser = .{ .params = action.params, .colon = action.sep == .colon }; while (p.next()) |attr| { - log.info("SGR attribute: {}", .{attr}); + // log.info("SGR attribute: {}", .{attr}); try self.handler.setAttribute(attr); } } else log.warn("unimplemented CSI callback: {}", .{action}),