fix(tui): default composer_arrows_scroll to true on Windows (#1255)#1578
Closed
kunpeng-ai-lab wants to merge 1 commit into
Closed
fix(tui): default composer_arrows_scroll to true on Windows (#1255)#1578kunpeng-ai-lab wants to merge 1 commit into
kunpeng-ai-lab wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request modifies the default behavior of composer_arrows_scroll to be true on Windows, regardless of mouse capture settings, to address terminal behavior where wheel events are mapped to arrow keys. The test suite has been updated with platform-specific logic to reflect these changes. The reviewer suggested simplifying the conditional logic in app.rs using a more concise boolean expression for improved readability.
Comment on lines
+1570
to
+1574
| .unwrap_or(if cfg!(windows) { | ||
| true | ||
| } else { | ||
| !use_mouse_capture | ||
| }), |
Contributor
There was a problem hiding this comment.
For better readability and conciseness, you can simplify this conditional logic using a boolean expression.
.unwrap_or(cfg!(windows) || !use_mouse_capture),) On Windows terminals (including Windows Terminal) mouse-wheel events are often mapped to arrow-key sequences rather than delivered as crossterm MouseEventKind::ScrollUp/ScrollDown. The existing fallback only enabled composer_arrows_scroll when mouse_capture was off, but on Windows Terminal mouse_capture defaults to on because WT_SESSION is set. This left Win10 users unable to scroll the transcript with the mouse wheel. Default composer_arrows_scroll to true on Windows regardless of the mouse-capture flag so that wheel-mapped arrow keys scroll the transcript instead of cycling input history.
29b7f51 to
8275867
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On Windows terminals (including Windows Terminal) mouse-wheel events are often mapped to arrow-key sequences rather than delivered as crossterm MouseEventKind::ScrollUp/ScrollDown. The existing fallback only enabled composer_arrows_scroll when mouse_capture was off, but on Windows Terminal mouse_capture defaults to on because WT_SESSION is set. This left Win10 users unable to scroll the transcript with the mouse wheel.
This PR defaults composer_arrows_scroll to true on Windows regardless of the mouse-capture flag so that wheel-mapped arrow keys scroll the transcript instead of cycling input history.