fix(vt): disable alternate screen scrollback - #948
Open
armenr wants to merge 1 commit into
Open
Conversation
NewEmulator creates both screens through NewScreen, which enables a 10,000-line history buffer. The emulator APIs only expose and configure primary-screen history, so alternate-screen output can retain cloned rows in a hidden buffer. Disable that buffer when constructing the alternate screen and assert it stays absent after scrolling. Primary history and visible screen behavior are unchanged.
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.
CONTRIBUTING.md.Fixes #947
Summary
Disable the alternate screen's scrollback buffer when constructing an
Emulatorand extend the existing alternate-screen regression to assert that the hidden buffer is absent after scrolling.NewScreenenables a 10,000-line history by default, andNewEmulatoruses it for both screens. The emulator's public scrollback methods expose and configure only the primary screen, so alternate-screen output currently clones rows into a buffer callers cannot inspect, bound, or clear.Before and after
Before this change, the focused regression fails on current
main:After this change, the same regression and the complete
vtsuite pass.Why this fix
Screen.SetScrollback(nil)is already the documented way to disable history. Applying it once toscrs[1]keeps primary history and visible terminal behavior unchanged while preventing unreachable alternate-screen retention. It adds no API and does not alter parsing, resizing, or screen switching.In a separate SwitchBoard integration measurement on an earlier
x/vtrevision with the same retention path, a 2 MiB alternate-screen flood retained 104,234,528 bytes of incremental live heap for one untouched emulator versus 6,025,248 bytes with this constructor change. Those workload-specific numbers are supporting evidence, not an upstream benchmark or memory guarantee.Validation
Run on Linux x86_64 with Go
1.26.5-X:nodwarf5, fromvt/:gofmt -l .— cleangit diff --check— cleango mod verify— passmain— fails as expected with hiddenmax=10000 len=6go build -v ./...— passgo test -count=1 -race -covermode=atomic -coverprofile=coverage.txt ./...— pass, 41.2% statement coveragego vet ./...— passgolangci.ymlwith golangci-lintv2.12.2, restricted to changes sinceupstream/main— 0 issuesgo test -cchecks — pass for Linux/arm64, Darwin/amd64, Darwin/arm64, and Windows/amd64The unrestricted shared lint command currently reports two unrelated findings already present on
main(vt/emulator.go's unusednolint:gosecdirective andvt/csi.go'sQF1012). Upstream's current-mainvtworkflow run31709584952fails on those same two findings; this patch adds no lint findings.Compatibility
Emulator; if alternate history is desired later, it should be exposed and bounded explicitly.