Skip to content

fix: enable mouse scrolling in pager mode#198

Open
elucid wants to merge 5 commits intomainfrom
elucid/tty-mouse-pager-fixes
Open

fix: enable mouse scrolling in pager mode#198
elucid wants to merge 5 commits intomainfrom
elucid/tty-mouse-pager-fixes

Conversation

@elucid
Copy link
Copy Markdown
Member

@elucid elucid commented Apr 12, 2026

Problem

Mouse wheel scrolling was disabled in pager mode. The old code tied useMouse directly to !cliInput.options.pager, so any pager-style session (piped stdin, explicit --pager flag) lost mouse support entirely.

Fix

Decouple mouse support from pager chrome. A new shouldUseMouseForApp() function enables mouse when an interactive terminal is available—either through a TTY stdin or a /dev/tty controlling terminal attachment. Pager mode now only controls UI chrome (hiding the menu bar), not input capabilities.

Also simplifies ControllingTerminal to only open /dev/tty for input, since process.stdout already carries the PTY output stream.

Testing

  • Unit tests for shouldUseMouseForApp() covering all TTY/controlling-terminal combinations.
  • Three new PTY integration tests verifying mouse wheel scroll in each pager path:
    • hunk patch - (piped stdin)
    • hunk pager (piped stdin)
    • hunk patch <file> --pager (file-backed with explicit flag)

Manual testing

Everything has pty tests but if you want to run through all of the different modes this covers by by hand:

Case 1: Piped patch -

  # old (mouse scroll broken)
  git diff HEAD~3..HEAD | hunk patch -

  # new (mouse scroll works)
  git diff HEAD~3..HEAD | bun run src/main.tsx -- patch -

Case 2: Piped pager

  # old (mouse scroll broken)
  git diff HEAD~3..HEAD | hunk pager

  # new (mouse scroll works)
  git diff HEAD~3..HEAD | bun run src/main.tsx -- pager

Case 3: File-backed --pager

  git diff HEAD~3..HEAD > /tmp/test.patch

  # old (mouse scroll broken)
  hunk patch /tmp/test.patch --pager

  # new (mouse scroll works)
  bun run src/main.tsx -- patch /tmp/test.patch --pager

Regression check: normal interactive mode (should work in both)

  # old (mouse scroll works)
  git diff HEAD~3..HEAD > /tmp/test.patch
  hunk diff HEAD~3..HEAD

  # new (mouse scroll works)
  git diff HEAD~3..HEAD > /tmp/test.patch
  bun run src/main.tsx -- diff HEAD~3..HEAD

@elucid elucid requested a review from benvinegar April 12, 2026 22:52
The piped-pager integration tests need a non-TTY stdin while keeping
the PTY on stdout. Use bash `exec cmd < file` in the harness to
swap fd 0 to a fixture file without introducing extra dependencies.
@elucid elucid force-pushed the elucid/tty-mouse-pager-fixes branch from 2642ddc to 8b97b6c Compare April 12, 2026 22:56
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 12, 2026

Greptile Summary

This PR fixes mouse wheel scrolling in pager mode by introducing shouldUseMouseForApp() in terminal.ts, decoupling useMouse from cliInput.options.pager and instead keying it off whether an interactive terminal is available (TTY stdin or /dev/tty controlling terminal). It also simplifies openControllingTerminal to stdin-only since process.stdout already carries the PTY output.

Confidence Score: 5/5

Safe to merge; the only finding is a minor test-harness style issue with the login shell flag

All production logic changes are correct and well-tested with both unit and PTY integration coverage. The single P2 finding is a cosmetic test-harness concern (unnecessary -l login flag) that does not affect correctness or CI reliability in practice.

test/pty/harness.ts — minor: the -l login-shell flag in launchShellCommand is unnecessary

Important Files Changed

Filename Overview
src/core/terminal.ts Adds shouldUseMouseForApp() decoupling mouse from pager chrome, and simplifies openControllingTerminal() to stdin-only; logic is correct and well-typed
src/core/terminal.test.ts Unit tests cover all three TTY/controlling-terminal combinations for shouldUseMouseForApp() and both open/fail paths for openControllingTerminal()
src/main.tsx Switches useMouse from !cliInput.options.pager to shouldUseMouseForApp({ hasControllingTerminal }); correctly threads controllingTerminal through startup plan
test/pty/harness.ts Adds launchShellCommand/buildHunkCommand/launchHunkWithFileBackedStdin for piped-stdin PTY tests; the -l login-shell flag in launchShellCommand is unnecessary and could cause env interference
test/pty/ui-integration.test.ts Three new PTY integration tests covering mouse wheel scroll in stdin-piped patch, pager, and explicit --pager-on-TTY paths; test structure is consistent with existing coverage
hunk-bugs.md Bug report for the mouse-scroll issue now being fixed; documents root cause accurately

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[prepareStartupPlan] --> B{usesPipedPatchInput?}
    B -- yes: kind=patch, file=- , stdin piped --> C[openControllingTerminal\nopen /dev/tty as ReadStream]
    B -- no: file-backed or TTY stdin --> D[controllingTerminal = null]
    C --> E[StartupPlan: app\ncontrollingTerminal set]
    D --> E
    E --> F[main.tsx\ncreateCLIRenderer]
    F --> G[shouldUseMouseForApp\nstdinIsTTY default OR hasControllingTerminal]
    G -- stdinIsTTY=true --> H[useMouse: true]
    G -- hasControllingTerminal=true --> H
    G -- both false --> I[useMouse: false]
    H --> J[Mouse wheel scroll enabled\nin pager UI]
    I --> K[No mouse support\nnon-interactive env]
Loading

Reviews (1): Last reviewed commit: "Launch piped PTY tests with bash exec st..." | Re-trigger Greptile

@elucid
Copy link
Copy Markdown
Member Author

elucid commented Apr 12, 2026

Good catch — fixed in d2626fb.

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.

1 participant