some cleanup and lints

7 files changed, 124 insertions, 92 deletions

diff --git a/src/atoms.rs b/src/atoms.rs
index 4dfcf47..5db75da 100644
--- a/src/atoms.rs
+++ b/src/atoms.rs
@@ -5,7 +5,7 @@ use xcb::Connection;
 pub struct InternedAtoms {}
 
 impl InternedAtoms {
-    pub fn new_with(conn: &Connection) -> Result<Self> {
+    pub fn new_with(_conn: &Connection) -> Result<Self> {
         // TODO: intern atoms
 
         // utf8string = XInternAtom(dpy, "UTF8_STRING", False);
diff --git a/src/clients.rs b/src/clients.rs
index 6a3d71c..341f73c 100644
--- a/src/clients.rs
+++ b/src/clients.rs
@@ -9,57 +9,8 @@ use xcb::{
 
 use crate::{error::*, WM};
 
-#[derive(Debug, Default)]
-pub struct ClientList(Vec<MonitorInfo>);
-
-impl ClientList {
-    /// Set the new amount of screens, moving clients away if necessary
-    pub fn truncate_screens(&mut self, new_size: usize) {
-        // hack: double borrow stuff
-        let mut moved_clients = vec![];
-        for old in self.0.drain(new_size - self.0.len()..self.0.len()) {
-            moved_clients.extend(old.clients.into_iter());
-        }
-        self.0[0].clients.extend(moved_clients.into_iter());
-    }
-
-    pub fn set_screen_info(&mut self, i: usize, info: ScreenInfo) {
-        while i >= self.0.len() {
-            self.0.push(MonitorInfo::default())
-        }
-        self.0[i].screen_info = info;
-    }
-
-    pub fn len(&self) -> usize {
-        self.0.len()
-    }
-}
-
-#[derive(Debug)]
-pub struct MonitorInfo {
-    clients: Vec<Client>,
-    screen_info: ScreenInfo,
-}
-
-impl Default for MonitorInfo {
-    fn default() -> Self {
-        Self {
-            clients: vec![],
-            screen_info: ScreenInfo {
-                x_org: 0,
-                y_org: 0,
-                width: 0,
-                height: 0,
-            },
-        }
-    }
-}
-
-#[derive(Debug)]
-pub struct Client {}
-
 impl WM<'_> {
-    /// Update the client list's recorded monitors and monitor sizes
+    /// Update the client state's recorded monitors and monitor sizes
     pub(crate) fn update_geometry(&mut self) -> Result<()> {
         if self
             .conn
@@ -71,17 +22,17 @@ impl WM<'_> {
                 .wait_for_reply(self.conn.send_request(&xinerama::QueryScreens {}))?;
 
             // Monitor removed, move its clients away
-            if reply.screen_info().len() > self.clients.len() {
+            if reply.screen_info().len() > self.clients.monitor_count() {
                 self.clients.truncate_screens(reply.screen_info().len());
             }
 
             // Update screen info & add new client lists if needed
             for (i, monitor) in reply.screen_info().iter().enumerate() {
-                self.clients.set_screen_info(i, monitor.clone());
+                self.clients.set_screen_info(i, *monitor);
             }
         } else {
             // Only one screen
-            if self.clients.len() > 1 {
+            if self.clients.monitor_count() > 1 {
                 self.clients.truncate_screens(1);
             }
 
@@ -106,19 +57,79 @@ impl WM<'_> {
         Ok(())
     }
 
-    pub(crate) fn handle_configure_request(&mut self, e: ConfigureRequestEvent) -> Result<()> {
+    pub(crate) fn handle_configure_request(&mut self, _e: ConfigureRequestEvent) -> Result<()> {
         todo!()
     }
-    pub(crate) fn handle_configure_notify(&mut self, e: ConfigureNotifyEvent) -> Result<()> {
+    pub(crate) fn handle_configure_notify(&mut self, _e: ConfigureNotifyEvent) -> Result<()> {
         todo!()
     }
-    pub(crate) fn handle_destroy_notify(&mut self, e: DestroyNotifyEvent) -> Result<()> {
+    pub(crate) fn handle_destroy_notify(&mut self, _e: DestroyNotifyEvent) -> Result<()> {
         todo!()
     }
-    pub(crate) fn handle_map_request(&mut self, e: MapRequestEvent) -> Result<()> {
+    pub(crate) fn handle_map_request(&mut self, _e: MapRequestEvent) -> Result<()> {
         todo!()
     }
-    pub(crate) fn handle_unmap_notify(&mut self, e: UnmapNotifyEvent) -> Result<()> {
+    pub(crate) fn handle_unmap_notify(&mut self, _e: UnmapNotifyEvent) -> Result<()> {
         todo!()
     }
 }
+
+/// Holds state related to the window manager's clients
+/// This contains a list of clients per monitor, alongside info on that monitor's screen size.
+#[derive(Debug, Default)]
+pub struct ClientState(Vec<MonitorInfo>);
+
+/// Info stored for a single monitor
+#[derive(Debug)]
+pub struct MonitorInfo {
+    /// Clients attached to that monitor
+    clients: Vec<Client>,
+
+    /// The monitor's geometry
+    screen_info: ScreenInfo,
+}
+
+/// Information about a single client / window
+#[derive(Debug)]
+pub struct Client {
+    // TODO
+}
+
+impl ClientState {
+    /// Set the new amount of screens, moving clients away if necessary
+    pub fn truncate_screens(&mut self, new_size: usize) {
+        // hack: double borrow stuff
+        let mut moved_clients = vec![];
+        for old in self.0.drain(new_size - self.0.len()..self.0.len()) {
+            moved_clients.extend(old.clients.into_iter());
+        }
+        self.0[0].clients.extend(moved_clients);
+    }
+
+    /// Set the info for the given screen, resizing the monitor list if necessary.
+    pub fn set_screen_info(&mut self, i: usize, info: ScreenInfo) {
+        while i >= self.0.len() {
+            self.0.push(MonitorInfo::default())
+        }
+        self.0[i].screen_info = info;
+    }
+
+    /// Get the amount of monitors this state is currently aware of
+    pub fn monitor_count(&self) -> usize {
+        self.0.len()
+    }
+}
+
+impl Default for MonitorInfo {
+    fn default() -> Self {
+        Self {
+            clients: vec![],
+            screen_info: ScreenInfo {
+                x_org: 0,
+                y_org: 0,
+                width: 0,
+                height: 0,
+            },
+        }
+    }
+}
diff --git a/src/cursors.rs b/src/cursors.rs
index 2cb8f0e..3153288 100644
--- a/src/cursors.rs
+++ b/src/cursors.rs
@@ -7,8 +7,10 @@ use xcb::{
 // https://tronche.com/gui/x/xlib/appendix/b/
 const XC_LEFT_PTR: u16 = 68;
 
+/// Caches X11 cursor objects
 #[derive(Debug)]
 pub struct Cursors {
+    #[allow(unused)] // Needs to be kept around since the cursors depend on it
     font: Font,
 
     pub normal: Cursor,
@@ -16,6 +18,7 @@ pub struct Cursors {
 }
 
 impl Cursors {
+    /// Load default cursors using the given connection.
     pub fn new_with(conn: &Connection) -> Result<Self> {
         // Open cursor font
         let font = conn.generate_id();
diff --git a/src/error.rs b/src/error.rs
index e12ae2c..3086204 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -1,7 +1,9 @@
 use thiserror::Error;
 
+/// The Result type used throughout
 pub type Result<T, E = Error> = std::result::Result<T, E>;
 
+/// All errors that can be encountered when running
 #[derive(Debug, Error)]
 pub enum Error {
     #[error("xcb returned screen that doesn't exist")]
@@ -10,12 +12,12 @@ pub enum Error {
     #[error("other wm is running")]
     OtherWMRunning,
 
+    #[error("generic xcb error: {0}")]
+    Xcb(#[from] xcb::Error),
+
     #[error("connection error: {0}")]
-    ConnectionError(#[from] xcb::ConnError),
+    Connection(#[from] xcb::ConnError),
 
     #[error("protocol error: {0}")]
-    ProtocolError(#[from] xcb::ProtocolError),
-
-    #[error("generic xcb error: {0}")]
-    XCBError(#[from] xcb::Error),
+    Protocol(#[from] xcb::ProtocolError),
 }
diff --git a/src/focus.rs b/src/focus.rs
index 5fec53f..ea4f021 100644
--- a/src/focus.rs
+++ b/src/focus.rs
@@ -1,19 +1,19 @@
-use xcb::x::{self, EnterNotifyEvent, FocusInEvent, MotionNotifyEvent};
+use xcb::x::{EnterNotifyEvent, FocusInEvent, MotionNotifyEvent};
 
 use crate::{error::*, WM};
 
 impl WM<'_> {
-    pub(crate) fn handle_enter_notify(&self, e: EnterNotifyEvent) -> Result<()> {
+    pub(crate) fn handle_enter_notify(&self, _e: EnterNotifyEvent) -> Result<()> {
         // todo!()
         Ok(())
     }
 
-    pub(crate) fn handle_focus_in(&self, e: FocusInEvent) -> Result<()> {
+    pub(crate) fn handle_focus_in(&self, _e: FocusInEvent) -> Result<()> {
         // todo!()
         Ok(())
     }
 
-    pub(crate) fn handle_motion_notify(&self, e: MotionNotifyEvent) -> Result<()> {
+    pub(crate) fn handle_motion_notify(&self, _e: MotionNotifyEvent) -> Result<()> {
         // todo!()
         Ok(())
     }
diff --git a/src/keys.rs b/src/keys.rs
index 7e3296f..7e94944 100644
--- a/src/keys.rs
+++ b/src/keys.rs
@@ -36,7 +36,7 @@ impl WM<'_> {
     /// Grab all keys specified by [`self::KEYBINDS`], ensuring we get events for them.
     pub(crate) fn grab_keys(&mut self) -> Result<()> {
         // Refresh keyboard state
-        self.keyboard_state = KeyboardState::new_with(&self.conn)?;
+        self.keyboard_state = KeyboardInfo::new_with(self.conn)?;
 
         // Ungrab all keys
         self.conn.send_request(&UngrabKey {
@@ -77,14 +77,14 @@ impl WM<'_> {
 }
 
 /// Cached information about our keyboard layout.
-pub struct KeyboardState {
+pub struct KeyboardInfo {
     min_keycode: RawKeyCode,
     max_keycode: RawKeyCode,
     numlock_mask: ModMask,
     mapping: GetKeyboardMappingReply,
 }
 
-impl KeyboardState {
+impl KeyboardInfo {
     /// Query information about the keyboard layout from the given connection.
     pub fn new_with(conn: &Connection) -> Result<Self> {
         let min_keycode = conn.get_setup().min_keycode();
@@ -107,8 +107,7 @@ impl KeyboardState {
         let keypermod = mod_map.keycodes().len() / 8;
         for i in 0..8 {
             for j in 0..keypermod {
-                if mod_map.keycodes()[(i * keypermod + j) as usize] as u32 == numlock_keycode.raw()
-                {
+                if mod_map.keycodes()[i * keypermod + j] as u32 == numlock_keycode.raw() {
                     this.numlock_mask =
                         ModMask::from_bits(1 << i).expect("x11 has unrecognised modifier");
                 }
@@ -152,7 +151,7 @@ impl KeyboardState {
             }
         }
 
-        return Ok(KeyCode::new(0));
+        Ok(KeyCode::new(0))
     }
 
     /// Lookup the keysym in the given column for the given keycode
diff --git a/src/main.rs b/src/main.rs
index f99e19b..6559ad7 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,8 +1,8 @@
 use atoms::InternedAtoms;
-use clients::ClientList;
+use clients::ClientState;
 use cursors::Cursors;
 use error::*;
-use keys::KeyboardState;
+use keys::KeyboardInfo;
 use xcb::{
     x::{self, ChangeWindowAttributes, PropertyNotifyEvent, Window},
     Connection, Event, Extension,
@@ -16,13 +16,10 @@ mod focus;
 mod keys;
 
 fn main() -> Result<()> {
-    // todo: cli stuff
-    let display_name = ":1";
-
     cleanup_process_children();
 
     let (conn, screen_num) =
-        Connection::connect_with_extensions(Some(display_name), &[], &[Extension::Xinerama])?;
+        Connection::connect_with_extensions(None, &[], &[Extension::Xinerama])?;
 
     let mut wm = WM::new(&conn, screen_num)?;
     wm.event_loop()?;
@@ -30,20 +27,35 @@ fn main() -> Result<()> {
     Ok(())
 }
 
+/// The window manager's state
 struct WM<'a> {
+    /// The open connection to an X server
     conn: &'a Connection,
+
+    /// The 'screen' number on the X server
+    /// Note this isn't what you think it is on multi-monitor setups
     screen_num: i32,
 
+    /// The root window
     root: Window,
-    clients: ClientList,
 
+    /// WM client state
+    clients: ClientState,
+
+    /// Cached cursors
     cursors: Cursors,
+
+    /// Cached atoms
     atoms: InternedAtoms,
-    keyboard_state: KeyboardState,
+
+    /// Cached keyboard layout information
+    keyboard_state: KeyboardInfo,
 }
 
 impl WM<'_> {
-    fn new<'a>(conn: &'a Connection, screen_num: i32) -> Result<WM<'a>> {
+    /// Prepare the window manager to run on the given connection and screen number.
+    /// This will fail if another WM is running.
+    fn new(conn: &'_ Connection, screen_num: i32) -> Result<WM<'_>> {
         // Fetch root window
         let setup = conn.get_setup();
         let screen = setup
@@ -64,7 +76,7 @@ impl WM<'_> {
         Ok(WM {
             atoms: InternedAtoms::new_with(conn)?,
             cursors: Cursors::new_with(conn)?,
-            keyboard_state: KeyboardState::new_with(conn)?,
+            keyboard_state: KeyboardInfo::new_with(conn)?,
             clients: Default::default(),
             conn,
             screen_num,
@@ -72,8 +84,9 @@ impl WM<'_> {
         })
     }
 
+    /// Set the correct properties on the root window
     fn setup_root(&mut self) -> Result<()> {
-        // TODO: set wm properties
+        // TODO: Set EHWM properties on root window
         self.conn
             .check_request(self.conn.send_request_checked(&ChangeWindowAttributes {
                 window: self.root,
@@ -100,42 +113,46 @@ impl WM<'_> {
     }
 
     fn event_loop(&mut self) -> Result<()> {
+        // Perform setup
         self.update_geometry()?;
         self.setup_root()?;
 
         loop {
             match self.conn.wait_for_event()? {
-                // todo: keybinding related stuff
+                // See keys.rs
                 Event::X(x::Event::KeyPress(e)) => self.handle_key_press(e)?,
                 Event::X(x::Event::MappingNotify(e)) => self.handle_mapping_notify(e)?,
 
-                // todo: windows coming and going
+                // See clients.rs
                 Event::X(x::Event::ConfigureRequest(e)) => self.handle_configure_request(e)?,
                 Event::X(x::Event::ConfigureNotify(e)) => self.handle_configure_notify(e)?,
                 Event::X(x::Event::DestroyNotify(e)) => self.handle_destroy_notify(e)?,
                 Event::X(x::Event::MapRequest(e)) => self.handle_map_request(e)?,
                 Event::X(x::Event::UnmapNotify(e)) => self.handle_unmap_notify(e)?,
 
-                // todo: focus stuff
+                // See focus.rs
                 Event::X(x::Event::EnterNotify(e)) => self.handle_enter_notify(e)?,
                 Event::X(x::Event::FocusIn(e)) => self.handle_focus_in(e)?,
                 Event::X(x::Event::MotionNotify(e)) => self.handle_motion_notify(e)?,
 
-                // todo: other
+                // See below
                 Event::X(x::Event::PropertyNotify(e)) => self.handle_property_notify(e)?,
                 _ => {}
             };
         }
     }
 
-    fn handle_property_notify(&self, e: PropertyNotifyEvent) -> Result<()> {
+    /// Handle a property notify event, by doing *todo*
+    fn handle_property_notify(&self, _e: PropertyNotifyEvent) -> Result<()> {
         todo!()
     }
 }
 
+/// Cleanup this process' children and set some flags.
+/// This is necessary when used with `startx`
 fn cleanup_process_children() {
-    // todo: dont transform children into zombies when they terminate
-    // todo: cleanup zombies
+    // TODO: dont transform children into zombies when they terminate
+    // TODO: cleanup zombies
 }
 
 impl<'a> std::fmt::Debug for WM<'a> {