fix: capture visible viewport in VHS.Buffer

[runesoerensen]

Describe your changes

The VHS.Buffer method was incorrectly reading from the top of the scrollback buffer.

This causes issues when using the Wait+Screen /regex/ command, and when writing output to golden files.

The logic now accounts for the scroll position (viewportY) to capture visible lines in the terminal viewport as expected.

More details in the issues linked below (which both include the output after this change in the "Expected Behavior" sections, as well as steps to reproduce the current unexpected behavior).

Additional Style Changes

• Prefer blank identifier for unused variable.
• Replaced fill().map() with Array.from() for clarity.
• Renamed the method CurrentLines and updated comments for clarity and accuracy.

Related issue/discussion

Closes #657, closes #659

Checklist before requesting a review

• I have read CONTRIBUTING.md
• I have performed a self-review of my code

[rhuiser]

Please @meowgorithm and @andreynering review this pull request so it can be merged & released.

If there is anything I can do to speed this up, please let me know.

[codevogel]

Awesome work!

I ran into the same exact bug recording a demo gif for commit for me.
As my demo is quite lengthy, I ran into the issue of timing out Waiting for the second time I run a command in my demo.

I cloned this repo, applied your patch, and now I have a working demo gif.

This does immediately raise a question: What if the output you're Waiting for scrolls past the visible buffer?
This could easily happen with a lengthy response...

Anyway, your PR already seems like a huge improvement, so big ups and I hope this can get merged soon!

[runesoerensen]

This does immediately raise a question: What if the output you're Waiting for scrolls past the visible buffer? This could easily happen with a lengthy response...

That's a good question @codevogel, and as you probably suspect, output may not always be matched if it scrolls past the visible lines. More specifically, I believe the answer is that a Wait+Screen command is less likely to match output that:

• exceeds the visible buffer during any ~10ms interval (i.e. the time between wait pattern checks).
• is printed before the Wait command is executed (the delay between Enter and Wait commands, for instance, can easily add +50ms before the first check).

Case in point: Adapting the wait-screen-test.tape repro sample from #659 to first check for a value that is no longer visible, like this wait-screen-test-v2.tape:

Set Height 300

Type "seq 10"
Enter
Wait+Screen /3/
Wait+Screen /9/

Will (with the changes in this PR) cause Wait+Screen /3/ to time out (as there are only 6 visible $LINES that seq 10 will immediately exceed):

% ../vhs wait-screen-test-v2.tape
File: wait-screen-test-v2.tape
Set Height 300
Type seq 10
Enter 1
Wait Screen 3
failed to execute command: timeout waiting for "Screen 3" to match 3; last value was: 6
7                                                                                      
8                                                                                      
9                                                                                      
10                                                                                     
>                                                                                      
recording failed

Using vhs version 0.10.0, Wait+Screen /3/ will match and Wait+Screen /9/ will time out instead:

% vhs wait-screen-test-v2.tape
File: wait-screen-test-v2.tape
Set Height 300
Type seq 10
Enter 1
Wait Screen 3
Wait Screen 9
failed to execute command: timeout waiting for "Screen 9" to match 9; last value was: > seq 10
1                                                                                             
2                                                                                             
3                                                                                             
4                                                                                             
5                                                                                             
recording failed

Note: The first check passes with the current vhs release because it only checks output from the top of the scrollback buffer. I'm mentioning it anyway because it's technically a breaking change compared to the current behavior (even if that behavior is unintended and is the result of the bug this PR addresses).

Adding a brief sleep to allow the Wait+Screen check to match the command output while still visible, e.g. like this wait-screen-test-v3.tape:

Set Height 300

Type "seq 4 && sleep 0.1 && seq 5 10"
Enter
Wait+Screen /3/
Wait+Screen /9/

Will now match both statements:

% ../vhs wait-screen-test-v3.tape
File: wait-screen-test-v3.tape
Set Height 300
Type seq 4 && sleep 0.1 && seq 5 10
Enter 1
Wait Screen 3
Wait Screen 9
Host your GIF on vhs.charm.sh: vhs publish <file>.gif

To summarize, I did consider this potential failure mode when preparing the PR, but opted to keep changes minimal and focused on just fixing the current implementation to work as intended (which I suspect in practice covers the majority of scenarios that call for using Wait+Screen).

It might very well make sense to consider ways to improve the Wait (+ the CI/golden file) logic though, and/or perhaps as a first step update the documentation to better reflect how it currently works (which for both Line and Screen scopes is a bit more nuanced than just waiting "for something to appear on the screen" :)).