ink
ink copied to clipboard
Fix rendering when terminal is erased on first render
Fixes https://github.com/vadimdemedes/ink/issues/585.
@matteodepalo did a great job documenting the issue, so I'm not going to repeat it here.
Context
Ink has two ways of rendering output, depending how tall it is:
-
If output is shorter than terminal height, Ink erases the lines from last output and writes lines from the new output. Ink tracks how many lines did the last output have for this to work. This is handled in https://github.com/vadimdemedes/ink/blob/master/src/log-update.ts, which is a fork of
log-update
package. -
If output is equal or taller than terminal height, Ink erases the entire terminal history (or scrollback as it's commonly referred to) and writes new output. Note that Ink doesn't need to track how many lines to erase here, because it just erases the entire contents of the current terminal. https://github.com/vadimdemedes/ink/blob/8a0476024cf997e0dbc02e493ad3972349b02474/src/ink.tsx#L176-L182
Root cause
The key insight here is this:
Note that Ink doesn't need to track how many lines to erase here, because it just erases the entire contents of the current terminal.
In https://github.com/vadimdemedes/ink/issues/585, the very first render starts with the second rendering method and erases the terminal completely. On next render, the first method kicks in, but there's no information how many lines to erase, since second method doesn't store that information.
That's why the output from the first render doesn't get erased, since for logUpdate
this variable is zero.
https://github.com/vadimdemedes/ink/blob/8a0476024cf997e0dbc02e493ad3972349b02474/src/log-update.ts#L12
Solution
This PR fixes this bug by separating the write operation that erases the terminal and writes the output. This way, even if the entire terminal is erased, logUpdate
is still used to render the output and keep track of how many lines were written.
I've also added force
option to logUpdate
render function to force it to always render the string we give to it without bailing out if it equals to the last output. This can happen when Static
output changes, but normal output isn't.
https://github.com/vadimdemedes/ink/blob/8a0476024cf997e0dbc02e493ad3972349b02474/src/log-update.ts#L23-L25
I've also added erase
option to logUpdate
render function, so that we can tell it to not do erasing of its own, if we erased the entire terminal already.
Not sure why tests are failing on CI, while passing locally consistently. Will need to look into it. Help also appreciated to ship this fix faster!
You can use https://github.com/nektos/act to debug the ci
<comment deleted as some testing revealed it didn't work>
@vadimdemedes strangely with this fix, tests pass locally if I run CI=true npm run test
, but they don't seem to pass on CI.
The reason for that fix is that in CI we don't clear the terminal, we return earlier in the code, so it makes sense that the check for ansiEscapes.clearTerminal
fails.
Hey there, wondering if there are any updates on this work. I have a need on this being resolved. Is there anything I can do to help get this over the finish line? :)
@DaniGuardiola unfortunately I haven't had the time to continue on this bug, the issue as I left it was one with tests. The code was working fine. CI is failing while local tests are passing, but I couldn't find the root cause. If you'd like to pick it up you can also check what I tried to do in a separate branch to maybe get some ideas.
@DaniGuardiola The weird issue is that I fixed the original issue in this PR and confirmed it works on my laptop, but when I submitted this PR, it consistently fails on CI. Huge help would be to figure out what's going on in CI here to unblock this PR.
I changed my approach and this stopped being a blocker for me. I don't think I'll have the time to look into it, sorry.