feat: parse ConEmu OSC9;1 #4327
Conversation
src/terminal/osc.zig
Outdated
| @@ -158,6 +158,9 @@ pub const Command = union(enum) { | |||
| /// End a hyperlink (OSC 8) | |||
| hyperlink_end: void, | |||
|
|
|||
| /// Sleep (OSC 9;1) in ms | |||
| sleep: u16, | |||
There was a problem hiding this comment.
I considered using the Duration type but this would introduce a dependency on Config. Would that be an issue?
There was a problem hiding this comment.
I don't think that the Duration type is necessary here, unless there's a need to display it with units or parse it from a string with units. You might consider scaling it to nanoseconds and using a u64 since that's what Zig tends to use for that sort of thing.
There was a problem hiding this comment.
Since the duration is in the ConEmu spec as ms I'm fine keeping this as u16 and documenting it (as @dmehala already has) as a millisecond unit.
src/terminal/osc.zig
Outdated
| }, | ||
|
|
||
| .conemu_sleep_value => switch (c) { | ||
| '0'...'9' => {}, |
There was a problem hiding this comment.
I could construct the number directly here, but I thought it would be safer to handle potential overflow correctly by using std.fmt.parseUnsigned.
There was a problem hiding this comment.
parseUnsigned allows _ as in 1_000_000 is a valid Zig number so I always feel a little icky when using it to parse data, but since you don't allow _ it shouldn't be a problem.
src/terminal/osc.zig
Outdated
| v.* = @min(num, 10000); | ||
| } else |_| { | ||
| v.* = 10000; |
There was a problem hiding this comment.
Not sure if we need to replicate ConEmu's behaviour here.
There was a problem hiding this comment.
What's ConEmu's behavior? We should either replicate ConEmu's behavior or document the differences.
There was a problem hiding this comment.
There was a problem hiding this comment.
OK, looks like we match ConEmu's behavior. Please be careful linking to code though as we can't use GPL code since we are MIT. ConEmu is BSD 3 clause so I think that's OK (IANAL).
| try testing.expect(cmd == .sleep); | ||
| try testing.expectEqual(10000, cmd.sleep); | ||
| } | ||
|
|
There was a problem hiding this comment.
You should test invalid codes as well, perhaps parameters that contain something other than digits.
There was a problem hiding this comment.
addressed in c98d207
- Add test with invalid value. - Fix inspector compilation.
src/terminal/osc.zig
Outdated
| test "OSC: conemu sleep invalid input" { | ||
| const testing = std.testing; | ||
|
|
||
| var p: Parser = .{}; | ||
|
|
||
| const input = "9;1;foo"; | ||
| for (input) |ch| p.next(ch); | ||
|
|
||
| const cmd = p.end('\x1b'); | ||
|
|
||
| try testing.expect(cmd == null); | ||
| } |
There was a problem hiding this comment.
Note to myself: well that should default to 100 to fully match conemu's behaviou
There was a problem hiding this comment.
addressed in 9d9fa60
- Default to 100 if the value can't be parsed as an integer or is missing entirely.
|
Alright, I think the logic matches ConEmu's now. |
|
Please rebase. I can't merge :) |
Description
This PR implements support for the ConEmu OSC9;1 escape sequence.
Based on my understanding of ConEmu's source code:
100milliseconds if no value is specified.10000milliseconds.#3125