-
Notifications
You must be signed in to change notification settings - Fork 0
[06/09] injection: clipboard-preserve + Wayland-first strategy #128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds clipboard preservation and introduces a Wayland-first strategy for text injection by consolidating clipboard and paste operations into a unified approach. The changes replace the separate clipboard and combo injectors with a streamlined ClipboardPasteInjector that saves/restores user clipboard content automatically.
Key changes:
- Consolidates clipboard operations with automatic preservation and configurable restore delays
- Implements unified clipboard+paste strategy with AT-SPI first, ydotool fallback approach
- Simplifies strategy ordering to prioritize environment detection over complex backend detection
- Removes separate
allow_ydotoolconfiguration as ydotool becomes an internal fallback mechanism
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
ydotool_injector.rs |
Removes allow_ydotool configuration checks and simplifies availability detection |
window_manager.rs |
Converts async window detection functions to synchronous operations and adds Sway support |
types.rs |
Updates injection method enum, removes clipboard configuration options, adds fail-fast mode |
test_*.rs |
Updates test files to use new ClipboardPasteFallback method and removes async calls |
clipboard_paste_injector.rs |
New composite injector that combines clipboard setting with paste actions and automatic restoration |
clipboard_injector.rs |
Refactors to internal helper with automatic clipboard preservation |
manager.rs |
Major refactor to environment-first strategy ordering and simplified method registration |
lib.rs |
Updates module exports to reflect new clipboard paste architecture |
focus.rs |
Temporarily disables AT-SPI focus detection due to API changes |
| Documentation files | Updates to reflect new clipboard preservation behavior and unified strategy approach |
|
|
||
| #[test] | ||
| fn test_x11_detection() { | ||
| fn test_x1_detection() { |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected spelling of 'test_x1_detection' to 'test_x11_detection'.
| fn test_x1_detection() { | |
| fn test_x11_detection() { |
| let total_methods = method_order.len(); | ||
|
|
||
| for method in method_order { | ||
| for method in method_order.clone() { |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary clone of method_order vector. The loop doesn't modify the vector, so the original can be used directly.
| for method in method_order.clone() { | |
| for method in method_order { |
| use std::io::Write; | ||
| use std::process; |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The std::io::Write and std::process imports are only used in the fail-fast error handling path. Consider moving these imports closer to their usage or making them conditional with #[cfg] attributes if they're only used in specific configurations.
| use std::io::Write; | |
| use std::process; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
ColdVox/examples/inject_demo.rs
Lines 43 to 48 in ec66d28
| // Create injection configuration | |
| let config = InjectionConfig { | |
| allow_ydotool: true, | |
| allow_kdotool: false, | |
| allow_enigo: false, | |
| restore_clipboard: true, |
The commit drops the allow_ydotool and restore_clipboard fields from InjectionConfig, but call sites still initialize them in the demo and tests (InjectionConfig { allow_ydotool: true, … restore_clipboard: true, … }). With those struct members gone the example no longer compiles (struct InjectionConfig has no field named …), so cargo check --workspace will fail. These assignments need to be removed or migrated to whatever replaces those settings.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
- Remove unused combo_clip_atspi.rs file (dead code, no references) - Deduplicate method ordering logic into compute_method_order() helper - Link TODOs to tracking issues (#38, #40) and add context notes - All tests pass (50 passed, 0 failed) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Remove allow_ydotool field references (deleted in config refactor) - Remove restore_clipboard field references (restoration now unconditional) - Update combo_clip_ydotool.rs to always restore clipboard Fixes compilation errors in: - examples/inject_demo.rs - crates/app/tests/integration/text_injection_integration_test.rs - crates/app/tests/integration/mock_injection_tests.rs - crates/coldvox-text-injection/src/combo_clip_ydotool.rs
Summary
Part 6 of 9 in the domain-based refactor split.
Adds clipboard preservation and Wayland-first injection strategy with fallback chains.
Changes
Text Injection Crate (
crates/coldvox-text-injection/**)Backends
atspi_injector.rs: AT-SPI direct insertion (when available)clipboard_paste_injector.rs: Clipboard + paste composite strategyclipboard_injector.rs: Internal clipboard operations helperydotool_injector.rs: Wayland fallback via ydotoolkdotool_injector.rs: X11 supportenigo_injector.rs: Cross-platform fallbackManagement
manager.rs: StrategyManager with per-app success trackingsession.rs: Injection session handlingwindow_manager.rs: Focus and window trackingfocus.rs: AT-SPI focus detection (currently short-circuited due to API regression)Dependencies
Validation
Metrics
Review Notes
Stack position: 6/9
Previous PR: #5 (app-runtime-wav)
Next PR: #7 (testing)