Don't crash if keysym is not processed

src/main.rs

@@ -27,7 +27,7 @@ use breadx::{
 
 use lazy_static::lazy_static;
 
-use tokio::sync::mpsc::unbounded_channel;
+use tokio::sync::mpsc;
 
 mod config;
 mod msg_listener;
@@ -41,8 +41,8 @@ pub enum XcrabError {
     Io(std::io::Error),
     Toml(toml::de::Error),
     Var(std::env::VarError),
-    FromStr(String),
     ClientDoesntExist,
+    Custom(String),
 }
 
 impl From<BreadError> for XcrabError {
@@ -71,7 +71,7 @@ impl From<std::env::VarError> for XcrabError {
 
 impl From<String> for XcrabError {
     fn from(v: String) -> Self {
-        Self::FromStr(v)
+        Self::Custom(v)
     }
 }
 
@@ -91,7 +91,7 @@ impl Display for XcrabError {
             Self::Io(ie) => Display::fmt(ie, f)?,
             Self::Toml(te) => Display::fmt(te, f)?,
             Self::Var(ve) => Display::fmt(ve, f)?,
-            Self::FromStr(fe) => Display::fmt(fe, f)?,
+            Self::Custom(fe) => Display::fmt(fe, f)?,
             Self::ClientDoesntExist => Display::fmt("client didn't exist", f)?,
         }
 
@@ -137,11 +137,13 @@ async fn main() -> Result<()> {
 
     conn.ungrab_server_async().await?;
 
-    let (send, mut recv) = unbounded_channel();
+    let (send, mut recv) = mpsc::unbounded_channel();
+    let (result_send, result_recv) = mpsc::unbounded_channel();
 
     tokio::spawn(msg_listener::listener_task(
         CONFIG.msg.clone().unwrap_or_default().socket_path,
         send,
+        result_recv,
     ));
 
     let mut keyboard_state = KeyboardState::new_async(&mut conn).await?;
@@ -151,7 +153,7 @@ async fn main() -> Result<()> {
         // starved by x11 events. Probably unnecessary, but better safe than sorry.
         tokio::select! {
             biased;
-            Some(s) = recv.recv() => msg_listener::on_recv(s, &mut manager, &mut conn).await?,
+            Some(s) = recv.recv() => msg_listener::on_recv(s, &mut manager, &mut conn, &result_send).await?,
             Ok(ev) = conn.wait_for_event_async() => process_event(ev,
                 &mut manager,
                 &mut conn,
@@ -162,6 +164,7 @@ async fn main() -> Result<()> {
     }
 }
 
+#[allow(clippy::too_many_lines)] // FIXME: missing help i have no idea how to make this shorter
 async fn process_event<Dpy: AsyncDisplay + ?Sized>(
     ev: Event,
     manager: &mut XcrabWindowManager,
@@ -169,8 +172,19 @@ async fn process_event<Dpy: AsyncDisplay + ?Sized>(
     root: Window,
     keyboard_state: &mut KeyboardState,
 ) -> Result<()> {
-    let focused = manager.get_focused().await?;
-    let focused_frame: x11::client::FramedWindow = manager.get_framed_window(focused).await?;
+    let focused = {
+        // A new scope is required in order to drop the future stored here, and therefore free the
+        // immutable borrow of `manager`
+        let orig = manager
+            .get_focused()
+            .await
+            .map(|w| (w, manager.get_framed_window(w)));
+        if let Some((w, fut)) = orig {
+            Some((w, fut.await))
+        } else {
+            None
+        }
+    };
 
     match ev {
         Event::MapRequest(ev) => {
@@ -211,56 +225,70 @@ async fn process_event<Dpy: AsyncDisplay + ?Sized>(
             if ev.detail == 1 && manager.has_client(ev.event) {
                 manager.set_focus(conn, ev.event).await?;
             }
-            if ev.event == focused_frame.input {
-                ev.event = focused;
-                conn.send_event_async(focused, EventMask::BUTTON_PRESS, Event::ButtonPress(ev))
-                    .await?;
+            if let Some((focused, focused_frame)) = focused {
+                if ev.event == focused_frame.input {
+                    ev.event = focused;
+                    conn.send_event_async(focused, EventMask::BUTTON_PRESS, Event::ButtonPress(ev))
+                        .await?;
+                }
             }
         }
         Event::KeyPress(mut ev) => {
-            if let Some(c) = keyboard_state
-                .process_keycode(ev.detail, ev.state)
-                .unwrap()
-                .as_char()
-            {
-                for (&bind, action) in &CONFIG.binds {
-                    if bind.key == c && bind.mods == ev.state {
-                        action.eval(manager, conn).await?;
+            if let Some(k) = keyboard_state.process_keycode(ev.detail, ev.state) {
+                if let Some(c) = k.as_char() {
+                    for (&bind, action) in &CONFIG.binds {
+                        if bind.key == c && bind.mods == ev.state {
+                            action.eval(manager, conn).await?;
+                        }
                     }
                 }
             }
 
             // keybind did not match, forward instead
-            ev.event = focused;
-            conn.send_event_async(focused, EventMask::KEY_PRESS, Event::KeyPress(ev))
-                .await?;
+            if let Some((focused, _)) = focused {
+                ev.event = focused;
+                conn.send_event_async(focused, EventMask::KEY_PRESS, Event::KeyPress(ev))
+                    .await?;
+            }
         }
         Event::KeyRelease(mut ev) => {
-            if ev.event == focused_frame.input {
-                ev.event = focused;
-                conn.send_event_async(focused, EventMask::KEY_RELEASE, Event::KeyRelease(ev))
-                    .await?;
+            if let Some((focused, focused_frame)) = focused {
+                if ev.event == focused_frame.input {
+                    ev.event = focused;
+                    conn.send_event_async(focused, EventMask::KEY_RELEASE, Event::KeyRelease(ev))
+                        .await?;
+                }
             }
         }
         Event::ButtonRelease(mut ev) => {
-            if ev.event == focused_frame.input {
-                ev.event = focused;
-                conn.send_event_async(focused, EventMask::BUTTON_RELEASE, Event::ButtonRelease(ev))
+            if let Some((focused, focused_frame)) = focused {
+                if ev.event == focused_frame.input {
+                    ev.event = focused;
+                    conn.send_event_async(
+                        focused,
+                        EventMask::BUTTON_RELEASE,
+                        Event::ButtonRelease(ev),
+                    )
                     .await?;
+                }
             }
         }
         Event::EnterNotify(mut ev) => {
-            if ev.event == focused_frame.input {
-                ev.event = focused;
-                conn.send_event_async(focused, EventMask::ENTER_WINDOW, Event::EnterNotify(ev))
-                    .await?;
+            if let Some((focused, focused_frame)) = focused {
+                if ev.event == focused_frame.input {
+                    ev.event = focused;
+                    conn.send_event_async(focused, EventMask::ENTER_WINDOW, Event::EnterNotify(ev))
+                        .await?;
+                }
             }
         }
         Event::LeaveNotify(mut ev) => {
-            if ev.event == focused_frame.input {
-                ev.event = focused;
-                conn.send_event_async(focused, EventMask::LEAVE_WINDOW, Event::LeaveNotify(ev))
-                    .await?;
+            if let Some((focused, focused_frame)) = focused {
+                if ev.event == focused_frame.input {
+                    ev.event = focused;
+                    conn.send_event_async(focused, EventMask::LEAVE_WINDOW, Event::LeaveNotify(ev))
+                        .await?;
+                }
             }
         }
 

src/msg/config.rs

@@ -17,12 +17,14 @@ use crate::Result;
 use serde::Deserialize;
 use std::path::PathBuf;
 
+#[allow(clippy::module_name_repetitions)]
 #[derive(Clone, Debug, Default, Deserialize)]
 // Dummy struct for deserializing the message config - we're using the same file for both binaries
 pub struct XcrabConfig {
     pub msg: XcrabMsgConfig,
 }
 
+#[allow(clippy::module_name_repetitions)]
 #[derive(Clone, Debug, Deserialize)]
 pub struct XcrabMsgConfig {
     pub socket_path: PathBuf,

src/msg/main.rs

@@ -13,12 +13,32 @@
 // You should have received a copy of the GNU General Public License
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+#![warn(clippy::pedantic)]
+
 mod config;
 
-use tokio::io::AsyncWriteExt;
+use std::error::Error;
+use std::fmt::{Debug, Display, Formatter, Result as FmtResult};
+use tokio::io::{AsyncReadExt, AsyncWriteExt};
 use tokio::net::UnixStream;
 
-type Result<T> = std::result::Result<T, Box<dyn std::error::Error>>;
+type Result<T> = std::result::Result<T, Box<dyn Error>>;
+
+struct CustomError(String);
+
+impl Debug for CustomError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
+        f.write_str(&self.0)
+    }
+}
+
+impl Display for CustomError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
+        f.write_str(&self.0)
+    }
+}
+
+impl Error for CustomError {}
 
 #[tokio::main]
 async fn main() -> Result<()> {
@@ -28,9 +48,20 @@ async fn main() -> Result<()> {
 
     let path = conf.msg.socket_path;
 
-    let mut stream = UnixStream::connect(path).await?;
+    let stream = UnixStream::connect(path).await?;
+    let (mut read, mut write) = stream.into_split();
 
-    stream.write_all(msg.as_bytes()).await?;
+    write.write_all(msg.as_bytes()).await?;
+    drop(write); // Shutdown the writer half so that the write actually goes through
+                 // "Don't cross the streams!""
+
+    let mut buf = String::new();
+
+    read.read_to_string(&mut buf).await?;
+
+    if !buf.is_empty() {
+        return Err(CustomError(buf).into());
+    }
 
     Ok(())
 }

src/msg_listener.rs

@@ -17,9 +17,10 @@ use crate::x11::client::XcrabWindowManager;
 use crate::Result;
 use breadx::AsyncDisplay;
 use std::path::Path;
-use tokio::io::AsyncReadExt;
+use std::str::FromStr;
+use tokio::io::{AsyncReadExt, AsyncWriteExt};
 use tokio::net::UnixListener;
-use tokio::sync::mpsc::UnboundedSender;
+use tokio::sync::mpsc::{UnboundedReceiver, UnboundedSender};
 
 macro_rules! unwrap_or_continue {
     ($e:expr) => {
@@ -34,6 +35,7 @@ macro_rules! unwrap_or_continue {
 pub async fn listener_task<P: AsRef<Path>>(
     socket_path: P,
     sender: UnboundedSender<String>,
+    mut result_recv: UnboundedReceiver<Result<()>>,
 ) -> Result<()> {
     let socket_path = socket_path.as_ref();
     if socket_path.exists() {
@@ -47,6 +49,13 @@ pub async fn listener_task<P: AsRef<Path>>(
         stream.read_to_string(&mut buf).await?;
 
         drop(sender.send(buf)); // go back to ms word clippy
+
+        // we can unwrap here because if the channel is closed then something's not right
+        if let Err(e) = result_recv.recv().await.unwrap() {
+            stream.write_all(format!("{}", e).as_bytes()).await?;
+        } else {
+            stream.write_all(&[]).await?;
+        }
     }
 }
 
@@ -54,9 +63,16 @@ pub async fn on_recv<Dpy: AsyncDisplay + ?Sized>(
     data: String,
     manager: &mut XcrabWindowManager,
     conn: &mut Dpy,
+    result_sender: &UnboundedSender<Result<()>>,
 ) -> Result<()> {
-    let a: Action = data.parse()?;
-    a.eval(manager, conn).await?;
+    let res = { data.parse::<Action>() };
+
+    if let Ok(ref a) = res {
+        a.eval(manager, conn).await?; // Don't send these errors over the channel, because they're
+                                      // xcrab errors, not msg errors
+    }
+
+    drop(result_sender.send(res.map(|_| ())));
 
     Ok(())
 }
@@ -67,20 +83,40 @@ pub enum Action {
     Close,
 }
 
-impl std::str::FromStr for Action {
-    type Err = String;
+impl FromStr for Action {
+    type Err = crate::XcrabError;
 
     fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
         #[allow(clippy::enum_glob_use)]
         use Action::*;
-        let v: Vec<String> = s.split(' ').map(str::to_ascii_lowercase).collect();
+        let parts: Vec<String> = s
+            .split(' ')
+            .map(str::to_ascii_lowercase)
+            .filter(|s| !s.is_empty())
+            .collect();
 
-        let a = match v[0].as_str() {
-            "close" => Close,
-            _ => return Err(format!("Unknown action: {}", v[0])),
-        };
+        if parts.is_empty() {
+            return Err(String::from("No action provided").into());
+        }
 
-        Ok(a)
+        macro_rules! eq_ignore_ascii_case_match {
+            (($scrutinee:expr) { $($s:literal => $v:expr,)+ else => $else:expr $(,)? }) => {
+                $(
+                    if $scrutinee.eq_ignore_ascii_case($s) {
+                        $v
+                    } else
+                )+ {
+                    $else
+                }
+            };
+        }
+
+        // TODO: When more actions are added (such as focus etc), they will take arguments. In that
+        // case, they will get passed the rest of `parts`.
+        eq_ignore_ascii_case_match!((parts[0]) {
+            "close" => Ok(Close),
+            else => Err(format!("Unknown action: {}", s).into()),
+        })
     }
 }
 
@@ -92,9 +128,11 @@ impl Action {
     ) -> Result<()> {
         #[allow(clippy::enum_glob_use)]
         use Action::*;
+
         match self {
             Close => manager.destroy_focused_client(conn).await?,
         }
+
         Ok(())
     }
 }

src/x11/client.rs

@@ -137,14 +137,12 @@ enum RectangleContents {
     Client(Client),
 }
 
-#[allow(clippy::module_name_repetitions)]
 #[derive(Debug, Clone)]
 struct Pane {
     children: Vec<XcrabKey>,
     directionality: Directionality,
 }
 
-#[allow(clippy::module_name_repetitions)]
 #[derive(Debug, Clone, Copy)]
 struct Client {
     frame: FramedWindow,
@@ -614,15 +612,15 @@ impl XcrabWindowManager {
         Ok(())
     }
 
-    pub async fn get_focused(&self) -> Result<Window> {
-        self.focused.ok_or(XcrabError::ClientDoesntExist)
+    pub async fn get_focused(&self) -> Option<Window> {
+        self.focused
     }
 
-    pub async fn get_framed_window(&self, window: Window) -> Result<FramedWindow> {
+    pub async fn get_framed_window(&self, window: Window) -> FramedWindow {
         let focused_key = self.clients.get(&window).unwrap();
         let focused = self.rects.get(*focused_key).unwrap();
         let focused_frame = focused.unwrap_client().frame;
-        Ok(focused_frame)
+        focused_frame
     }
 }