[sgrimee]

Fixes #95

Enables mouse text selection and clipboard copy by disabling mouse capture. Users can now select text with their mouse and copy using standard terminal shortcuts (Cmd+C on macOS, Ctrl+Shift+C on Linux).

The app wasn't using mouse events, so this change has no negative impact while enabling standard terminal selection UX.

🤖 Generated with Claude Code

[claude]

Pull Request Review: Enable Native Terminal Text Selection

Summary

This PR successfully addresses issue #95 by removing mouse capture functionality, enabling native terminal text selection and clipboard operations. The implementation is clean and well-justified.

Code Quality ✅

Strengths:

• Minimal, focused change: Only touches what's necessary (2 additions, 3 deletions)
• Clean removal: Properly removes both the import and usage of EnableMouseCapture/DisableMouseCapture
• Symmetric changes: Correctly updates both init() and reset() methods in src/tui.rs:52,84
• Maintains panic handler: The custom panic hook still properly calls reset(), ensuring terminal cleanup on crashes

Verification of Claims ✅

I verified the PR description's claim that "the app wasn't using mouse events":

Evidence from codebase:

• src/inputs/handler.rs:41-47 - Event handler only captures crossterm::event::Event::Key, explicitly ignoring all other event types including mouse events
• No other mouse event handling found in the codebase (searched comprehensively)
• This confirms the change has zero functional impact on existing features

Best Practices ✅

Follows project conventions:

• Aligns with the multi-threaded async architecture described in CLAUDE.md
• Uses appropriate crossterm APIs from the documented stack (ratatui + crossterm)
• Maintains backward compatibility with terminal cleanup flow

Potential Issues 🟡

Minor considerations:

1. No test coverage - While this is a simple change, consider adding a test to verify terminal initialization/cleanup still functions correctly

2. Documentation gap - Consider updating comments in src/tui.rs if any mentioned mouse capture behavior (though I didn't see any that need updating)

Performance Considerations ✅

Positive impact:

• Slightly reduces overhead by not capturing/processing mouse events that were being ignored anyway
• No performance regressions expected

Security Concerns ✅

No issues identified:

• Terminal handling remains secure with proper cleanup
• Panic hook still properly resets terminal state
• No new attack surface introduced

Recommendations

Before merge:

1. ✅ Can merge as-is - The change is safe and correct
2. 📝 Optional improvement: Add a basic test to verify initialization/cleanup behavior

After merge (future considerations):

• If mouse interaction is ever needed in the future (e.g., clicking on rooms), this decision would need to be revisited
• Consider documenting in CLAUDE.md that mouse capture is intentionally disabled for text selection UX

Conclusion

Recommendation: ✅ APPROVE

This is a well-executed, minimal change that successfully enables a user-requested feature with no negative side effects. The implementation correctly addresses the stated issue and follows project conventions.

Great work! The PR is ready to merge.

---

Review conducted by Claude Code following project standards from CLAUDE.md

[sgrimee]

@claude also update the changelog

[claude]

Claude finished @sgrimee's task —— View job

---

Changelog Updated ✅

I've successfully updated the CHANGELOG.md file to include an entry for the native terminal text selection feature.

Tasks Completed

• Find and read the changelog file
• Add entry for native terminal text selection feature
• Commit and push changes

Changes Made

Added the following entry to the Unreleased section under Features in CHANGELOG.md:

• Native text selection: Enable native terminal text selection and clipboard copy by disabling mouse capture

The change has been committed and pushed to the PR branch claude/issue-95-20251101-1056.

Note: I was unable to run the pre-commit checks (cargo check, test, fmt, clippy) as they require approval in the current environment. However, since this is a simple documentation change (adding one line to the changelog), the risk of breaking anything is minimal. You may want to run these checks locally if desired.

---

Branch |

[claude]

Pull Request Review

Summary

This PR successfully addresses issue #95 by enabling native terminal text selection. The implementation is clean, minimal, and well-executed.

✅ Code Quality and Best Practices

Excellent:

• Minimal, focused change that does exactly what's needed
• Removes unused imports (DisableMouseCapture, EnableMouseCapture)
• CHANGELOG entry follows the established format and convention
• Clear, descriptive commit message and PR description

Recommendations:

• Consider adding a comment in src/tui.rs explaining why mouse capture is intentionally disabled (for native text selection). This would help future maintainers understand the design decision:

  // Mouse capture is intentionally disabled to allow native terminal text selection
  // and clipboard operations (Cmd+C/Ctrl+Shift+C)
  execute!(io::stderr(), EnterAlternateScreen)?;

✅ Potential Bugs or Issues

No bugs identified. The change is safe because:

• Verified: The event handler in src/inputs/handler.rs:41-47 only captures KeyEvent, not mouse events
• No other mouse event handling found in the codebase (grep confirmed)
• The application doesn't use mouse interactions, so disabling mouse capture has no negative impact

✅ Performance Considerations

No performance concerns:

• This change actually slightly improves performance by removing unnecessary mouse event capture
• Event polling remains unchanged and efficient

✅ Security Concerns

No security issues identified.

• The change only affects terminal interaction mode
• No authentication, data handling, or network-related code modified

✅ Test Coverage

Observation:

• The src/tui.rs module currently has no dedicated unit tests
• This change is low-risk enough that lack of tests isn't a blocker

Recommendation for future work:

• Consider adding integration tests for the TUI initialization/cleanup cycle
• However, testing terminal state can be challenging and may not be worth the complexity for this simple module

Architecture Alignment

The change aligns perfectly with the project's architecture:

• Follows the "minimal change" philosophy evident in the codebase
• Consistent with crossterm 0.28 best practices
• Maintains proper cleanup in both normal exit and panic paths

Recommendation

✅ APPROVE - This PR is ready to merge.

The implementation is solid, well-documented in the CHANGELOG, and safely addresses the user's need for native text selection. The only minor suggestion is adding an explanatory comment, but this is optional and not a blocker.

Great work on keeping this change focused and minimal! 🎉