fix(tui): try wl-copy before arboard on non-wlroots Wayland (1920)#1938
fix(tui): try wl-copy before arboard on non-wlroots Wayland (1920)#1938ousamabenyounes wants to merge 2 commits into
Conversation
arboard's Wayland backend targets wlr-data-control-unstable-v1, which non-wlroots compositors (niri, River, cosmic-comp, GNOME mutter) do not implement. Clipboard::set_text() silently returns Ok(()) without writing anything, so the existing OSC 52 fallback is never reached and copy appears successful but produces an empty clipboard. Reorder set_text() on Linux to try wl-copy first (uses the standard wl_data_device_manager protocol supported by every Wayland compositor), then fall through to arboard and OSC 52 only when wl-copy is missing or fails. The helper is parameterised so unit tests cover the success / missing binary / non-zero exit paths without depending on wl-copy being installed on the build host. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request implements a Linux-specific clipboard fallback using the wl-copy command-line utility to improve compatibility with Wayland compositors that do not support the wlr-data-control protocol. It introduces a helper function for executing external commands with piped input and adds corresponding unit tests. The feedback suggests using the bail! macro for more idiomatic error handling and including the process exit status in error messages to improve observability.
Address review feedback on the wl-copy clipboard fallback:
- Surface the process exit status in the failure message
(`bail!("{program} failed with {status}")`) so the cause is
visible when fallthrough to arboard / OSC 52 occurs.
- Switch to the `bail!` macro for the exit-code path, matching the
style of `osc52_sequence` elsewhere in this file.
While touching the helper, generalise the parameter to a `&[&str]`
argv slice. The exit-code test was racy against `/bin/false`'s
immediate stdin close (intermittent broken-pipe write errors instead
of the exit-status path); driving the new test through
`sh -c 'cat >/dev/null; exit 1'` drains stdin first and exercises the
status branch deterministically.
Co-Authored-By: Claude <noreply@anthropic.com>
|
Thanks for the careful Wayland report and scoped patch. I agree this is a real bug class for non-wlroots compositors, and the helper-level tests are useful. I am leaving this open rather than merging it into v0.8.42 because only GitGuardian has run here and I do not have a niri/non-wlroots Wayland repro environment in this release pass. Next maintainer pass should fetch the branch, run the wlcopy-focused tests under the current crate names, and ask for issue #1920 reporter verification on the actual compositor before closing the loop. |
- grep_files now respects cancellation token (Hmbown#1839, thanks @LING71671) - Ctrl+Z restores last cleared composer draft (Hmbown#1911, thanks @LING71671) - Clipboard works on non-wlroots Wayland via wl-copy (Hmbown#1938, thanks @ousamabenyounes)
…c, receipt truncation fix - SWE-bench: codewhale swebench run/export writes prediction JSONL from working-tree diff, with untracked-file inclusion via git add -N - CLI: --workspace / -C global flag forwards to TUI for file ops - CLI: codewhale exec --auto semantics clarified in help text - Markdown: table pipes inside inline code no longer create phantom columns (split_table_cells with backtick-awareness) - Receipt: floor_char_boundary prevents multibyte UTF-8 slice panic - Contributors: Ling (LING71671 Hmbown#1839 Hmbown#1911), Ben Younes (ousamabenyounes Hmbown#1938), jeoor npm fix (Hmbown#1860) credited across all 3 READMEs - ja-JP README: 19 contributors synced to parity with EN/zh-CN (80 each) - Docs: SWEBENCH.md, RECURSIVE_SELF_IMPROVEMENT.md, MODES.md exec clarification - Sub-agent footer: Alt+V hint now says 'details' not 'raw'
|
Thanks @ousamabenyounes — this landed in #1988 and is on main via commit 8878ac0. The shipped change tries wl-copy before arboard on Linux and keeps the helper-level tests for success, missing binary, and non-zero exit.\n\nI am closing this PR now that the code is on main. The release notes and README Thanks list both credit you for #1938. If the linked Wayland issue still needs reporter retest on niri or another non-wlroots compositor, that can stay tracked separately; this PR no longer needs to sit open after the harvest. Apologies that the harvest commit missed the exact 'Harvested from PR #1938 by @ousamabenyounes' trailer, which is why the auto-close workflow did not close this at merge time. |
Summary
Fix 1920
arboard's Wayland backend targets thewlr-data-control-unstable-v1protocol, which non-wlroots compositors (niri, River, cosmic-comp, GNOME mutter) do not implement.Clipboard::set_text()silently returnsOk(())without actually writing to the system clipboard, so the existing OSC 52 fallback is never reached and copy appears successful but produces an empty clipboard.Reorder the Linux branch of
set_text()to trywl-copyfirst (uses the standardwl_data_device_managerprotocol supported by every Wayland compositor), then fall through toarboardand OSC 52 only whenwl-copyis missing or fails.write_text_with_wlcopyinvoked beforearboardon Linux.write_text_with_wlcopy_using(program, text)so unit tests can swap the binary without depending onwl-copybeing on the build host (usescatfor success,falsefor non-zero exit, a non-existent name for missing binary).Testing
cargo test --bin deepseek-tui wlcopy— 3 new tests passcargo fmt --all -- --check— cleancargo clippy -p deepseek-tui --all-targets --all-features --no-deps— no new warningsTDD note
The 3 new tests guard the new
write_text_with_wlcopy_usinghelper's contract — success, missing binary, and non-zero exit. They are intentionally scoped to the helper becauseset_text's production branch is#[cfg(not(test))]and its fall-through ordering cannot be exercised from a unit test without refactoring the clipboard subsystem. The integration is verified by the issue reporter's repro on niri (the same compositor environment the patch targets).Checklist
Files changed
crates/tui/src/tui/clipboard.rswrite_text_with_wlcopy+ inner helper, reorderedset_texton Linux, 3 unit tests