fix: use viewportY offset in Buffer() to read visible viewport#704
Open
gluonfield wants to merge 1 commit intocharmbracelet:mainfrom
Open
fix: use viewportY offset in Buffer() to read visible viewport#704gluonfield wants to merge 1 commit intocharmbracelet:mainfrom
gluonfield wants to merge 1 commit intocharmbracelet:mainfrom
Conversation
Buffer() was reading lines from the top of the scrollback buffer (index 0..N) instead of the visible viewport. This caused Wait+Screen to fail after the terminal scrolled, since it matched against lines no longer visible on screen. The fix adds viewportY as the starting offset, consistent with how CurrentLine() already works in the same file. Fixes charmbracelet#659
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Buffer()reads lines starting from index 0 of the scrollback buffer instead of the visible viewport. This causesWait+Screen(andSaveOutput) to match against lines that have already scrolled off-screen.The fix adds
viewportYas the starting offset, consistent with howCurrentLine()already works in the same file.Changes
One-line change in
testing.go:Reproduction
This tape fails on the current
mainbut passes with this fix:Stock VHS reports:
timeout waiting for "Screen 9" to match 9; last value was: > seq 10 1 2 3 4 5— it's reading from the top of the scrollback instead of the visible viewport.Test plan
go test ./...passesCurrentLine()already usesviewportY(consistent approach)Fixes #659