From 3e6355c10e54fa2ff4231c5dad40a6303b6d6b08 Mon Sep 17 00:00:00 2001 From: Eval Exec Date: Mon, 5 Oct 2026 23:05:27 +0800 Subject: [PATCH 1/5] refactor(clipboard): collapse the Wayland text policy into one module PR #482's data-control fallback re-implemented smithay-clipboard's MIME preference and decode rules, and both the fallback trigger and the arboard path classified native errors inline, including a string match on smithay's "selection is empty" message. Move the MIME vocabulary, the decode rules, the fallback decision, and the two native-error classifiers into the clipboard text_policy module, and give reads a typed answer: TextRead::{Text, NoSelection, TargetUnavailable}. "No owner" and "an owner with no readable text type" are now distinct inside the runtime, and smithay's message strings exist in exactly one adapter. This is behaviour-preserving: execute_command maps the typed answer back to the evaluator-facing Option, with both absences reading as nil, so batch semantics and the oracle pins are untouched. --- .../neomacs-display-runtime/src/clipboard.rs | 79 ++++--- .../src/clipboard/tests/clipboard_test.rs | 97 ++------- .../src/clipboard/tests/text_policy_test.rs | 195 ++++++++++++++++++ .../tests/wayland_data_control_test.rs | 53 +---- .../src/clipboard/text_policy.rs | 183 ++++++++++++++++ .../src/clipboard/wayland_data_control.rs | 59 +----- 6 files changed, 442 insertions(+), 224 deletions(-) create mode 100644 crates/neomacs-display-runtime/src/clipboard/tests/text_policy_test.rs create mode 100644 crates/neomacs-display-runtime/src/clipboard/text_policy.rs diff --git a/crates/neomacs-display-runtime/src/clipboard.rs b/crates/neomacs-display-runtime/src/clipboard.rs index 8534b37b88..52dbcf8b21 100644 --- a/crates/neomacs-display-runtime/src/clipboard.rs +++ b/crates/neomacs-display-runtime/src/clipboard.rs @@ -11,6 +11,8 @@ use arboard::{ClearExtLinux, GetExtLinux, LinuxClipboardKind, SetExtLinux}; #[cfg(target_os = "linux")] use raw_window_handle::{HasDisplayHandle, RawDisplayHandle}; +mod text_policy; + #[cfg(target_os = "linux")] mod wayland_data_control; @@ -21,7 +23,7 @@ trait ClipboardBackend: Send { fn set_text(&mut self, selection: ClipboardSelection, text: Option<&str>) -> Result<(), String>; - fn text(&mut self, selection: ClipboardSelection) -> Result, String>; + fn text(&mut self, selection: ClipboardSelection) -> Result; fn owner(&mut self, selection: ClipboardSelection) -> Result; } @@ -186,7 +188,11 @@ fn execute_command(backend: &mut dyn ClipboardBackend, command: ClipboardCommand ClipboardCommand::GetText { selection, reply, .. } => { - let result = backend.text(selection); + // The evaluator-facing wire carries text or absence; the two + // absence reasons stay inside the runtime (see `text_policy`). + let result = backend + .text(selection) + .map(text_policy::TextRead::into_option); if let Err(err) = &result { tracing::warn!(?selection, "clipboard read failed: {err}"); } @@ -289,11 +295,17 @@ impl ArboardClipboard { .map_err(|err| format!("failed to initialize the system clipboard: {err}")) } - fn text_result(result: Result) -> Result, String> { + /// Classify an arboard read into the clipboard's text vocabulary; this is + /// the adapter between arboard's error type and `text_policy`. + fn text_result( + result: Result, + ) -> Result { match result { - Ok(text) => Ok(Some(text)), - Err(arboard::Error::ContentNotAvailable) => Ok(None), - Err(err) => Err(err.to_string()), + Ok(text) => Ok(text_policy::TextRead::Text(text)), + Err(err) => match text_policy::classify_arboard(&err) { + Some(failure) => Ok(text_policy::TextRead::from(failure)), + None => Err(err.to_string()), + }, } } } @@ -336,7 +348,7 @@ impl ClipboardBackend for ArboardClipboard { } } - fn text(&mut self, selection: ClipboardSelection) -> Result, String> { + fn text(&mut self, selection: ClipboardSelection) -> Result { std::cfg_select! { target_os = "linux" => { let selection = match selection { @@ -348,7 +360,10 @@ impl ClipboardBackend for ArboardClipboard { _ => { match selection { ClipboardSelection::Clipboard => Self::text_result(self.clipboard.get_text()), - ClipboardSelection::Primary => Ok(self.primary.load()), + ClipboardSelection::Primary => Ok(match self.primary.load() { + Some(text) => text_policy::TextRead::Text(text), + None => text_policy::TextRead::NoSelection, + }), } } } @@ -396,7 +411,10 @@ impl WaylandClipboard { } } - fn data_control_text(&mut self) -> Result, String> { + /// The fallback's answer, or `None` when this compositor has no + /// data-control (or the reader could not be bound); the caller keeps + /// smithay's classification in that case. + fn data_control_text(&mut self) -> Result, String> { if let DataControl::Unbound = self.data_control { self.data_control = match self.bind_data_control() { Ok(Some(reader)) => DataControl::Bound(reader), @@ -408,7 +426,7 @@ impl WaylandClipboard { }; } match &mut self.data_control { - DataControl::Bound(reader) => reader.read_text(), + DataControl::Bound(reader) => Ok(Some(reader.read_text()?)), DataControl::Unbound | DataControl::Unavailable => Ok(None), } } @@ -428,35 +446,6 @@ impl WaylandClipboard { } } -/// Whether smithay-clipboard saw no usable selection offer. -/// -/// smithay-clipboard reports a missing offer as the untyped error -/// `selection is empty` and a missing text type as `NotFound`; both mean -/// "nothing to paste" rather than failure. Its other errors (no seat, no -/// keyboard focus) are real failures. -#[cfg(target_os = "linux")] -fn smithay_saw_no_offer(err: &std::io::Error) -> bool { - err.kind() == std::io::ErrorKind::NotFound || err.to_string() == "selection is empty" -} - -/// Map a smithay-clipboard read result, consulting `fallback` only when it -/// saw no usable selection offer. A failed or timed-out fallback reads as -/// an empty selection (nil), not an error. -#[cfg(target_os = "linux")] -fn smithay_text_or_fallback( - result: std::io::Result, - fallback: impl FnOnce() -> Result, String>, -) -> Result, String> { - match result { - Ok(text) => Ok(Some(text)), - Err(err) if smithay_saw_no_offer(&err) => fallback().or_else(|err| { - tracing::warn!("clipboard data-control read failed: {err}"); - Ok(None) - }), - Err(err) => Err(err.to_string()), - } -} - #[cfg(target_os = "linux")] impl ClipboardBackend for WaylandClipboard { fn set_text( @@ -479,16 +468,20 @@ impl ClipboardBackend for WaylandClipboard { Ok(()) } - fn text(&mut self, selection: ClipboardSelection) -> Result, String> { + fn text(&mut self, selection: ClipboardSelection) -> Result { match selection { ClipboardSelection::Clipboard => { - let result = self.clipboard.load(); - smithay_text_or_fallback(result, || self.data_control_text()) + // Hyprland delivers the selection only to winit's data device, + // so smithay's device can report an absence while the + // compositor holds a selection; only an absence consults the + // data-control fallback (see `text_policy::text_or_fallback`). + let native = self.clipboard.load(); + text_policy::text_or_fallback(native, || self.data_control_text()) } // smithay-clipboard owns the only primary-selection device, so // its result is complete. ClipboardSelection::Primary => { - smithay_text_or_fallback(self.clipboard.load_primary(), || Ok(None)) + text_policy::text_or_fallback(self.clipboard.load_primary(), || Ok(None)) } } } diff --git a/crates/neomacs-display-runtime/src/clipboard/tests/clipboard_test.rs b/crates/neomacs-display-runtime/src/clipboard/tests/clipboard_test.rs index c596225d76..09f52ecd39 100644 --- a/crates/neomacs-display-runtime/src/clipboard/tests/clipboard_test.rs +++ b/crates/neomacs-display-runtime/src/clipboard/tests/clipboard_test.rs @@ -37,8 +37,11 @@ impl ClipboardBackend for MemoryClipboard { Ok(()) } - fn text(&mut self, selection: ClipboardSelection) -> Result, String> { - Ok(self.selections.get(&selection).cloned()) + fn text(&mut self, selection: ClipboardSelection) -> Result { + Ok(match self.selections.get(&selection) { + Some(text) => text_policy::TextRead::Text(text.clone()), + None => text_policy::TextRead::NoSelection, + }) } fn owner(&mut self, selection: ClipboardSelection) -> Result { @@ -59,10 +62,10 @@ impl ClipboardBackend for BlockingClipboard { Ok(()) } - fn text(&mut self, _selection: ClipboardSelection) -> Result, String> { + fn text(&mut self, _selection: ClipboardSelection) -> Result { self.started.send(()).unwrap(); self.release.recv().unwrap(); - Ok(Some("released".to_owned())) + Ok(text_policy::TextRead::Text("released".to_owned())) } fn owner(&mut self, _selection: ClipboardSelection) -> Result { @@ -79,8 +82,8 @@ impl ClipboardBackend for BlockingDropClipboard { Ok(()) } - fn text(&mut self, _selection: ClipboardSelection) -> Result, String> { - Ok(None) + fn text(&mut self, _selection: ClipboardSelection) -> Result { + Ok(text_policy::TextRead::NoSelection) } fn owner(&mut self, _selection: ClipboardSelection) -> Result { @@ -105,10 +108,10 @@ impl ClipboardBackend for ExpiringClipboard { Ok(()) } - fn text(&mut self, _selection: ClipboardSelection) -> Result, String> { + fn text(&mut self, _selection: ClipboardSelection) -> Result { self.read_started.send(()).unwrap(); self.release_read.recv().unwrap(); - Ok(None) + Ok(text_policy::TextRead::NoSelection) } fn owner(&mut self, _selection: ClipboardSelection) -> Result { @@ -319,82 +322,14 @@ fn arboard_backend_keeps_primary_in_process_local_state() { .expect("PRIMARY store must not fail on this platform"); assert_eq!( backend.text(ClipboardSelection::Primary), - Ok(Some("selected".to_owned())) + Ok(text_policy::TextRead::Text("selected".to_owned())) ); backend .set_text(ClipboardSelection::Primary, None) .expect("PRIMARY disown must not fail on this platform"); - assert_eq!(backend.text(ClipboardSelection::Primary), Ok(None)); -} - -#[cfg(target_os = "linux")] -mod wayland_fallback { - use super::super::smithay_text_or_fallback; - use std::cell::Cell; - use std::io; - - fn no_offer() -> io::Result { - Err(io::Error::other("selection is empty")) - } - - #[test] - fn smithay_text_is_returned_without_consulting_data_control() { - let consulted = Cell::new(false); - let result = smithay_text_or_fallback(Ok("native".to_owned()), || { - consulted.set(true); - Ok(Some("data-control".to_owned())) - }); - assert_eq!(result, Ok(Some("native".to_owned()))); - assert!(!consulted.get()); - } - - #[test] - fn missing_selection_offer_reads_through_data_control() { - // Hyprland delivers the selection only to winit's data device, so - // smithay-clipboard's device never holds an offer. - let result = smithay_text_or_fallback(no_offer(), || Ok(Some("foreign".to_owned()))); - assert_eq!(result, Ok(Some("foreign".to_owned()))); - } - - #[test] - fn missing_text_mime_reads_through_data_control() { - let result = smithay_text_or_fallback( - Err(io::Error::new( - io::ErrorKind::NotFound, - "supported mime-type is not found", - )), - || Ok(Some("text from data-control".to_owned())), - ); - assert_eq!(result, Ok(Some("text from data-control".to_owned()))); - } - - #[test] - fn empty_transfer_is_text_not_a_missing_selection() { - let result = smithay_text_or_fallback(Ok(String::new()), || { - panic!("an empty transfer must not consult data-control") - }); - assert_eq!(result, Ok(Some(String::new()))); - } - - #[test] - fn seat_and_focus_errors_are_not_masked_by_data_control() { - for message in [ - "client doesn't have focus", - "no events received on any seat", - ] { - let result = smithay_text_or_fallback(Err(io::Error::other(message)), || { - panic!("{message} must not consult data-control") - }); - assert_eq!(result, Err(message.to_owned())); - } - } - - #[test] - fn failed_data_control_read_keeps_the_empty_result() { - let result = smithay_text_or_fallback(no_offer(), || { - Err("data-control transfer timed out".to_owned()) - }); - assert_eq!(result, Ok(None)); - } + assert_eq!( + backend.text(ClipboardSelection::Primary), + Ok(text_policy::TextRead::NoSelection) + ); } diff --git a/crates/neomacs-display-runtime/src/clipboard/tests/text_policy_test.rs b/crates/neomacs-display-runtime/src/clipboard/tests/text_policy_test.rs new file mode 100644 index 0000000000..d03530743e --- /dev/null +++ b/crates/neomacs-display-runtime/src/clipboard/tests/text_policy_test.rs @@ -0,0 +1,195 @@ +//! The text policy's decision table, pinned. +//! +//! Every row here is behaviour the backends must agree on: the MIME preference +//! and decode rules mirror smithay-clipboard 0.7.3 (`mime.rs::find_allowed`, +//! `state.rs` post-processing), and the classifiers are the only place its and +//! arboard's native errors become clipboard state. + +use super::*; +use std::cell::Cell; +use std::io; + +fn offered(mimes: &[&str]) -> Vec { + mimes.iter().map(|mime| (*mime).to_owned()).collect() +} + +fn no_offer() -> io::Result { + Err(io::Error::other("selection is empty")) +} + +fn no_text_mime() -> io::Result { + Err(io::Error::new( + io::ErrorKind::NotFound, + "supported mime-type is not found", + )) +} + +#[test] +fn utf8_types_win_in_offer_order_and_plain_text_is_a_fallback() { + assert_eq!( + TextMime::choose(&offered(&[ + "text/plain", + "UTF8_STRING", + "text/plain;charset=utf-8" + ])), + Some(TextMime::Utf8String) + ); + assert_eq!( + TextMime::choose(&offered(&["TEXT", "text/plain;charset=utf-8"])), + Some(TextMime::TextPlainUtf8) + ); + assert_eq!( + TextMime::choose(&offered(&["image/png", "text/plain"])), + Some(TextMime::TextPlain) + ); +} + +#[test] +fn offers_without_a_text_type_have_no_text() { + assert_eq!(TextMime::choose(&offered(&[])), None); + assert_eq!( + TextMime::choose(&offered(&[ + "image/png", + "text/plain;charset=iso-8859-1", + "STRING" + ])), + None + ); +} + +#[test] +fn plain_text_line_ends_are_normalized_like_smithay_clipboard() { + let bytes = b"a\r\nb\rc\n"; + assert_eq!(TextMime::TextPlainUtf8.decode(bytes), "a\nb\nc\n"); + assert_eq!(TextMime::TextPlain.decode(bytes), "a\nb\nc\n"); + assert_eq!(TextMime::Utf8String.decode(bytes), "a\r\nb\rc\n"); +} + +#[test] +fn invalid_utf8_is_decoded_lossily() { + assert_eq!(TextMime::Utf8String.decode(b"ok\xff"), "ok\u{fffd}"); +} + +#[test] +fn smithay_absences_keep_their_two_reasons() { + assert_eq!( + classify_smithay(&no_offer().unwrap_err()), + Some(TextReadFailure::NoSelection) + ); + assert_eq!( + classify_smithay(&no_text_mime().unwrap_err()), + Some(TextReadFailure::TargetUnavailable) + ); +} + +#[test] +fn smithay_failures_are_not_absences() { + for message in [ + "client doesn't have focus", + "no events received on any seat", + "active seat lost", + "clipboard is dead.", + ] { + assert_eq!( + classify_smithay(&io::Error::other(message)), + None, + "{message} is a failure, not an absence" + ); + } +} + +#[test] +fn arboard_content_not_available_is_the_only_absence() { + assert_eq!( + classify_arboard(&arboard::Error::ContentNotAvailable), + Some(TextReadFailure::NoSelection) + ); + for err in [ + arboard::Error::ClipboardNotSupported, + arboard::Error::ClipboardOccupied, + arboard::Error::ConversionFailure, + ] { + assert_eq!(classify_arboard(&err), None, "{err:?} is a failure"); + } +} + +#[test] +fn smithay_text_is_returned_without_consulting_data_control() { + let consulted = Cell::new(false); + let result = text_or_fallback(Ok("native".to_owned()), || { + consulted.set(true); + Ok(Some(TextRead::Text("data-control".to_owned()))) + }); + assert_eq!(result, Ok(TextRead::Text("native".to_owned()))); + assert!(!consulted.get()); +} + +#[test] +fn missing_selection_offer_reads_through_data_control() { + // Hyprland delivers the selection only to winit's data device, so + // smithay-clipboard's device never holds an offer. + let result = text_or_fallback(no_offer(), || { + Ok(Some(TextRead::Text("foreign".to_owned()))) + }); + assert_eq!(result, Ok(TextRead::Text("foreign".to_owned()))); +} + +#[test] +fn missing_text_mime_reads_through_data_control() { + let result = text_or_fallback(no_text_mime(), || { + Ok(Some(TextRead::Text("text from data-control".to_owned()))) + }); + assert_eq!( + result, + Ok(TextRead::Text("text from data-control".to_owned())) + ); +} + +#[test] +fn empty_transfer_is_text_not_a_missing_selection() { + let result = text_or_fallback(Ok(String::new()), || { + panic!("an empty transfer must not consult data-control") + }); + assert_eq!(result, Ok(TextRead::Text(String::new()))); +} + +#[test] +fn seat_and_focus_errors_are_not_masked_by_data_control() { + for message in [ + "client doesn't have focus", + "no events received on any seat", + ] { + let result = text_or_fallback(Err(io::Error::other(message)), || { + panic!("{message} must not consult data-control") + }); + assert_eq!(result, Err(message.to_owned())); + } +} + +#[test] +fn a_failed_fallback_keeps_smithays_absence() { + let result = text_or_fallback(no_offer(), || { + Err("data-control transfer timed out".to_owned()) + }); + assert_eq!(result, Ok(TextRead::NoSelection)); +} + +#[test] +fn a_compositor_without_data_control_keeps_smithays_absence() { + let result = text_or_fallback(no_text_mime(), || Ok(None)); + assert_eq!(result, Ok(TextRead::TargetUnavailable)); +} + +#[test] +fn only_text_reaches_the_evaluator_facing_wire() { + assert_eq!( + TextRead::Text("x".to_owned()).into_option(), + Some("x".to_owned()) + ); + assert_eq!( + TextRead::Text(String::new()).into_option(), + Some(String::new()) + ); + assert_eq!(TextRead::NoSelection.into_option(), None); + assert_eq!(TextRead::TargetUnavailable.into_option(), None); +} diff --git a/crates/neomacs-display-runtime/src/clipboard/tests/wayland_data_control_test.rs b/crates/neomacs-display-runtime/src/clipboard/tests/wayland_data_control_test.rs index aa409d0145..599f7131b3 100644 --- a/crates/neomacs-display-runtime/src/clipboard/tests/wayland_data_control_test.rs +++ b/crates/neomacs-display-runtime/src/clipboard/tests/wayland_data_control_test.rs @@ -1,54 +1,7 @@ -use super::*; - -fn offered(mimes: &[&str]) -> Vec { - mimes.iter().map(|mime| (*mime).to_owned()).collect() -} - -#[test] -fn utf8_types_win_in_offer_order_and_plain_text_is_a_fallback() { - assert_eq!( - TextMime::choose(&offered(&[ - "text/plain", - "UTF8_STRING", - "text/plain;charset=utf-8" - ])), - Some(TextMime::Utf8String) - ); - assert_eq!( - TextMime::choose(&offered(&["TEXT", "text/plain;charset=utf-8"])), - Some(TextMime::TextPlainUtf8) - ); - assert_eq!( - TextMime::choose(&offered(&["image/png", "text/plain"])), - Some(TextMime::TextPlain) - ); -} +//! Transfer-mechanics tests for the data-control reader. The MIME policy it +//! shares with the other backends is tested in `text_policy_test.rs`. -#[test] -fn offers_without_a_text_type_have_no_text() { - assert_eq!(TextMime::choose(&offered(&[])), None); - assert_eq!( - TextMime::choose(&offered(&[ - "image/png", - "text/plain;charset=iso-8859-1", - "STRING" - ])), - None - ); -} - -#[test] -fn plain_text_line_ends_are_normalized_like_smithay_clipboard() { - let bytes = b"a\r\nb\rc\n"; - assert_eq!(TextMime::TextPlainUtf8.decode(bytes), "a\nb\nc\n"); - assert_eq!(TextMime::TextPlain.decode(bytes), "a\nb\nc\n"); - assert_eq!(TextMime::Utf8String.decode(bytes), "a\r\nb\rc\n"); -} - -#[test] -fn invalid_utf8_is_decoded_lossily() { - assert_eq!(TextMime::Utf8String.decode(b"ok\xff"), "ok\u{fffd}"); -} +use super::*; #[test] fn transfer_reads_until_the_owner_closes_its_end() { diff --git a/crates/neomacs-display-runtime/src/clipboard/text_policy.rs b/crates/neomacs-display-runtime/src/clipboard/text_policy.rs new file mode 100644 index 0000000000..92df78e656 --- /dev/null +++ b/crates/neomacs-display-runtime/src/clipboard/text_policy.rs @@ -0,0 +1,183 @@ +//! The one clipboard text policy: MIME preference, decoding, and absence +//! classification. +//! +//! Three backends can answer a text read — smithay-clipboard's `wl_data_device` +//! path, the `ext-data-control-v1` fallback, and arboard (X11 and the other +//! platforms) — and they must all answer it the same way. This module owns +//! the parts of that answer we control: +//! +//! * which MIME spellings count as text and in what order (`TextMime::choose`, +//! matching smithay-clipboard's `mime.rs::find_allowed`); +//! * how transferred bytes become a `String` (`TextMime::decode`, matching +//! smithay-clipboard's `state.rs` post-processing); +//! * which native errors mean "nothing to paste" rather than failure +//! (`classify_smithay`, `classify_arboard`); +//! * when the data-control fallback may answer for smithay +//! (`text_or_fallback`). +//! +//! # What this module does not do +//! +//! It does not decode for smithay-clipboard or arboard. Both pick and decode a +//! representation internally and hand back a `String` (arboard even decodes +//! X11's Latin-1 `STRING` atom), and smithay-clipboard never exposes the +//! offer's MIME list. Their choice is dependency behaviour we cannot +//! intercept, so the policy classifies their outcomes instead. The raw-bytes +//! decode path (the data-control reader) is the one this module fully owns; a +//! backend that receives bytes routes them through `choose`/`decode` here +//! rather than growing a fourth copy. + +/// The absence half of a text read, before there is a payload. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(super) enum TextReadFailure { + /// No selection owner. + NoSelection, + /// An owner exists, but it offers none of the text MIMEs this backend reads. + TargetUnavailable, +} + +impl From for TextRead { + fn from(failure: TextReadFailure) -> Self { + match failure { + TextReadFailure::NoSelection => Self::NoSelection, + TextReadFailure::TargetUnavailable => Self::TargetUnavailable, + } + } +} + +/// What a best-available text read produced. +#[derive(Debug, Eq, PartialEq)] +pub(super) enum TextRead { + /// An owner published text (possibly the empty string). + Text(String), + /// No selection owner. + NoSelection, + /// An owner exists, but it offers none of the text MIMEs we read. + TargetUnavailable, +} + +impl TextRead { + /// The answer the evaluator-facing wire carries: text, or `None` for + /// either absence. The two absences stay distinct inside the runtime so + /// callers can tell "nothing is selected" from "something is selected that + /// we cannot read". + pub(super) fn into_option(self) -> Option { + match self { + Self::Text(text) => Some(text), + Self::NoSelection | Self::TargetUnavailable => None, + } + } +} + +/// The text MIME spellings, in smithay-clipboard's preference order. +#[cfg(target_os = "linux")] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(super) enum TextMime { + TextPlainUtf8, + Utf8String, + TextPlain, +} + +#[cfg(target_os = "linux")] +impl TextMime { + pub(super) fn as_str(self) -> &'static str { + match self { + Self::TextPlainUtf8 => "text/plain;charset=utf-8", + Self::Utf8String => "UTF8_STRING", + Self::TextPlain => "text/plain", + } + } + + /// The first UTF-8 type offered, else plain text as a fallback; mirrors + /// smithay-clipboard's `MimeType::find_allowed` exactly, so the + /// data-control reader and smithay answer the same offer identically. + pub(super) fn choose(offered: &[String]) -> Option { + let mut fallback = None; + for mime in offered { + match mime.as_str() { + "text/plain;charset=utf-8" => return Some(Self::TextPlainUtf8), + "UTF8_STRING" => return Some(Self::Utf8String), + "text/plain" => fallback = Some(Self::TextPlain), + _ => {} + } + } + fallback + } + + /// Decode transferred bytes the way smithay-clipboard does: lossy UTF-8, + /// and CR/CRLF line ends normalized to LF for the `text/plain` types. + pub(super) fn decode(self, bytes: &[u8]) -> String { + let text = String::from_utf8_lossy(bytes).into_owned(); + match self { + Self::TextPlainUtf8 | Self::TextPlain => text.replace("\r\n", "\n").replace('\r', "\n"), + Self::Utf8String => text, + } + } +} + +/// Classify a smithay-clipboard read error. +/// +/// This is the only place its message strings appear. They are matched +/// because smithay-clipboard 0.7.3 exposes no typed error for them: an unowned +/// selection is `Error::other("selection is empty")` and an offer without a +/// readable text type is `Error::new(NotFound, "supported mime-type is not +/// found")` (`state.rs:149-186`). Anything else — no keyboard focus, no seat, +/// a dead worker — is a real failure and returns `None`. +#[cfg(target_os = "linux")] +pub(super) fn classify_smithay(err: &std::io::Error) -> Option { + if err.kind() == std::io::ErrorKind::NotFound + || err.to_string() == "supported mime-type is not found" + { + return Some(TextReadFailure::TargetUnavailable); + } + match err.to_string().as_str() { + "selection is empty" => Some(TextReadFailure::NoSelection), + _ => None, + } +} + +/// Classify an arboard read error. +/// +/// arboard's `ContentNotAvailable` covers both "the clipboard is empty" and +/// "the contents have an incompatible format" (`common.rs:19-24`), so it can +/// never name a specific absence; the conservative answer is `NoSelection`. +/// Every other variant is a real failure. (`Error` is `#[non_exhaustive]`, so +/// the catch-all is required.) +pub(super) fn classify_arboard(err: &arboard::Error) -> Option { + match err { + arboard::Error::ContentNotAvailable => Some(TextReadFailure::NoSelection), + _ => None, + } +} + +/// Classify a smithay read, consulting `fallback` only when smithay saw no +/// usable text offer. +/// +/// A failed fallback is logged and smithay's answer is kept: absence stays +/// absence, and the caller must not turn it into an error. A smithay error +/// that does not classify (no focus, no seat, dead worker) is a real failure +/// and is returned without asking the fallback. +#[cfg(target_os = "linux")] +pub(super) fn text_or_fallback( + result: std::io::Result, + fallback: impl FnOnce() -> Result, String>, +) -> Result { + let native = match result { + Ok(text) => return Ok(TextRead::Text(text)), + Err(err) => match classify_smithay(&err) { + Some(failure) => TextRead::from(failure), + None => return Err(err.to_string()), + }, + }; + match fallback() { + Ok(Some(read)) => Ok(read), + Ok(None) => Ok(native), + Err(err) => { + tracing::warn!("clipboard data-control read failed: {err}"); + Ok(native) + } + } +} + +#[cfg(all(test, target_os = "linux"))] +#[path = "tests/text_policy_test.rs"] +mod tests; diff --git a/crates/neomacs-display-runtime/src/clipboard/wayland_data_control.rs b/crates/neomacs-display-runtime/src/clipboard/wayland_data_control.rs index 48b3c11c91..af320daa35 100644 --- a/crates/neomacs-display-runtime/src/clipboard/wayland_data_control.rs +++ b/crates/neomacs-display-runtime/src/clipboard/wayland_data_control.rs @@ -15,6 +15,8 @@ //! read, takes the selection the compositor sends on creation, transfers it //! and destroys every object it was given. +use super::text_policy::{TextMime, TextRead}; + use std::ffi::c_void; use std::io::{ErrorKind, Read}; use std::os::fd::AsFd; @@ -37,49 +39,6 @@ use wayland_protocols::ext::data_control::v1::client::{ /// a stalled selection owner fails this read instead of the next request. const TRANSFER_TIMEOUT: Duration = Duration::from_secs(3); -/// Text MIME types in smithay-clipboard's preference order, so both read -/// paths choose the same representation of a selection. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(super) enum TextMime { - TextPlainUtf8, - Utf8String, - TextPlain, -} - -impl TextMime { - fn as_str(self) -> &'static str { - match self { - Self::TextPlainUtf8 => "text/plain;charset=utf-8", - Self::Utf8String => "UTF8_STRING", - Self::TextPlain => "text/plain", - } - } - - /// The first UTF-8 type offered, else plain text as a fallback. - pub(super) fn choose(offered: &[String]) -> Option { - let mut fallback = None; - for mime in offered { - match mime.as_str() { - "text/plain;charset=utf-8" => return Some(Self::TextPlainUtf8), - "UTF8_STRING" => return Some(Self::Utf8String), - "text/plain" => fallback = Some(Self::TextPlain), - _ => {} - } - } - fallback - } - - /// Decode transferred bytes the way smithay-clipboard does: lossy UTF-8, - /// and CR/CRLF line ends normalized to LF for the `text/plain` types. - pub(super) fn decode(self, bytes: &[u8]) -> String { - let text = String::from_utf8_lossy(bytes).into_owned(); - match self { - Self::TextPlainUtf8 | Self::TextPlain => text.replace("\r\n", "\n").replace('\r', "\n"), - Self::Utf8String => text, - } - } -} - #[derive(Default)] struct ReaderState { /// Every offer introduced while the per-read device existed. @@ -199,9 +158,9 @@ impl DataControlReader { })) } - /// The current CLIPBOARD selection as text: `Ok(None)` when there is no - /// selection or it offers no text type. - pub(super) fn read_text(&mut self) -> Result, String> { + /// The current CLIPBOARD selection as text, keeping "no owner" distinct + /// from "an owner with no readable text type". + pub(super) fn read_text(&mut self) -> Result { let mut state = ReaderState::default(); let device = self .manager @@ -226,9 +185,9 @@ impl DataControlReader { fn transfer( &self, selection: Option<&ext_data_control_offer_v1::ExtDataControlOfferV1>, - ) -> Result, String> { + ) -> Result { let Some(offer) = selection else { - return Ok(None); + return Ok(TextRead::NoSelection); }; let mime = { let mimes = offer @@ -238,7 +197,7 @@ impl DataControlReader { TextMime::choose(&mimes) }; let Some(mime) = mime else { - return Ok(None); + return Ok(TextRead::TargetUnavailable); }; let (mut reader, writer) = std::io::pipe().map_err(|err| format!("data-control pipe: {err}"))?; @@ -252,7 +211,7 @@ impl DataControlReader { Err(err) => return Err(format!("data-control flush: {err}")), } let bytes = read_to_end_before(&mut reader, Instant::now() + TRANSFER_TIMEOUT)?; - Ok(Some(mime.decode(&bytes))) + Ok(TextRead::Text(mime.decode(&bytes))) } } From ec18e99c958db2a9977a3685d6fd9716c96757e5 Mon Sep 17 00:00:00 2001 From: Eval Exec Date: Mon, 5 Oct 2026 23:05:32 +0800 Subject: [PATCH 2/5] test(clipboard): pin the backend text contract Add a contract test suite for ClipboardBackend: an absent selection is an answer rather than a transport error, an empty string is text, and CLIPBOARD and PRIMARY never alias. Two deliberate violators -- a backend that reports absence as an error, and one that answers both selections from a single slot -- prove the assertions can fail, and an opt-in live check runs the same contract against the real system clipboard. --- .../neomacs-display-runtime/src/clipboard.rs | 4 + .../src/clipboard/tests/contract_test.rs | 193 ++++++++++++++++++ 2 files changed, 197 insertions(+) create mode 100644 crates/neomacs-display-runtime/src/clipboard/tests/contract_test.rs diff --git a/crates/neomacs-display-runtime/src/clipboard.rs b/crates/neomacs-display-runtime/src/clipboard.rs index 52dbcf8b21..737e57e997 100644 --- a/crates/neomacs-display-runtime/src/clipboard.rs +++ b/crates/neomacs-display-runtime/src/clipboard.rs @@ -494,3 +494,7 @@ impl ClipboardBackend for WaylandClipboard { #[cfg(test)] #[path = "clipboard/tests/clipboard_test.rs"] mod tests; + +#[cfg(test)] +#[path = "clipboard/tests/contract_test.rs"] +mod contract_tests; diff --git a/crates/neomacs-display-runtime/src/clipboard/tests/contract_test.rs b/crates/neomacs-display-runtime/src/clipboard/tests/contract_test.rs new file mode 100644 index 0000000000..29ce7f726a --- /dev/null +++ b/crates/neomacs-display-runtime/src/clipboard/tests/contract_test.rs @@ -0,0 +1,193 @@ +//! The contract every [`ClipboardBackend`] satisfies, and the fakes that prove +//! the assertions have teeth. +//! +//! The contract is deliberately about the *shape* of an answer, not about any +//! one backend's mechanism: +//! +//! 1. an absent selection is an answer (`TextRead::NoSelection` / +//! `TextRead::TargetUnavailable`), never an `Err` — `Err` is reserved for +//! transport and worker failures; +//! 2. text round-trips where the backend can read the selection it set; +//! 3. `Text("")` is text, not absence; +//! 4. CLIPBOARD and PRIMARY never alias. +//! +//! What these tests cannot see: the real X11 owner/requestor exchange, the +//! Wayland data-control protocol against a compositor, and cross-process +//! ownership. The GUI test job covers PRIMARY end to end; the live check at +//! the bottom is opt-in. + +use super::*; + +/// The smallest backend that honours the contract: one owned string per +/// selection. +#[derive(Default)] +struct FakeBackend { + selections: std::collections::HashMap, +} + +impl ClipboardBackend for FakeBackend { + fn set_text( + &mut self, + selection: ClipboardSelection, + text: Option<&str>, + ) -> Result<(), String> { + match text { + Some(text) => { + self.selections.insert(selection, text.to_owned()); + } + None => { + self.selections.remove(&selection); + } + } + Ok(()) + } + + fn text(&mut self, selection: ClipboardSelection) -> Result { + Ok(match self.selections.get(&selection) { + Some(text) => text_policy::TextRead::Text(text.clone()), + None => text_policy::TextRead::NoSelection, + }) + } + + fn owner(&mut self, selection: ClipboardSelection) -> Result { + Ok(if self.selections.contains_key(&selection) { + SelectionOwner::ThisProcess + } else { + SelectionOwner::None + }) + } +} + +/// Reports every absence as a transport error; the contract's first check +/// must catch this. +struct LyingBackend; + +impl ClipboardBackend for LyingBackend { + fn set_text( + &mut self, + _selection: ClipboardSelection, + _text: Option<&str>, + ) -> Result<(), String> { + Ok(()) + } + + fn text(&mut self, _selection: ClipboardSelection) -> Result { + Err("selection is empty".to_owned()) + } + + fn owner(&mut self, _selection: ClipboardSelection) -> Result { + Ok(SelectionOwner::Unknown) + } +} + +/// Answers both selections from one slot; the contract's alias check must +/// catch this. +#[derive(Default)] +struct AliasingBackend { + value: String, +} + +impl ClipboardBackend for AliasingBackend { + fn set_text( + &mut self, + _selection: ClipboardSelection, + text: Option<&str>, + ) -> Result<(), String> { + self.value = text.unwrap_or_default().to_owned(); + Ok(()) + } + + fn text(&mut self, _selection: ClipboardSelection) -> Result { + Ok(text_policy::TextRead::Text(self.value.clone())) + } + + fn owner(&mut self, _selection: ClipboardSelection) -> Result { + Ok(SelectionOwner::Unknown) + } +} + +/// Assert the text contract against any backend. +fn assert_contract(backend: &mut dyn ClipboardBackend) { + const PROBE: &str = "neomacs clipboard contract probe 3f8a2c"; + let clipboard = ClipboardSelection::Clipboard; + let primary = ClipboardSelection::Primary; + + // 1. Absence is an answer, not a transport error. + backend + .text(clipboard) + .expect("absence must be an answer, not a transport error"); + + // 2. Text round-trips. + backend + .set_text(clipboard, Some(PROBE)) + .expect("setting text must not fail"); + assert_eq!( + backend.text(clipboard).expect("reading text must not fail"), + text_policy::TextRead::Text(PROBE.to_owned()), + "a backend must read back the text it was given" + ); + + // 3. An empty string is text, not absence. + backend + .set_text(clipboard, Some("")) + .expect("setting empty text must not fail"); + assert_eq!( + backend.text(clipboard).expect("reading text must not fail"), + text_policy::TextRead::Text(String::new()), + "an empty selection is text, not absence" + ); + + // 4. CLIPBOARD and PRIMARY never alias. + backend + .set_text(clipboard, Some(PROBE)) + .expect("setting text must not fail"); + assert_ne!( + backend + .text(primary) + .expect("reading PRIMARY must not fail"), + text_policy::TextRead::Text(PROBE.to_owned()), + "PRIMARY must not alias CLIPBOARD" + ); + + // 1 again, for the cleared state. + backend + .set_text(clipboard, None) + .expect("clearing must not fail"); + let cleared = backend + .text(clipboard) + .expect("absence must be an answer, not a transport error"); + assert_ne!( + cleared, + text_policy::TextRead::Text(String::new()), + "a cleared selection must not read as empty text" + ); +} + +#[test] +fn the_fake_backend_satisfies_the_text_contract() { + assert_contract(&mut FakeBackend::default()); +} + +#[test] +#[should_panic(expected = "absence must be an answer")] +fn a_backend_that_reports_absence_as_an_error_fails_the_contract() { + assert_contract(&mut LyingBackend); +} + +#[test] +#[should_panic(expected = "PRIMARY must not alias CLIPBOARD")] +fn a_backend_that_aliases_the_selections_fails_the_contract() { + assert_contract(&mut AliasingBackend::default()); +} + +/// Opt-in live check against the real system clipboard. It writes and clears +/// the CLIPBOARD selection, so it runs only when +/// `NEOMACS_CLIPBOARD_LIVE_TEST` is set. +#[test] +fn live_backend_satisfies_the_text_contract() { + if std::env::var_os("NEOMACS_CLIPBOARD_LIVE_TEST").is_none() { + return; + } + let mut backend = ArboardClipboard::new().expect("system clipboard should open"); + assert_contract(&mut backend); +} From ae6bdd7c32bf88e96f784880ba4210bd39592a50 Mon Sep 17 00:00:00 2001 From: Eval Exec Date: Mon, 5 Oct 2026 23:27:06 +0800 Subject: [PATCH 3/5] refactor(clipboard): clarify the absence type and pin fallback routing Review follow-ups on #487. Copilot noted that TextReadFailure reads backwards: the classifiers answer Some for a recognized absence and None for a real error, so a type named "failure" lands on the Ok side of a match. Rename it to TextAbsence, which matches the side of the Option it appears on and the module's thesis that an absence is an answer. CodeRabbit noted that no test reached the CLIPBOARD-versus-PRIMARY fallback routing, so changing either caller closure could disable the Hyprland fallback, or route a PRIMARY absence through data-control, without failing anything. Move the routing into text_policy::wayland_read, with the native result passed in, so it is testable without a live display, and pin the outcomes: CLIPBOARD consults the fallback, PRIMARY never does. --- .../neomacs-display-runtime/src/clipboard.rs | 24 +++----- .../src/clipboard/tests/text_policy_test.rs | 35 +++++++++++- .../src/clipboard/text_policy.rs | 56 ++++++++++++++----- 3 files changed, 84 insertions(+), 31 deletions(-) diff --git a/crates/neomacs-display-runtime/src/clipboard.rs b/crates/neomacs-display-runtime/src/clipboard.rs index 737e57e997..1ec9bc7ae2 100644 --- a/crates/neomacs-display-runtime/src/clipboard.rs +++ b/crates/neomacs-display-runtime/src/clipboard.rs @@ -469,21 +469,15 @@ impl ClipboardBackend for WaylandClipboard { } fn text(&mut self, selection: ClipboardSelection) -> Result { - match selection { - ClipboardSelection::Clipboard => { - // Hyprland delivers the selection only to winit's data device, - // so smithay's device can report an absence while the - // compositor holds a selection; only an absence consults the - // data-control fallback (see `text_policy::text_or_fallback`). - let native = self.clipboard.load(); - text_policy::text_or_fallback(native, || self.data_control_text()) - } - // smithay-clipboard owns the only primary-selection device, so - // its result is complete. - ClipboardSelection::Primary => { - text_policy::text_or_fallback(self.clipboard.load_primary(), || Ok(None)) - } - } + // Hyprland delivers the selection only to winit's data device, so + // smithay's device can report an absence while the compositor holds a + // selection. Which selections may then consult the data-control + // fallback is `text_policy::wayland_read`'s business. + let native = match selection { + ClipboardSelection::Clipboard => self.clipboard.load(), + ClipboardSelection::Primary => self.clipboard.load_primary(), + }; + text_policy::wayland_read(selection, native, || self.data_control_text()) } fn owner(&mut self, _selection: ClipboardSelection) -> Result { diff --git a/crates/neomacs-display-runtime/src/clipboard/tests/text_policy_test.rs b/crates/neomacs-display-runtime/src/clipboard/tests/text_policy_test.rs index d03530743e..6dbfaf07a1 100644 --- a/crates/neomacs-display-runtime/src/clipboard/tests/text_policy_test.rs +++ b/crates/neomacs-display-runtime/src/clipboard/tests/text_policy_test.rs @@ -74,11 +74,11 @@ fn invalid_utf8_is_decoded_lossily() { fn smithay_absences_keep_their_two_reasons() { assert_eq!( classify_smithay(&no_offer().unwrap_err()), - Some(TextReadFailure::NoSelection) + Some(TextAbsence::NoSelection) ); assert_eq!( classify_smithay(&no_text_mime().unwrap_err()), - Some(TextReadFailure::TargetUnavailable) + Some(TextAbsence::TargetUnavailable) ); } @@ -102,7 +102,7 @@ fn smithay_failures_are_not_absences() { fn arboard_content_not_available_is_the_only_absence() { assert_eq!( classify_arboard(&arboard::Error::ContentNotAvailable), - Some(TextReadFailure::NoSelection) + Some(TextAbsence::NoSelection) ); for err in [ arboard::Error::ClipboardNotSupported, @@ -180,6 +180,35 @@ fn a_compositor_without_data_control_keeps_smithays_absence() { assert_eq!(result, Ok(TextRead::TargetUnavailable)); } +#[test] +fn a_clipboard_absence_may_be_answered_by_data_control() { + let consulted = Cell::new(false); + let result = wayland_read(ClipboardSelection::Clipboard, no_offer(), || { + consulted.set(true); + Ok(Some(TextRead::Text("foreign".to_owned()))) + }); + assert!(consulted.get(), "CLIPBOARD must consult the fallback"); + assert_eq!(result, Ok(TextRead::Text("foreign".to_owned()))); +} + +#[test] +fn a_primary_absence_never_consults_data_control() { + let result = wayland_read(ClipboardSelection::Primary, no_offer(), || { + panic!("PRIMARY must not consult the data-control fallback") + }); + assert_eq!(result, Ok(TextRead::NoSelection)); +} + +#[test] +fn primary_text_is_returned_without_consulting_data_control() { + let result = wayland_read( + ClipboardSelection::Primary, + Ok("selected".to_owned()), + || panic!("PRIMARY text must not consult the data-control fallback"), + ); + assert_eq!(result, Ok(TextRead::Text("selected".to_owned()))); +} + #[test] fn only_text_reaches_the_evaluator_facing_wire() { assert_eq!( diff --git a/crates/neomacs-display-runtime/src/clipboard/text_policy.rs b/crates/neomacs-display-runtime/src/clipboard/text_policy.rs index 92df78e656..361019b25c 100644 --- a/crates/neomacs-display-runtime/src/clipboard/text_policy.rs +++ b/crates/neomacs-display-runtime/src/clipboard/text_policy.rs @@ -25,21 +25,32 @@ //! decode path (the data-control reader) is the one this module fully owns; a //! backend that receives bytes routes them through `choose`/`decode` here //! rather than growing a fourth copy. +//! +//! Only the smithay-clipboard rows are Linux-gated. `TextRead` and +//! `classify_arboard` serve every platform, because arboard is the X11, macOS, +//! and Windows backend alike and none of them needs a different text policy +//! today. + +#[cfg(target_os = "linux")] +use crate::thread_comm::ClipboardSelection; -/// The absence half of a text read, before there is a payload. +/// Why a text read produced no text: a recognized absence, not a failure. +/// +/// The classifiers answer `Some` for these and `None` for a real error, so the +/// name matches the side of the `Option` it appears on. #[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub(super) enum TextReadFailure { +pub(super) enum TextAbsence { /// No selection owner. NoSelection, /// An owner exists, but it offers none of the text MIMEs this backend reads. TargetUnavailable, } -impl From for TextRead { - fn from(failure: TextReadFailure) -> Self { - match failure { - TextReadFailure::NoSelection => Self::NoSelection, - TextReadFailure::TargetUnavailable => Self::TargetUnavailable, +impl From for TextRead { + fn from(absence: TextAbsence) -> Self { + match absence { + TextAbsence::NoSelection => Self::NoSelection, + TextAbsence::TargetUnavailable => Self::TargetUnavailable, } } } @@ -123,14 +134,14 @@ impl TextMime { /// found")` (`state.rs:149-186`). Anything else — no keyboard focus, no seat, /// a dead worker — is a real failure and returns `None`. #[cfg(target_os = "linux")] -pub(super) fn classify_smithay(err: &std::io::Error) -> Option { +pub(super) fn classify_smithay(err: &std::io::Error) -> Option { if err.kind() == std::io::ErrorKind::NotFound || err.to_string() == "supported mime-type is not found" { - return Some(TextReadFailure::TargetUnavailable); + return Some(TextAbsence::TargetUnavailable); } match err.to_string().as_str() { - "selection is empty" => Some(TextReadFailure::NoSelection), + "selection is empty" => Some(TextAbsence::NoSelection), _ => None, } } @@ -142,9 +153,9 @@ pub(super) fn classify_smithay(err: &std::io::Error) -> Option /// never name a specific absence; the conservative answer is `NoSelection`. /// Every other variant is a real failure. (`Error` is `#[non_exhaustive]`, so /// the catch-all is required.) -pub(super) fn classify_arboard(err: &arboard::Error) -> Option { +pub(super) fn classify_arboard(err: &arboard::Error) -> Option { match err { - arboard::Error::ContentNotAvailable => Some(TextReadFailure::NoSelection), + arboard::Error::ContentNotAvailable => Some(TextAbsence::NoSelection), _ => None, } } @@ -164,7 +175,7 @@ pub(super) fn text_or_fallback( let native = match result { Ok(text) => return Ok(TextRead::Text(text)), Err(err) => match classify_smithay(&err) { - Some(failure) => TextRead::from(failure), + Some(absence) => TextRead::from(absence), None => return Err(err.to_string()), }, }; @@ -178,6 +189,25 @@ pub(super) fn text_or_fallback( } } +/// The Wayland read policy for one selection: consult the data-control +/// fallback only for CLIPBOARD. +/// +/// smithay-clipboard owns the only primary-selection device and the reader +/// speaks CLIPBOARD only, so a PRIMARY absence is final. The native result is +/// passed in rather than read here so the routing is testable without a live +/// display. +#[cfg(target_os = "linux")] +pub(super) fn wayland_read( + selection: ClipboardSelection, + native: std::io::Result, + data_control: impl FnOnce() -> Result, String>, +) -> Result { + match selection { + ClipboardSelection::Clipboard => text_or_fallback(native, data_control), + ClipboardSelection::Primary => text_or_fallback(native, || Ok(None)), + } +} + #[cfg(all(test, target_os = "linux"))] #[path = "tests/text_policy_test.rs"] mod tests; From ad2aad315658470696500fcb5b55056d3037b8a6 Mon Sep 17 00:00:00 2001 From: Eval Exec Date: Mon, 5 Oct 2026 23:35:52 +0800 Subject: [PATCH 4/5] refactor(clipboard): keep arboard's ambiguous absence indeterminate CodeRabbit noted on #487 that arboard reports an empty clipboard and contents in an unreadable format with one error, so mapping ContentNotAvailable to NoSelection asserts a fact arboard does not know. Add TextAbsence::Indeterminate, and the matching TextRead variant, for that case. The evaluator-facing wire is unchanged: into_option answers nil for every absence, and only the contract wording grows a third name. --- .../src/clipboard/tests/contract_test.rs | 6 ++--- .../src/clipboard/tests/text_policy_test.rs | 7 ++++-- .../src/clipboard/text_policy.rs | 25 +++++++++++++------ 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/crates/neomacs-display-runtime/src/clipboard/tests/contract_test.rs b/crates/neomacs-display-runtime/src/clipboard/tests/contract_test.rs index 29ce7f726a..c63d1dd9f9 100644 --- a/crates/neomacs-display-runtime/src/clipboard/tests/contract_test.rs +++ b/crates/neomacs-display-runtime/src/clipboard/tests/contract_test.rs @@ -4,9 +4,9 @@ //! The contract is deliberately about the *shape* of an answer, not about any //! one backend's mechanism: //! -//! 1. an absent selection is an answer (`TextRead::NoSelection` / -//! `TextRead::TargetUnavailable`), never an `Err` — `Err` is reserved for -//! transport and worker failures; +//! 1. an absent selection is an answer (`TextRead::NoSelection`, +//! `TextRead::TargetUnavailable`, or `TextRead::Indeterminate`), never an +//! `Err` — `Err` is reserved for transport and worker failures; //! 2. text round-trips where the backend can read the selection it set; //! 3. `Text("")` is text, not absence; //! 4. CLIPBOARD and PRIMARY never alias. diff --git a/crates/neomacs-display-runtime/src/clipboard/tests/text_policy_test.rs b/crates/neomacs-display-runtime/src/clipboard/tests/text_policy_test.rs index 6dbfaf07a1..79ed06c13a 100644 --- a/crates/neomacs-display-runtime/src/clipboard/tests/text_policy_test.rs +++ b/crates/neomacs-display-runtime/src/clipboard/tests/text_policy_test.rs @@ -99,10 +99,12 @@ fn smithay_failures_are_not_absences() { } #[test] -fn arboard_content_not_available_is_the_only_absence() { +fn arboard_content_not_available_is_an_indeterminate_absence() { + // arboard reports empty and unreadable-format with one error, so the + // policy must not claim to know which one it was. assert_eq!( classify_arboard(&arboard::Error::ContentNotAvailable), - Some(TextAbsence::NoSelection) + Some(TextAbsence::Indeterminate) ); for err in [ arboard::Error::ClipboardNotSupported, @@ -221,4 +223,5 @@ fn only_text_reaches_the_evaluator_facing_wire() { ); assert_eq!(TextRead::NoSelection.into_option(), None); assert_eq!(TextRead::TargetUnavailable.into_option(), None); + assert_eq!(TextRead::Indeterminate.into_option(), None); } diff --git a/crates/neomacs-display-runtime/src/clipboard/text_policy.rs b/crates/neomacs-display-runtime/src/clipboard/text_policy.rs index 361019b25c..01747e4848 100644 --- a/crates/neomacs-display-runtime/src/clipboard/text_policy.rs +++ b/crates/neomacs-display-runtime/src/clipboard/text_policy.rs @@ -44,6 +44,11 @@ pub(super) enum TextAbsence { NoSelection, /// An owner exists, but it offers none of the text MIMEs this backend reads. TargetUnavailable, + /// The backend cannot tell those two apart: it reports one error for both + /// an empty clipboard and contents in a format it cannot read (arboard's + /// `ContentNotAvailable`). Guessing either way would state a fact the + /// backend does not know. + Indeterminate, } impl From for TextRead { @@ -51,6 +56,7 @@ impl From for TextRead { match absence { TextAbsence::NoSelection => Self::NoSelection, TextAbsence::TargetUnavailable => Self::TargetUnavailable, + TextAbsence::Indeterminate => Self::Indeterminate, } } } @@ -64,17 +70,20 @@ pub(super) enum TextRead { NoSelection, /// An owner exists, but it offers none of the text MIMEs we read. TargetUnavailable, + /// The backend cannot tell "no owner" from "unreadable contents"; see + /// `TextAbsence::Indeterminate`. + Indeterminate, } impl TextRead { - /// The answer the evaluator-facing wire carries: text, or `None` for - /// either absence. The two absences stay distinct inside the runtime so + /// The answer the evaluator-facing wire carries: text, or `None` for any + /// absence. The absence reasons stay distinct inside the runtime so /// callers can tell "nothing is selected" from "something is selected that - /// we cannot read". + /// we cannot read", and can see when the backend does not know which. pub(super) fn into_option(self) -> Option { match self { Self::Text(text) => Some(text), - Self::NoSelection | Self::TargetUnavailable => None, + Self::NoSelection | Self::TargetUnavailable | Self::Indeterminate => None, } } } @@ -150,12 +159,12 @@ pub(super) fn classify_smithay(err: &std::io::Error) -> Option { /// /// arboard's `ContentNotAvailable` covers both "the clipboard is empty" and /// "the contents have an incompatible format" (`common.rs:19-24`), so it can -/// never name a specific absence; the conservative answer is `NoSelection`. -/// Every other variant is a real failure. (`Error` is `#[non_exhaustive]`, so -/// the catch-all is required.) +/// never name a specific absence and maps to `Indeterminate`. Every other +/// variant is a real failure. (`Error` is `#[non_exhaustive]`, so the +/// catch-all is required.) pub(super) fn classify_arboard(err: &arboard::Error) -> Option { match err { - arboard::Error::ContentNotAvailable => Some(TextAbsence::NoSelection), + arboard::Error::ContentNotAvailable => Some(TextAbsence::Indeterminate), _ => None, } } From 824f44e4f2127089b1537b5f55218f54c0df92df Mon Sep 17 00:00:00 2001 From: Eval Exec Date: Mon, 5 Oct 2026 23:40:12 +0800 Subject: [PATCH 5/5] refactor(clipboard): gate the Linux-only absence variant Copilot noted that TextAbsence::TargetUnavailable is never constructed off Linux -- only the Wayland classifier produces it -- so the macOS/Windows cargo check jobs would report the variant as dead code. The same holds for the matching TextRead variant, and the ambiguous absence change had left TextAbsence::NoSelection unconstructed there too. Gate both TargetUnavailable variants on Linux, with their match arms, and route the process-local PRIMARY vacancy through TextAbsence::NoSelection so every remaining variant is constructed on every platform. The display-crate layout guide forbids allow(dead_code), so the variants that cannot occur are removed from those builds instead of silenced. --- crates/neomacs-display-runtime/src/clipboard.rs | 13 ++++++------- .../src/clipboard/tests/clipboard_test.rs | 14 ++++++++++---- .../src/clipboard/text_policy.rs | 13 +++++++++++-- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/crates/neomacs-display-runtime/src/clipboard.rs b/crates/neomacs-display-runtime/src/clipboard.rs index 1ec9bc7ae2..71b4c768e8 100644 --- a/crates/neomacs-display-runtime/src/clipboard.rs +++ b/crates/neomacs-display-runtime/src/clipboard.rs @@ -262,10 +262,12 @@ impl PrivateSelection { }; } - fn load(&self) -> Option { + /// The stored text, or the absence a vacant selection reports; going + /// through the policy keeps every "no owner" answer one vocabulary. + fn load(&self) -> text_policy::TextRead { match self { - Self::Owned(text) => Some(text.clone()), - Self::Vacant => None, + Self::Owned(text) => text_policy::TextRead::Text(text.clone()), + Self::Vacant => text_policy::TextAbsence::NoSelection.into(), } } @@ -360,10 +362,7 @@ impl ClipboardBackend for ArboardClipboard { _ => { match selection { ClipboardSelection::Clipboard => Self::text_result(self.clipboard.get_text()), - ClipboardSelection::Primary => Ok(match self.primary.load() { - Some(text) => text_policy::TextRead::Text(text), - None => text_policy::TextRead::NoSelection, - }), + ClipboardSelection::Primary => Ok(self.primary.load()), } } } diff --git a/crates/neomacs-display-runtime/src/clipboard/tests/clipboard_test.rs b/crates/neomacs-display-runtime/src/clipboard/tests/clipboard_test.rs index 09f52ecd39..d2f6765294 100644 --- a/crates/neomacs-display-runtime/src/clipboard/tests/clipboard_test.rs +++ b/crates/neomacs-display-runtime/src/clipboard/tests/clipboard_test.rs @@ -296,16 +296,22 @@ fn mutation_that_expires_behind_a_slow_read_is_never_executed() { #[test] fn private_selection_round_trips_owned_and_vacant_states() { let mut selection = PrivateSelection::default(); - assert_eq!(selection.load(), None); + assert_eq!(selection.load(), text_policy::TextRead::NoSelection); selection.store(Some("selected")); - assert_eq!(selection.load(), Some("selected".to_owned())); + assert_eq!( + selection.load(), + text_policy::TextRead::Text("selected".to_owned()) + ); selection.store(Some("reselected")); - assert_eq!(selection.load(), Some("reselected".to_owned())); + assert_eq!( + selection.load(), + text_policy::TextRead::Text("reselected".to_owned()) + ); selection.store(None); - assert_eq!(selection.load(), None); + assert_eq!(selection.load(), text_policy::TextRead::NoSelection); } /// GNU's NS port keeps PRIMARY in a private pasteboard instead of diff --git a/crates/neomacs-display-runtime/src/clipboard/text_policy.rs b/crates/neomacs-display-runtime/src/clipboard/text_policy.rs index 01747e4848..e77610ed0e 100644 --- a/crates/neomacs-display-runtime/src/clipboard/text_policy.rs +++ b/crates/neomacs-display-runtime/src/clipboard/text_policy.rs @@ -43,6 +43,10 @@ pub(super) enum TextAbsence { /// No selection owner. NoSelection, /// An owner exists, but it offers none of the text MIMEs this backend reads. + /// + /// Only a Wayland backend can tell this apart from `NoSelection`, so the + /// variant exists only on a platform that has one. + #[cfg(target_os = "linux")] TargetUnavailable, /// The backend cannot tell those two apart: it reports one error for both /// an empty clipboard and contents in a format it cannot read (arboard's @@ -55,6 +59,7 @@ impl From for TextRead { fn from(absence: TextAbsence) -> Self { match absence { TextAbsence::NoSelection => Self::NoSelection, + #[cfg(target_os = "linux")] TextAbsence::TargetUnavailable => Self::TargetUnavailable, TextAbsence::Indeterminate => Self::Indeterminate, } @@ -68,7 +73,9 @@ pub(super) enum TextRead { Text(String), /// No selection owner. NoSelection, - /// An owner exists, but it offers none of the text MIMEs we read. + /// An owner exists, but it offers none of the text MIMEs we read; see + /// `TextAbsence::TargetUnavailable` for why this is Linux-only. + #[cfg(target_os = "linux")] TargetUnavailable, /// The backend cannot tell "no owner" from "unreadable contents"; see /// `TextAbsence::Indeterminate`. @@ -83,7 +90,9 @@ impl TextRead { pub(super) fn into_option(self) -> Option { match self { Self::Text(text) => Some(text), - Self::NoSelection | Self::TargetUnavailable | Self::Indeterminate => None, + Self::NoSelection | Self::Indeterminate => None, + #[cfg(target_os = "linux")] + Self::TargetUnavailable => None, } } }