-
Notifications
You must be signed in to change notification settings - Fork 0
fix(cli): only exit alternate screen buffer on SIGINT if it was entered #245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,12 +9,20 @@ | |
|
|
||
| import { processUtils } from "./processUtils.js"; | ||
|
|
||
| let _inAlternateScreenBuffer = false; | ||
|
|
||
| /** Returns true if the alternate screen buffer is currently active. */ | ||
| export function isInAlternateScreenBuffer(): boolean { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe worth adding a small test to verify
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
| return _inAlternateScreenBuffer; | ||
| } | ||
|
|
||
| /** | ||
| * Enter the alternate screen buffer. | ||
| * This provides a fullscreen experience where content won't mix with | ||
| * previous terminal output. Like vim or top. | ||
| */ | ||
| export function enterAlternateScreenBuffer(): void { | ||
| _inAlternateScreenBuffer = true; | ||
| processUtils.stdout.write("\x1b[?1049h"); | ||
| } | ||
|
|
||
|
|
@@ -23,6 +31,7 @@ export function enterAlternateScreenBuffer(): void { | |
| * This returns the terminal to its original state before enterAlternateScreen() was called. | ||
| */ | ||
| export function exitAlternateScreenBuffer(): void { | ||
| _inAlternateScreenBuffer = false; | ||
| processUtils.stdout.write("\x1b[?1049l"); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| /** | ||
| * Tests for terminal screen buffer utilities. | ||
| */ | ||
|
|
||
| import { describe, it, expect, beforeEach, afterEach } from "@jest/globals"; | ||
| import { processUtils } from "../../../src/utils/processUtils.js"; | ||
| import { | ||
| isInAlternateScreenBuffer, | ||
| enterAlternateScreenBuffer, | ||
| exitAlternateScreenBuffer, | ||
| } from "../../../src/utils/screen.js"; | ||
|
|
||
| describe("alternate screen buffer tracking", () => { | ||
| let originalWrite: typeof processUtils.stdout.write; | ||
|
|
||
| beforeEach(() => { | ||
| // Swallow the escape sequences so they don't leak into test output. | ||
| originalWrite = processUtils.stdout.write; | ||
| processUtils.stdout.write = () => true; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| // Leave the buffer flag in a clean state for other tests. | ||
| exitAlternateScreenBuffer(); | ||
| processUtils.stdout.write = originalWrite; | ||
| }); | ||
|
|
||
| it("tracks enter and exit transitions", () => { | ||
| expect(isInAlternateScreenBuffer()).toBe(false); | ||
|
|
||
| enterAlternateScreenBuffer(); | ||
| expect(isInAlternateScreenBuffer()).toBe(true); | ||
|
|
||
| exitAlternateScreenBuffer(); | ||
| expect(isInAlternateScreenBuffer()).toBe(false); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should also move, or document a follow-up to move,
benchmark-job watchto use these helpers (right now it does its own handling of the raw escape sequences, but that also meansisInAlternateScreenBuffer()isfalsewhen we're in the benchmark-job watch alternate screen buffer -- potential footgun)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Migrated
benchmark-job watchto the shared helpers (replaced its rawenterAltScreen/exitAltScreen/cursor/clear sequences).isInAlternateScreenBuffer()now reports true while in the watch alt-screen, so the global SIGINT handler restores it correctly. (ac87f8b)