Skip to content

[codex] Fix composer arrows with mouse capture#1866

Open
Lee-take wants to merge 2 commits into
Hmbown:mainfrom
Lee-take:fix/windows-composer-arrows
Open

[codex] Fix composer arrows with mouse capture#1866
Lee-take wants to merge 2 commits into
Hmbown:mainfrom
Lee-take:fix/windows-composer-arrows

Conversation

@Lee-take
Copy link
Copy Markdown

Summary

  • Default composer arrow scrolling now stays off when mouse capture is enabled.
  • Keeps the existing opt-in behavior for configurations without mouse capture.
  • Updates regression tests for both app config and UI config loading.

Fixes #1720

Validation

  • cargo fmt --check
  • cargo test -p deepseek-tui composer_arrows_scroll_default -- --nocapture

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request standardizes the default behavior for composer arrow scrolling across all platforms by removing the Windows-specific logic that enabled it when mouse capture was active. The default_composer_arrows_scroll_for_platform function and associated tests have been updated to ensure arrow scrolling defaults to false when mouse capture is enabled, regardless of the operating system. I have no feedback to provide.

@Lee-take Lee-take marked this pull request as ready for review May 21, 2026 07:23
@Hmbown Hmbown added this to the v0.8.41 milestone May 21, 2026
Copy link
Copy Markdown
Owner

@Hmbown Hmbown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this fix — the behavior change looks right for mouse-capture mode. The new test assertion will fail the workspace clippy gate (-D warnings) on clippy::bool_assert_comparison:

crates/tui/src/tui/ui/tests.rs:5732

Suggested one-line tweak:

assert!(
    !app.composer_arrows_scroll,
    "arrows-scroll should default to false when mouse capture is on"
);

Local repro (from a worktree of this PR):

cargo clippy -p deepseek-tui --all-targets --all-features --locked -- -D warnings

Once that's applied, behavior tests, fmt, and clippy all pass locally. Happy to push the fix if it's easier.

@Hmbown
Copy link
Copy Markdown
Owner

Hmbown commented May 24, 2026

Thanks for the Windows composer-arrow fix. I checked current main: default_composer_arrows_scroll_for_platform still returns is_windows || !use_mouse_capture, and issue #1720 remains open, so this is not already fully handled.

I am not merging it into v0.8.42 because Windows input/mouse-capture defaults need a focused pass and current PR CI only shows GitGuardian. Keeping it open for a Windows input maintenance pass with current crate-name tests and, ideally, the matrix from #1720 rechecked against Windows Terminal/conhost/VS Code integrated terminal.

@Lee-take
Copy link
Copy Markdown
Author

Rechecked the Windows input matrix on the current PR branch (ac1ae026).

Environment:

  • Windows 11 Home 10.0.26200 / build 26200
  • cargo 1.95.0, rustc 1.95.0
  • Windows Terminal available via wt.exe; VS Code available via code.cmd

Local checks:

  • cargo fmt --check
  • cargo clippy -p deepseek-tui --all-targets --all-features --locked -- -D warnings
  • cargo test -p deepseek-tui mouse_capture --all-features --locked — 20 related tests passed
  • cargo test -p deepseek-tui composer_arrows_scroll --all-features --locked — 10 related tests passed

I also ran a small crossterm event probe in each terminal host. The probe logged terminal env markers and event types, then I sent Win32 wheel input to the focused terminal window.

Host Env marker Probe mode Wheel events seen by app Up/Down key events from wheel PR/default behavior this supports
Windows Terminal WT_SESSION present mouse capture on 9 Mouse(ScrollUp/ScrollDown) 0 default mouse capture is on; with this PR composer_arrows_scroll defaults false, so plain Up/Down can navigate history while wheel is handled as mouse scroll
Windows Terminal WT_SESSION present mouse capture off 0 0 explicit capture-off keeps composer_arrows_scroll = true, preserving arrow-scroll behavior
cmd.exe / conhost no WT_SESSION, no ConEmuPID, no TERM_PROGRAM mouse capture off 0 0 in this synthetic probe default mouse capture remains off; with this PR composer_arrows_scroll = true, preserving the arrow-scroll fallback
cmd.exe / conhost no WT_SESSION, no ConEmuPID, no TERM_PROGRAM mouse capture on 9 Mouse(ScrollUp/ScrollDown) 0 explicit capture can receive wheel as mouse events, but the current default still keeps capture off for the conhost leak-risk path
VS Code integrated terminal TERM_PROGRAM=vscode mouse capture off 0 0 in this synthetic probe default mouse capture remains off; with this PR composer_arrows_scroll = true, preserving the arrow-scroll fallback
VS Code integrated terminal TERM_PROGRAM=vscode mouse capture on 9 Mouse(ScrollUp/ScrollDown) 0 explicit capture can receive wheel as mouse events

Caveat: the wheel rows used synthesized Win32 wheel input, not a physical mouse wheel, so I would treat them as terminal event-routing evidence rather than a full manual UX pass. The #1720 behavior itself is covered by the unit tests above: on Windows, composer_arrows_scroll now defaults to false when mouse capture is on and remains true when mouse capture is off.

@Hmbown
Copy link
Copy Markdown
Owner

Hmbown commented May 25, 2026

Thanks for the focused fix here. I harvested the same behavior into a fresh maintainer branch with current tests and CI as #2103, and credited this PR plus the Windows terminal matrix in the new PR body. The landed changelog-level behavior is: Windows Terminal / mouse-capture sessions now keep Up/Down history recall, while no-mouse-capture terminals keep the transcript-scroll fallback.

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.

Empty composer: Up/Down scrolls transcript instead of navigating history on Windows

2 participants