refactor: fix handle_selection_request 8/7 args

Resolve Clippy's `too_many_arguments` lint on
`handle_selection_request` by removing the `success` and `refuse`
parameters and returning a bool indicating which needs to be run.

Use the helper functions defined last commit to simplify
`incr_copy_from_x11`

Reduce the `match` statement checking if the target is the TARGET
atom to an if statement. Although using `match` statements as
`if`/`else if`/`else` chains occur elsewhere, this one is both only 2
cases, and already of particular relevance to the current work.

Refactor `paste_impl` to be much easier to read. I did this code algebra
way back while investigating #142, but since I never figured that out,
the change has just been sitting in my stash.

refactor: simplify paste_data logic, some unnests

src/server/selection.rs

@@ -240,9 +240,10 @@ impl SelectionType for Clipboard {
 
     fn receive_offer(offer: &Self::Offer, mime_type: String) -> std::io::Result<ReadPipe> {
         offer.receive(mime_type).map_err(|e| {
+            use smithay_client_toolkit::data_device_manager::data_offer::DataOfferError;
             match e {
-                smithay_client_toolkit::data_device_manager::data_offer::DataOfferError::InvalidReceive => std::io::Error::from(std::io::ErrorKind::Other),
+                DataOfferError::InvalidReceive => std::io::Error::from(std::io::ErrorKind::Other),
-                smithay_client_toolkit::data_device_manager::data_offer::DataOfferError::Io(e) => e
+                DataOfferError::Io(e) => e,
             }
         })
     }

src/xstate/selection.rs

@@ -250,10 +250,8 @@ trait SelectionDataImpl {
         atoms: &super::Atoms,
         request: &x::SelectionRequestEvent,
         max_req_bytes: usize,
-        success: &dyn Fn(),
-        refuse: &dyn Fn(),
         server_state: &mut RealServerState,
-    );
+    ) -> bool;
     fn atom(&self) -> x::Atom;
 }
 
@@ -418,10 +416,8 @@ impl<T: SelectionType> SelectionDataImpl for SelectionData<T> {
         atoms: &super::Atoms,
         request: &x::SelectionRequestEvent,
         max_req_bytes: usize,
-        success: &dyn Fn(),
-        refuse: &dyn Fn(),
         server_state: &mut RealServerState,
-    ) {
+    ) -> bool {
         let Some(CurrentSelection::Wayland(WaylandSelection {
             mimes,
             inner,
@@ -429,12 +425,11 @@ impl<T: SelectionType> SelectionDataImpl for SelectionData<T> {
         })) = &mut self.current_selection
         else {
             warn!("Got selection request, but we don't seem to be the selection owner");
-            refuse();
+            return false;
-            return;
         };
 
-        match request.target() {
+        let req_target = request.target();
-            x if x == atoms.targets => {
+        if req_target == atoms.targets {
             let atoms: Box<[x::Atom]> = mimes.iter().map(|t| t.atom).collect();
 
             connection
@@ -447,16 +442,16 @@ impl<T: SelectionType> SelectionDataImpl for SelectionData<T> {
                 })
                 .unwrap();
 
-                success();
+            true
-            }
+        } else {
-            other => {
+            let Some(target) = mimes.iter().find(|t| t.atom == req_target) else {
-                let Some(target) = mimes.iter().find(|t| t.atom == other) else {
                 if log::log_enabled!(log::Level::Debug) {
-                        let name = get_atom_name(connection, other);
+                    let name = get_atom_name(connection, req_target);
-                        debug!("refusing selection request because given atom could not be found ({name})");
+                    debug!(
+                        "refusing selection request because given atom could not be found ({name})"
+                    );
                 }
-                    refuse();
+                return false;
-                    return;
             };
 
             let mime_name = target
@@ -471,8 +466,7 @@ impl<T: SelectionType> SelectionDataImpl for SelectionData<T> {
                     value_list: &[x::Cw::EventMask(x::EventMask::PROPERTY_CHANGE)],
                 }) {
                     warn!("Failed to set up property change notifications: {e:?}");
-                        refuse();
+                    return false;
-                        return;
                 }
                 if let Err(e) = connection.send_and_check_request(&x::ChangeProperty {
                     mode: x::PropMode::Replace,
@@ -482,8 +476,7 @@ impl<T: SelectionType> SelectionDataImpl for SelectionData<T> {
                     data: &[data.len() as u32],
                 }) {
                     warn!("Failed to set incr property for large transfer: {e:?}");
-                        refuse();
+                    return false;
-                        return;
                 }
                 debug!(
                     "beginning incr for {}",
@@ -497,7 +490,7 @@ impl<T: SelectionType> SelectionDataImpl for SelectionData<T> {
                     target_type: target.atom,
                     max_req_bytes,
                 });
-                    success()
+                true
             } else {
                 match connection.send_and_check_request(&x::ChangeProperty {
                     mode: x::PropMode::Replace,
@@ -506,11 +499,10 @@ impl<T: SelectionType> SelectionDataImpl for SelectionData<T> {
                     r#type: target.atom,
                     data: &data,
                 }) {
-                        Ok(_) => success(),
+                    Ok(_) => true,
                     Err(e) => {
                         warn!("Failed setting selection property: {e:?}");
-                            refuse();
+                        false
-                        }
                     }
                 }
             }
@@ -665,7 +657,7 @@ impl XState {
         self.selection_state
             .primary
             .set_owner(&self.connection, self.wm_window);
-        debug!("Primaryset from Wayland");
+        debug!("Primary set from Wayland");
     }
 
     pub(super) fn handle_selection_event(
@@ -771,15 +763,17 @@ impl XState {
                     return true;
                 }
 
-                data.handle_selection_request(
+                if data.handle_selection_request(
                     &self.connection,
                     &self.atoms,
                     e,
                     self.max_req_bytes,
-                    &success,
-                    &refuse,
                     server_state,
-                );
+                ) {
+                    success()
+                } else {
+                    refuse()
+                }
             }
 
             xcb::Event::XFixes(xcb::xfixes::Event::SelectionNotify(e)) => match e.selection() {

tests/integration.rs

@@ -1520,12 +1520,7 @@ fn incr_copy_from_x11() {
         let request = connection.await_selection_request();
         assert_eq!(request.target(), connection.atoms.mime1);
 
-        connection
+        connection.get_property_change_events(request.requestor());
-            .send_and_check_request(&x::ChangeWindowAttributes {
-                window: request.requestor(),
-                value_list: &[x::Cw::EventMask(x::EventMask::PROPERTY_CHANGE)],
-            })
-            .unwrap();
         connection.set_property(
             request.requestor(),
             connection.atoms.incr,
@@ -1534,10 +1529,7 @@ fn incr_copy_from_x11() {
         );
         connection.send_selection_notify(&request);
         // skip NewValue
-        let notify = match connection.poll_for_event().unwrap() {
+        let notify = connection.await_property_notify();
-            Some(xcb::Event::X(x::Event::PropertyNotify(p))) => p,
-            other => panic!("Didn't get property notify event, instead got {other:?}"),
-        };
         assert_eq!(notify.atom(), request.property());
         assert_eq!(notify.state(), x::Property::NewValue);
         request.property()
@@ -1555,11 +1547,7 @@ fn incr_copy_from_x11() {
         }
         assert_ne!(destination_property, x::Atom::none());
 
-        let notify = match connection.await_event() {
+        let notify = connection.await_property_notify();
-            xcb::Event::X(x::Event::PropertyNotify(p)) => p,
-            other => panic!("Didn't get property notify event, instead got {other:?}"),
-        };
-
         match it.next() {
             Some((idx, chunk)) => {
                 assert_eq!(notify.atom(), destination_property, "chunk {idx}");
@@ -1572,10 +1560,7 @@ fn incr_copy_from_x11() {
                 );
                 testwl.dispatch();
                 // skip NewValue
-                let notify = match connection.poll_for_event().unwrap() {
+                let notify = connection.await_property_notify();
-                    Some(xcb::Event::X(x::Event::PropertyNotify(p))) => p,
-                    other => panic!("Didn't get property notify event, instead got {other:?}"),
-                };
                 assert_eq!(notify.atom(), destination_property, "chunk {idx}");
                 assert_eq!(notify.state(), x::Property::NewValue, "chunk {idx}");
                 false

testwl/src/lib.rs

@@ -669,8 +669,19 @@ impl Server {
         };
 
         let mut ret = Vec::new();
-        let mut try_transfer =
+        while let Some((mime, pending)) = pending_ret.pop() {
-            |pending_ret: &mut PendingRet, mime: String, mut pending: PendingData| {
+            let mut pending = pending.unwrap_or_else(|| {
+                let (rx, tx) = rustix::pipe::pipe().unwrap();
+                send_selection(mime.clone(), tx.as_fd());
+                drop(tx);
+
+                let rx = std::fs::File::from(rx);
+                PendingData {
+                    rx,
+                    data: Vec::new(),
+                }
+            });
+
             self.display.flush_clients().unwrap();
             let transfer_complete = send_data_for_mime(&mime, self);
             if transfer_complete {
@@ -691,27 +702,6 @@ impl Server {
                 pending_ret.push((mime, Some(pending)));
                 self.dispatch();
             }
-            };
-
-        while let Some((mime, pending)) = pending_ret.pop() {
-            match pending {
-                Some(pending) => try_transfer(&mut pending_ret, mime, pending),
-                None => {
-                    let (rx, tx) = rustix::pipe::pipe().unwrap();
-                    send_selection(mime.clone(), tx.as_fd());
-                    drop(tx);
-
-                    let rx = std::fs::File::from(rx);
-                    try_transfer(
-                        &mut pending_ret,
-                        mime,
-                        PendingData {
-                            rx,
-                            data: Vec::new(),
-                        },
-                    );
-                }
-            }
         }
 
         ret