Skip to content

fix(vt): disable alternate screen scrollback - #948

Open
armenr wants to merge 1 commit into
charmbracelet:mainfrom
armenr:fix/vt-alternate-screen-scrollback
Open

fix(vt): disable alternate screen scrollback#948
armenr wants to merge 1 commit into
charmbracelet:mainfrom
armenr:fix/vt-alternate-screen-scrollback

Conversation

@armenr

@armenr armenr commented Aug 14, 2026

Copy link
Copy Markdown
  • I have read CONTRIBUTING.md.
  • I have created a discussion that was approved by a maintainer (for new features). Not applicable: this is a bug fix.

Fixes #947

Summary

Disable the alternate screen's scrollback buffer when constructing an Emulator and extend the existing alternate-screen regression to assert that the hidden buffer is absent after scrolling.

NewScreen enables a 10,000-line history by default, and NewEmulator uses 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:

$ go test -count=1 -run='TestScrollback/alt_screen_does_not_have_scrollback' ./...
--- FAIL: TestScrollback (0.00s)
    --- FAIL: TestScrollback/alt_screen_does_not_have_scrollback (0.00s)
        scrollback_test.go:118: expected nil alt-screen scrollback, got max=10000 len=6
FAIL

After this change, the same regression and the complete vt suite pass.

Why this fix

Screen.SetScrollback(nil) is already the documented way to disable history. Applying it once to scrs[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/vt revision 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, from vt/:

  • gofmt -l . — clean
  • git diff --check — clean
  • go mod verify — pass
  • focused regression on unmodified main — fails as expected with hidden max=10000 len=6
  • focused regression after the fix, 100 repetitions — pass
  • go build -v ./... — pass
  • go test -count=1 -race -covermode=atomic -coverprofile=coverage.txt ./... — pass, 41.2% statement coverage
  • go vet ./... — pass
  • Charm's current golangci.yml with golangci-lint v2.12.2, restricted to changes since upstream/main — 0 issues
  • compile-only go test -c checks — pass for Linux/arm64, Darwin/amd64, Darwin/arm64, and Windows/amd64

The unrestricted shared lint command currently reports two unrelated findings already present on main (vt/emulator.go's unused nolint:gosec directive and vt/csi.go's QF1012). Upstream's current-main vt workflow run 31709584952 fails on those same two findings; this patch adds no lint findings.

Compatibility

  • No public API changes.
  • Primary scrollback behavior and its configurable size are unchanged.
  • Visible primary/alternate screen behavior is unchanged by the existing test suite.
  • Callers that somehow relied on the private, inaccessible alternate buffer cannot observe it through Emulator; if alternate history is desired later, it should be exposed and bounded explicitly.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vt: alternate screen retains hidden scrollback

1 participant