a38cff59ea7efffaf79d3fcaaec3a15c6f0eee55
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
d5656b8060 |
fix(text-injection): route XWayland xdotool fallback through clipboard paste (#680)
* fix(text-injection): route XWayland xdotool fallback through clipboard paste xdotool type simulates keypresses against the active X keyboard layout, so a character the layout doesn't map comes out as the wrong glyph or garbled (#657). The native ydotool path already avoids this by pasting via the clipboard instead of typing (_inject_with_wayland_tool); the XWayland xdotool fallback (_inject_with_xdotool) never got the same treatment. Prefer clipboard paste on that fallback too when ydotool is installed, falling back to raw xdotool type only when no paste tool is available or the paste itself fails. Fixes #657 * fix(text-injection): X11 clipboard paste for the XWayland fallback jatinkrmalik's review on #680 pointed out the AppImage build does not bundle ydotool, so the prior fix's ydotool clipboard paste never ran on the actual reported setup; it fell through to the same layout-dependent xdotool type as before. It also flagged a real paste-target mismatch: an XWayland window reads the X11 CLIPBOARD selection, not the Wayland clipboard wl-copy/ydotool write to. The XWayland fallback now copies via xclip/xsel and pastes with xdotool key --clearmodifiers ctrl+v when either is installed, waiting for held modifiers to release first (the same wait already used by the native ydotool path). ydotool clipboard paste stays as a second fallback when xclip/xsel are absent; xdotool type is the last resort. New regression test drives inject_text() end-to-end with no ydotool on PATH, reproducing the reported AppImage/GNOME log rather than calling the fallback method directly, so it also covers the recovery check that could otherwise skip this path. * fix(text-injection): thread x11_only through clipboard read/clear so restore matches the paste backend jatinkrmalik's rebase pulled in the layout-aware paste work from #788, and greptile's review on the merged branch caught a real gap in this PR's own XWayland path: _copy_to_clipboard already takes x11_only to write the X11 CLIPBOARD via xclip/xsel (since that is what an xdotool paste actually reads), but _read_clipboard and _clear_clipboard never gained the same parameter. On a Wayland host, _read_clipboard prefers wl-paste, so the "previous clipboard" saved before an XWayland paste could be read from an unrelated Wayland clipboard, and restoring it back through _copy_to_clipboard with no x11_only wrote to that same wrong backend, leaving the X11 CLIPBOARD stuck on the dictated text. Threads x11_only through both helpers and every restore call site (the immediate-failure path and the delayed _restore closure), so an XWayland paste captures, clears, and restores the same X11 CLIPBOARD selection it wrote to. * fix(text-injection): annotate clipboard helpers and XWayland paste tests Greptile P2 on #680: give `_get_clipboard_tools` a return annotation and mark the new XWayland clipboard-paste regression tests `-> None`. * fix(text-injection): derive XWayland paste from environment Drop the ydotool/wl-copy second fallback on WAYLAND_XDOTOOL (wrong clipboard, and rc=0 is not a paste) and the x11_only flag stack. Paste picks xclip/xsel + xdotool from environment; user-facing copy stays on the session clipboard. * fix(text-injection): always pass host_env() to the paste chord The AST guard only recognizes a direct host_env() call. The ternary around it looked like we were handing the AppImage library path to xdotool. * test(text-injection): give XWayland injector stubs clipboard restore state _inject_with_xdotool now pastes first, and that path reads _clipboard_restore_target. The __new__ helper in test_text_injector_ext never set it. * fix(text-injection): skip clipboard restore on a stale paste failure If a newer paste has already copied its text, the older paste's failure handler must not write its saved clipboard back. Success restores already honored the generation counter; the failure path did not. * fix(text-injection): keep original clipboard across a stale paste failure Publish the restore target as soon as copy succeeds (first writer wins) so an overlapping paste inherits it. If the older paste then fails, the newer restore still writes the pre-first clipboard, not intermediate dictated text. --------- Co-authored-by: Jatin K Malik <jatinkrmalik@gmail.com> |
||
|
|
4a1d3ed9df |
fix(clipboard): text-only reads and safe overlapping restore (#646)
* fix(clipboard): text-only reads and safe overlapping restore Force wl-paste/xclip to request text MIME types so image/file clipboards are not decoded and "restored" as corrupted text. Stop treating xclip's "target not available" as an empty clipboard. Coordinate overlapping ydotool pastes with a generation counter and a pending restore target so a second paste within the 300ms window restores the original pre-injection content, not intermediate text. Co-authored-by: Jatin K Malik <jatinkrmalik@users.noreply.github.com> * refactor(clipboard): simplify restore ownership after ponytail review Bump the restore generation only after a successful clipboard overwrite so a failed copy cannot cancel an in-flight restore. Clear pending target on paste failure unconditionally. Drop redundant state reads in the restore thread and the artificial stale-restore unit test. Co-authored-by: Jatin K Malik <jatinkrmalik@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Jatin K Malik <jatinkrmalik@users.noreply.github.com> |
||
|
|
a15baa8170 |
fix: restore clipboard after ydotool clipboard-paste injection (#588)
* fix: restore clipboard after ydotool clipboard-paste injection When ydotool is used on Wayland to inject non-ASCII text (Arabic, accented letters, CJK, etc.), the injection path copies the text to the clipboard and simulates Ctrl+V. This permanently overwrites whatever the user had on their clipboard, which is surprising and disruptive — especially for users dictating in languages that always trigger this path. This commit adds two changes: 1. `_read_clipboard()` — reads the current clipboard content using the first available tool (wl-paste, xclip, xsel), returning None if the clipboard is empty or unreadable. 2. `_inject_via_clipboard_paste()` now saves the clipboard before overwriting it and schedules a background thread to restore it 300 ms after the Ctrl+V paste completes. The delay gives the target application time to process the paste before the clipboard content changes. The previous note "there is no safe race-free way to restore" was accurate in the strict sense, but a best-effort restore after a short delay is far better than never restoring, and covers all practical use cases (the user is not expected to trigger another paste within 300 ms of a dictation commit). Fixes the UX issue where every dictation commit silently destroyed the user's clipboard content. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * tests: add unit tests for clipboard save/restore (PR #588) 15 tests covering _read_clipboard() (tool fallback chain, error handling, UTF-8 decoding) and the restore behaviour in _inject_via_clipboard_paste (content restored after delay, no restore on empty clipboard or paste failure, daemon thread assertion). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qg9NpTr6QaeXEyEBc5fvvz * style: apply ruff formatting to test_clipboard_restore.py Fixes CI lint failure — ruff format requires line length ≤ 88 chars. No logic changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qg9NpTr6QaeXEyEBc5fvvz * style: apply black formatting to fix CI lint check ruff format and black disagree on some style choices; the CI runs black. No logic changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qg9NpTr6QaeXEyEBc5fvvz * fix: use text=True in _read_clipboard and respect copy_to_clipboard setting Two issues: 1. _read_clipboard() called subprocess.run without text=True, meaning stdout was bytes and .decode() was called. Existing tests that mock subprocess.run return stdout="" (str), causing AttributeError on .decode(). Switch to text=True + encoding/errors params so stdout is always a str and no .decode() is needed. 2. When the user has copy_to_clipboard=true, inject_text() copies the dictated text to clipboard after injection. The restore thread in _inject_via_clipboard_paste was firing 300ms later and overwriting that with the old content, silently breaking the setting. Skip the restore when _should_copy_to_clipboard() returns True. Tests: adds test_no_restore_when_copy_to_clipboard_setting_enabled, updates all _read_clipboard mock stdout to str (was bytes). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qg9NpTr6QaeXEyEBc5fvvz * fix: Python 3.9 compat and cover restore-failure branch Use Optional[str] instead of str | None for _read_clipboard return type so the code runs on Python 3.9 (PEP 604 union syntax requires 3.10+). Add test_restore_failure_is_handled_gracefully to exercise the else-branch inside the _restore() closure, covering the two previously uncovered lines reported by Codecov. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qg9NpTr6QaeXEyEBc5fvvz * fix: restore clipboard on paste failure and handle empty clipboard Fix 1 (comment #1): when Ctrl+V simulation fails the clipboard was already overwritten with dictated text but the original content was silently lost. Now restore it immediately in the except block before returning False. Fix 3 (comment #3): wl-paste and xclip signal an empty clipboard with a non-zero exit code, causing _read_clipboard() to return None and skip the restore — leaving dictated text in the clipboard forever. Detect the "Nothing is copied" / "target STRING not available" stderr patterns and return "" so the restore path can clear the clipboard back to empty. Tests added for both behaviours; existing test updated to assert the correct post-fix behaviour for paste failure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qg9NpTr6QaeXEyEBc5fvvz * test: cover paste-failure branch when clipboard is unreadable Add test for the case where Ctrl+V fails and previous_clipboard is None (clipboard was unreadable), ensuring the restore is correctly skipped. Closes the 1298->1300 branch gap reported by Codecov. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qg9NpTr6QaeXEyEBc5fvvz * test: cover wl-paste fallback on X11 (XWayland) path Add a test for the `not host_is_wayland and shutil.which("wl-paste")` branch in _read_clipboard(), which was showing as yellow/red on Codecov. Simulates an X11 session with wl-paste available (XWayland environments). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qg9NpTr6QaeXEyEBc5fvvz * fix: proper clipboard clear and race guard on restore Fix 5 (comment #5): add _clear_clipboard() that uses each backend's native clear command (wl-copy --clear, xsel --clear, xclip with empty input) instead of passing "" to _copy_to_clipboard(). Called when previous_clipboard is "" to truly clear rather than set an empty offer. Fix 6 (comment #6): before restoring, read the clipboard inside the _restore() thread and skip if it no longer holds the injected text — protecting against the user manually copying something else during the 300ms window. Tests added for _clear_clipboard() per backend and for both new restore-path behaviours. Existing restore tests updated to use side_effect lists since _read_clipboard() is now called twice. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qg9NpTr6QaeXEyEBc5fvvz * test: cover remaining _clear_clipboard() branches for Codecov Add tests for: - xclip path in _clear_clipboard() (line 931) - health-check skip (line 911) - exception fallthrough to next tool (line 947) - _clear_clipboard() called immediately when paste fails with empty previous clipboard (line 1354) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qg9NpTr6QaeXEyEBc5fvvz --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Jatin K Malik <jatinkrmalik@gmail.com> |