Applied-Software/ghostty
5
0
Fork 0
mirror of https://github.com/ghostty-org/ghostty.git synced 2025-07-17 09:16:11 +03:00
Code Issues Packages Projects Releases Wiki Activity

Fix SGR direct-color parsing issue (#4216)

Browse Source 
Fix for a little parsing issue I took note of
[here](https://github.com/ghostty-org/ghostty/issues/2125#issuecomment-2466537683).

The disparity in behavior between `ghostty@main` and `xterm` can be seen
with this reproduction script:
```sh
printf "\e[0m\nForeground:\n";
printf "\e[38:2:0:255:0mGreen\n";
printf "\e[38;2;0;255;0mGreen\n";
printf "\e[38:2:0:255:0:255mMagenta\n";
printf "\e[38;2;0;255;0;255mGreen\n";

printf "\e[0m\nBackground:\n";
printf "\e[48:2:0:255:0mGreen\n";
printf "\e[48;2;0;255;0mGreen\n";
printf "\e[48:2:0:255:0:255mMagenta\n";
printf "\e[48;2;0;255;0;255mGreen\n";

printf "\e[0m\nUnderline:\n";
printf "\e[58:2:0:255:0m\e[4mGreen\n";
printf "\e[58;2;0;255;0m\e[4mGreen\n";
printf "\e[58:2:0:255:0:255m\e[4mMagenta\n";
printf "\e[58;2;0;255;0;255m\e[4mGreen\n";

printf "\e[0m\n";
```

### Outputs:
|`xterm`|`ghostty@main`|this PR|
|-|-|-|
|<img width="85" alt="image"
src="https://github.com/user-attachments/assets/a0aacff2-2103-4fff-9160-5e663d8a70a2"
/>|<img width="110" alt="image"
src="https://github.com/user-attachments/assets/0ad12e67-3f2c-46f3-b0ee-9230032d188a"
/>|<img width="110" alt="image"
src="https://github.com/user-attachments/assets/7477e3cf-7d27-419e-986b-8df581e52398"
/>|
This commit is contained in:
Mitchell Hashimoto
2024-12-31 12:13:05 -08:00
committed by GitHub
parent 47cf5cbb40 4543cdeac8
commit df0620afe9
1 changed files with 152 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

123
src/terminal/sgr.zig
View File

@ -189,11 +189,20 @@ pub const Parser = struct {
.@"8_fg" = @enumFromInt(slice[0] - 30),
},
38 => if (slice.len >= 5 and slice[1] == 2) {
38 => if (slice.len >= 2) switch (slice[1]) {
// `2` indicates direct-color (r, g, b).
// We need at least 3 more params for this to make sense.
2 => if (slice.len >= 5) {
self.idx += 4;
// In the 6-len form, ignore the 3rd param.
const rgb = slice[2..5];
// When a colon separator is used, there may or may not be
// a color space identifier as the third param, which we
// need to ignore (it has no standardized behavior).
const rgb = if (slice.len == 5 or !self.colon)
slice[2..5]
else rgb: {
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.
@ -204,12 +213,16 @@ pub const Parser = struct {
.b = @truncate(rgb[2]),
},
};
} else if (slice.len >= 3 and slice[1] == 5) {
},
// `5` indicates indexed color.
5 => if (slice.len >= 3) {
self.idx += 2;
return Attribute{
.@"256_fg" = @truncate(slice[2]),
};
},
else => {},
},
39 => return Attribute{ .reset_fg = {} },
@ -217,11 +230,20 @@ pub const Parser = struct {
.@"8_bg" = @enumFromInt(slice[0] - 40),
},
48 => if (slice.len >= 5 and slice[1] == 2) {
48 => if (slice.len >= 2) switch (slice[1]) {
// `2` indicates direct-color (r, g, b).
// We need at least 3 more params for this to make sense.
2 => if (slice.len >= 5) {
self.idx += 4;
// We only support the 5-len form.
const rgb = slice[2..5];
// When a colon separator is used, there may or may not be
// a color space identifier as the third param, which we
// need to ignore (it has no standardized behavior).
const rgb = if (slice.len == 5 or !self.colon)
slice[2..5]
else rgb: {
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.
@ -232,24 +254,33 @@ pub const Parser = struct {
.b = @truncate(rgb[2]),
},
};
} else if (slice.len >= 3 and slice[1] == 5) {
},
// `5` indicates indexed color.
5 => if (slice.len >= 3) {
self.idx += 2;
return Attribute{
.@"256_bg" = @truncate(slice[2]),
};
},
else => {},
},
49 => return Attribute{ .reset_bg = {} },
53 => return Attribute{ .overline = {} },
55 => return Attribute{ .reset_overline = {} },
58 => if (slice.len >= 5 and slice[1] == 2) {
58 => if (slice.len >= 2) switch (slice[1]) {
// `2` indicates direct-color (r, g, b).
// We need at least 3 more params for this to make sense.
2 => if (slice.len >= 5) {
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
// When a colon separator is used, there may or may not be
// a color space identifier as the third param, which we
// need to ignore (it has no standardized behavior).
const rgb = if (slice.len == 5 or !self.colon)
slice[2..5]
else rgb: {
self.idx += 1;
break :rgb slice[3..6];
};
@ -263,12 +294,16 @@ pub const Parser = struct {
.b = @truncate(rgb[2]),
},
};
} else if (slice.len >= 3 and slice[1] == 5) {
},
// `5` indicates indexed color.
5 => if (slice.len >= 3) {
self.idx += 2;
return Attribute{
.@"256_underline_color" = @truncate(slice[2]),
};
},
else => {},
},
59 => return Attribute{ .reset_underline_color = {} },
@ -566,3 +601,59 @@ test "sgr: direct color bg missing color" {
var p: Parser = .{ .params = &[_]u16{ 48, 5 }, .colon = false };
while (p.next()) |_| {}
}
test "sgr: direct fg/bg/underline ignore optional color space" {
// These behaviors have been verified against xterm.
// Colon version should skip the optional color space identifier
{
// 3 8 : 2 : Pi : Pr : Pg : Pb
const v = testParseColon(&[_]u16{ 38, 2, 0, 1, 2, 3, 4 });
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);
try testing.expectEqual(@as(u8, 3), v.direct_color_fg.b);
}
{
// 4 8 : 2 : Pi : Pr : Pg : Pb
const v = testParseColon(&[_]u16{ 48, 2, 0, 1, 2, 3, 4 });
try testing.expect(v == .direct_color_bg);
try testing.expectEqual(@as(u8, 1), v.direct_color_bg.r);
try testing.expectEqual(@as(u8, 2), v.direct_color_bg.g);
try testing.expectEqual(@as(u8, 3), v.direct_color_bg.b);
}
{
// 5 8 : 2 : Pi : Pr : Pg : Pb
const v = testParseColon(&[_]u16{ 58, 2, 0, 1, 2, 3, 4 });
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);
}
// Semicolon version should not parse optional color space identifier
{
// 3 8 ; 2 ; Pr ; Pg ; Pb
const v = testParse(&[_]u16{ 38, 2, 0, 1, 2, 3, 4 });
try testing.expect(v == .direct_color_fg);
try testing.expectEqual(@as(u8, 0), v.direct_color_fg.r);
try testing.expectEqual(@as(u8, 1), v.direct_color_fg.g);
try testing.expectEqual(@as(u8, 2), v.direct_color_fg.b);
}
{
// 4 8 ; 2 ; Pr ; Pg ; Pb
const v = testParse(&[_]u16{ 48, 2, 0, 1, 2, 3, 4 });
try testing.expect(v == .direct_color_bg);
try testing.expectEqual(@as(u8, 0), v.direct_color_bg.r);
try testing.expectEqual(@as(u8, 1), v.direct_color_bg.g);
try testing.expectEqual(@as(u8, 2), v.direct_color_bg.b);
}
{
// 5 8 ; 2 ; Pr ; Pg ; Pb
const v = testParse(&[_]u16{ 58, 2, 0, 1, 2, 3, 4 });
try testing.expect(v == .underline_color);
try testing.expectEqual(@as(u8, 0), v.underline_color.r);
try testing.expectEqual(@as(u8, 1), v.underline_color.g);
try testing.expectEqual(@as(u8, 2), v.underline_color.b);
}
}