search

buildkite / terminal-to-html

Converts arbitrary shell output (with ANSI) into beautifully rendered HTML
http://buildkite.github.io/terminal-to-html
MIT License
642 stars 45 forks source link

Buildkite timestamps are zero-width via persistent elements #97

Closed pda closed 1 year ago

pda commented 1 year ago

As reported in #96, Buildkite's timestamping ANSI Application Program Command is interfering with cursor movements.

For example a 10-character-wide progress bars might move the cursor back 10 characters to overwrite itself with an update. An invisible timestamp within the progress bar would erroneously consume on of the cursor movements, making it move only 9 characters, leaving a stray character on the left, and accumulating as this repeats.

Closes #96

pda commented 1 year ago

Here's the failing minimal reproduction test case:

--- FAIL: TestRendererAgainstCases (0.00s)
    terminal_test.go:285: handles _bk timestmaps as zero-width for cursor movement
        input           "Buildb\x1b_bk;t=0\aox\x1b[3Dkite"
        received        "Buildbkite"
        expected        "Buildkite"
FAIL
pda commented 1 year ago

So it turns out the current implementation doesn't really support zero-width nodes/elements. Elements like Buildkite timestamps are appended as a node at the next x coordinate as an attachment to a new node with an zero-value blob rune. The presence of an element on a node means the blob rune isn't rendered, but it does still takes up a position in the X,Y coordinate system, which I think is a bug, but requires some data structure changes to fix.

I wonder what it even means to backspace through a timestamp and overwrite it, potentially with a different timestamp. Or to place a later timestamp prior to an earlier one. 🤔

pda commented 1 year ago

It's also possible that the mid-line timestamps are a bug in the agent's timestamping Prefixer, and that they shouldn't be expected to work. I believe it intends to only timestamp start-of-lines. But it would be nice if it worked anyway.

lox commented 1 year ago

I think we added mid-line timestamps to handle long running single-line progress bars so they could show how long they had been running. Rspec comes to mind.

pda commented 1 year ago

I've extracted the general refactoring etc from this PR into #98 and rebased onto that. I don't really want to merge this PR, and have a better solution in mind.