style(cli): polish matrix tui launcher#249
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR polishes the Matrix CLI TUI launcher with a Matrix-themed home screen: a centered wordmark, a 25-line ASCII rabbit mascot, responsive layout breakpoints, and a restyled command palette that respects terminal width. It also adds proper alternate-screen entry/exit and a
Confidence Score: 4/5Safe to merge with the fullscreen height overflow addressed; all other changes are purely visual and well-tested. The fullscreen layout path activates at rows >= 30, but the medium-width art layout totals ~37 lines. Ink/yoga clips children that overflow a height-constrained Box, so the status line and blocking-action hint disappear silently in 30-36-row terminals. packages/sync-client/src/cli/tui/views/HomeView.tsx — the fullScreenHeight threshold and mascot-visibility condition need to be reconciled. Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/sync-client/src/cli/tui/views/HomeView.tsx:34-55
**Full-screen mascot overflows the height-constrained Box**
`fullScreenHeight` activates at `rows >= 30`, but in the medium-width band (`!narrow && !extraWide`, roughly 80–123 columns), the mascot is rendered inline above the prompt. The mascot alone is 25 lines; with the title (1), `marginBottom` gaps (2), the bordered prompt panel (7), and the status row (2), the column totals ~37 lines. Ink/yoga clips children that overflow a height-constrained Box, so in a 30-to-36-row terminal the entire status line and the `blockingActions` hint are silently cut off. The fullscreen threshold must account for this art height: raising `rows >= 30` to `rows >= 40` (or conditionally suppressing the mascot when `fullScreenHeight && rows < 40`) closes the gap.
Reviews (11): Last reviewed commit: "fix(cli): restore fullscreen tui on proc..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@greptileai please re-review the latest commit. |
|
@greptileai please re-review latest commit f6c512c. |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| export async function launchTui(options: { noColor?: boolean } = {}): Promise<void> { | ||
| const { waitUntilExit } = render(<MatrixTuiApp noColor={options.noColor} />); | ||
| await waitUntilExit(); | ||
| const useAlternateScreen = shouldUseAlternateScreen(); | ||
| if (useAlternateScreen) { | ||
| process.stdout.write(ENTER_ALTERNATE_SCREEN); | ||
| } | ||
| try { | ||
| const { waitUntilExit } = render(<MatrixTuiApp noColor={options.noColor} />); | ||
| await waitUntilExit(); | ||
| } finally { | ||
| if (useAlternateScreen) { | ||
| process.stdout.write(EXIT_ALTERNATE_SCREEN); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
process.exit()
async finally blocks are skipped when process.exit() is called — the event loop stops immediately. Ink's default Ctrl+C handling in raw mode calls process.exit(0), so pressing Ctrl+C while the TUI is open will leave the terminal stuck in alternate screen mode (blank screen, no scrollback) until the user runs reset. Adding a synchronous process.on('exit', ...) handler guarantees cleanup regardless of how the process terminates.
| export async function launchTui(options: { noColor?: boolean } = {}): Promise<void> { | |
| const { waitUntilExit } = render(<MatrixTuiApp noColor={options.noColor} />); | |
| await waitUntilExit(); | |
| const useAlternateScreen = shouldUseAlternateScreen(); | |
| if (useAlternateScreen) { | |
| process.stdout.write(ENTER_ALTERNATE_SCREEN); | |
| } | |
| try { | |
| const { waitUntilExit } = render(<MatrixTuiApp noColor={options.noColor} />); | |
| await waitUntilExit(); | |
| } finally { | |
| if (useAlternateScreen) { | |
| process.stdout.write(EXIT_ALTERNATE_SCREEN); | |
| } | |
| } | |
| } | |
| export async function launchTui(options: { noColor?: boolean } = {}): Promise<void> { | |
| const useAlternateScreen = shouldUseAlternateScreen(); | |
| if (useAlternateScreen) { | |
| process.stdout.write(ENTER_ALTERNATE_SCREEN); | |
| const restore = () => process.stdout.write(EXIT_ALTERNATE_SCREEN); | |
| process.once("exit", restore); | |
| try { | |
| const { waitUntilExit } = render(<MatrixTuiApp noColor={options.noColor} />); | |
| await waitUntilExit(); | |
| } finally { | |
| process.removeListener("exit", restore); | |
| restore(); | |
| } | |
| } else { | |
| const { waitUntilExit } = render(<MatrixTuiApp noColor={options.noColor} />); | |
| await waitUntilExit(); | |
| } | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/sync-client/src/cli/tui/app.tsx
Line: 136-149
Comment:
**Alternate screen not restored on `process.exit()`**
`async` `finally` blocks are skipped when `process.exit()` is called — the event loop stops immediately. Ink's default Ctrl+C handling in raw mode calls `process.exit(0)`, so pressing Ctrl+C while the TUI is open will leave the terminal stuck in alternate screen mode (blank screen, no scrollback) until the user runs `reset`. Adding a synchronous `process.on('exit', ...)` handler guarantees cleanup regardless of how the process terminates.
```suggestion
export async function launchTui(options: { noColor?: boolean } = {}): Promise<void> {
const useAlternateScreen = shouldUseAlternateScreen();
if (useAlternateScreen) {
process.stdout.write(ENTER_ALTERNATE_SCREEN);
const restore = () => process.stdout.write(EXIT_ALTERNATE_SCREEN);
process.once("exit", restore);
try {
const { waitUntilExit } = render(<MatrixTuiApp noColor={options.noColor} />);
await waitUntilExit();
} finally {
process.removeListener("exit", restore);
restore();
}
} else {
const { waitUntilExit } = render(<MatrixTuiApp noColor={options.noColor} />);
await waitUntilExit();
}
}
```
How can I resolve this? If you propose a fix, please make it concise.
Nima-Naderi
force-pushed
the
084-matrix-cli-tui-polish
branch
from
2ed89d4 to
fe4ca8c
Compare
1 participant
Summary
Tests
pnpm --filter @finnaai/matrix exec vitest run tests/tui/home-render.test.tsx tests/tui/command-palette.test.tsx tests/tui/accessibility.test.tsx: passed, 10 tests.pnpm --filter @finnaai/matrix exec tsc --noEmit: passed.pnpm run check:patterns: 0 violations, 5 existing warning groups outside this visual TUI diff.git diff --check: passed.Review/Monitoring
413a2c49b0b7ad007d9861f009ed499287a7dba6; latest commitf6c512c6d60317bbf76fb8598aef4ca71105987cis pushed and awaiting check/Greptile refresh.413a2c49b0b7ad007d9861f009ed499287a7dba6, then noted one cosmetic compact-width spacing follow-up; that follow-up is fixed inf6c512c6d60317bbf76fb8598aef4ca71105987c.Notes
bunandfloxare not installed in this environment, so validation used directpnpmequivalents.pnpm --filter @finnaai/matrix testis blocked locally by stale/tmp/test-auth-1.jsonand/tmp/test-auth-2.jsonfiles owned bynobody; the unrelated OAuth tests fail on EPERM rename into those paths. The changed TUI tests pass.