fix: Only honour removal of fullscreen state if the state is currentl…

…y present (#311)

* chore: updating logging levels to make debug output less verbose

* fix: only honouring removal of fullscreen if the client is currently fullscreen

crates/penrose_ui/src/bar/schedule.rs

@@ -8,7 +8,7 @@ use std::{
     thread,
     time::{Duration, Instant},
 };
-use tracing::{debug, trace};
+use tracing::trace;
 
 /// The minimum allowed interval for an [UpdateSchedule].
 pub const MIN_DURATION: Duration = Duration::from_secs(1);
@@ -87,7 +87,7 @@ impl UpdateSchedule {
 /// their requested intervals.
 pub(crate) fn run_update_schedules(mut schedules: Vec<UpdateSchedule>) {
     thread::spawn(move || loop {
-        debug!("running UpdateSchedule updates for all pending widgets");
+        trace!("running UpdateSchedule updates for all pending widgets");
         while schedules[0].next < Instant::now() {
             schedules[0].update_text();
             schedules.sort_by(|a, b| a.next.cmp(&b.next));
@@ -98,7 +98,7 @@ pub(crate) fn run_update_schedules(mut schedules: Vec<UpdateSchedule>) {
         let _ = spawn_with_args("xsetroot", &["-name", ""]);
 
         let interval = schedules[0].next - Instant::now();
-        debug!(?interval, "sleeping until next update point");
+        trace!(?interval, "sleeping until next update point");
         thread::sleep(interval);
     });
 }

src/extensions/actions/mod.rs

@@ -6,7 +6,7 @@ use crate::{
     x::{atom::Atom, property::Prop, XConn, XConnExt},
     Error, Result, Xid,
 };
-use tracing::error;
+use tracing::{debug, error};
 
 mod dynamic_select;
 
@@ -42,6 +42,7 @@ pub fn set_fullscreen_state<X: XConn>(
     };
 
     let currently_fullscreen = wstate.contains(&full_screen);
+    debug!(%currently_fullscreen, ?action, %id, "setting fullscreen state");
 
     if action == Add || (action == Toggle && !currently_fullscreen) {
         let r = state
@@ -51,7 +52,7 @@ pub fn set_fullscreen_state<X: XConn>(
             .r;
         state.client_set.float(id, r)?;
         wstate.push(*full_screen);
-    } else if action == Remove || (action == Toggle && currently_fullscreen) {
+    } else if currently_fullscreen && (action == Remove || action == Toggle) {
         state.client_set.sink(&id);
         wstate.retain(|&val| val != *full_screen);
     }
@@ -86,7 +87,7 @@ pub fn toggle_fullscreen<X: XConn>() -> Box<dyn KeyEventHandler<X>> {
 /// DefaultWorkspace hook that allows for auto populating named Workspaces when first focusing them.
 ///
 /// > If you just want to dynamically select an existing workspace then you can use
-/// [switch_to_workspace] to select from known workspace names.
+/// > [switch_to_workspace] to select from known workspace names.
 ///
 ///   [0]: crate::pure::Workspace
 pub fn create_or_switch_to_workspace<X>(

src/extensions/hooks/ewmh.rs

@@ -16,7 +16,7 @@ use crate::{
     },
     Result, Xid,
 };
-use tracing::warn;
+use tracing::{debug, warn};
 
 /// The set of Atoms this extension adds support for.
 ///
@@ -95,6 +95,8 @@ pub fn event_hook<X: XConn>(event: &XEvent, state: &mut State<X>, x: &X) -> Resu
         _ => return Ok(true),
     };
 
+    debug!(?dtype, "processing client message in ewmh hook");
+
     match dtype.as_ref() {
         // Focus the requested desktop
         "_NET_CURRENT_DESKTOP" => {

src/pure/stack_set.rs

@@ -15,6 +15,7 @@ use std::{
     hash::Hash,
     mem::{swap, take},
 };
+use tracing::debug;
 
 /// The side-effect free internal state representation of the window manager.
 #[derive(Default, Debug, Clone)]
@@ -259,6 +260,7 @@ where
     pub(crate) fn float_unchecked<R: RelativeTo>(&mut self, client: C, r: R) {
         let screen = self.screen_for_client(&client).expect("client to be known");
         let r = r.relative_to(&screen.r);
+        debug!(?r, "setting floating position");
         self.floating.insert(client, r);
     }
 

src/util.rs

@@ -8,7 +8,7 @@ use std::{
     io::Read,
     process::{Command, Stdio},
 };
-use tracing::debug;
+use tracing::trace;
 
 /// Run an external command
 ///
@@ -58,7 +58,7 @@ pub fn spawn_with_args<S: Into<String>>(cmd: S, args: &[&str]) -> Result<()> {
 /// > output of a process that you spawn.
 pub fn spawn_for_output<S: Into<String>>(cmd: S) -> std::io::Result<String> {
     let cmd = cmd.into();
-    debug!(?cmd, "spawning subprocess for output");
+    trace!(?cmd, "spawning subprocess for output");
     let parts: Vec<&str> = cmd.split_whitespace().collect();
     let result = if parts.len() > 1 {
         Command::new(parts[0])
@@ -69,7 +69,7 @@ pub fn spawn_for_output<S: Into<String>>(cmd: S) -> std::io::Result<String> {
         Command::new(parts[0]).stdout(Stdio::piped()).spawn()
     };
 
-    debug!(?cmd, "reading output");
+    trace!(?cmd, "reading output");
     let mut child = result?;
     let mut buff = String::new();
     child
@@ -91,13 +91,13 @@ pub fn spawn_for_output_with_args<S: Into<String>>(
 ) -> std::io::Result<String> {
     let cmd = cmd.into();
 
-    debug!(?cmd, ?args, "spawning subprocess for output");
+    trace!(?cmd, ?args, "spawning subprocess for output");
     let mut child = Command::new(&cmd)
         .stdout(Stdio::piped())
         .args(args)
         .spawn()?;
 
-    debug!(?cmd, ?args, "reading output");
+    trace!(?cmd, ?args, "reading output");
     let mut buff = String::new();
     child
         .stdout

src/x/mod.rs

@@ -12,7 +12,7 @@ use crate::{
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 use std::collections::{HashMap, HashSet};
-use tracing::{error, trace};
+use tracing::{debug, error, trace};
 
 pub mod atom;
 pub mod event;
@@ -258,19 +258,20 @@ pub trait XConnExt: XConn + Sized {
         trace!(%client, "fetching WmClass prop");
         if let Some(Prop::UTF8String(strs)) = self.get_prop(client, Atom::WmClass.as_ref())? {
             if strs.iter().any(|c| floating_classes.contains(c)) {
-                trace!(%client, ?floating_classes, "window has a floating class: setting to floating state");
+                debug!(%client, ?floating_classes, "window has a floating class: setting to floating state");
                 return Ok(true);
             }
         }
 
+        trace!(%client, "fetching NetWmWindowType prop");
+        let window_types = self.get_prop(client, Atom::NetWmWindowType.as_ref())?;
+        debug!(?window_types, "client window types");
+
         let float_types: Vec<&str> = AUTO_FLOAT_WINDOW_TYPES.iter().map(|a| a.as_ref()).collect();
 
-        trace!(%client, "fetching NetWmWindowType prop");
-        let p = self.get_prop(client, Atom::NetWmWindowType.as_ref())?;
-        let should_float = if let Some(Prop::Atom(atoms)) = p {
-            atoms.iter().any(|a| float_types.contains(&a.as_ref()))
-        } else {
-            false
+        let should_float = match window_types {
+            Some(Prop::Atom(atoms)) => atoms.iter().any(|a| float_types.contains(&a.as_ref())),
+            _ => false,
         };
 
         Ok(should_float)
@@ -459,6 +460,7 @@ pub(crate) fn manage_without_refresh<X: XConn>(
                 .or(tag)
                 .map(|t| t.to_string());
 
+            debug!(%id, %parent, ?owned_tag, "client is transient");
             (owned_tag, Some(parent))
         }
 
@@ -474,6 +476,7 @@ pub(crate) fn manage_without_refresh<X: XConn>(
     }
 
     if should_float {
+        debug!(%id, "client should float");
         let r = floating_client_position(id, transient_for, state, x)?;
         if state.client_set.float(id, r).is_err() {
             error!(%id, "attempted to float client which was not in state");
@@ -489,6 +492,10 @@ pub(crate) fn manage_without_refresh<X: XConn>(
     }
     state.config.manage_hook = hook;
 
+    debug!(
+        floating=?state.client_set.floating, "floating clients"
+    );
+
     Ok(())
 }
 
@@ -502,22 +509,27 @@ fn floating_client_position<X: XConn>(
     state: &State<X>,
     x: &X,
 ) -> Result<Rect> {
+    trace!(%id, "fetching client geometry");
     let r_initial = x.client_geometry(id)?;
+    debug!(?r_initial, "initial geometry");
 
     if (r_initial.x, r_initial.y) != (0, 0) {
+        debug!(?r_initial, "accepting client's requested position");
         return Ok(r_initial);
     }
 
-    let r_screen = transient_for
+    let r_parent = transient_for
         .and_then(|parent| state.client_set.screen_for_client(&parent))
         .unwrap_or(&state.client_set.screens.focus)
         .r;
+    debug!(?r_parent, "parent geometry");
 
-    let r_final = r_initial.centered_in(&r_screen).unwrap_or_else(|| {
+    let r_final = r_initial.centered_in(&r_parent).unwrap_or_else(|| {
         r_initial
             .centered_in(&state.client_set.screens.focus.r)
             .unwrap_or(r_initial)
     });
+    debug!(?r_final, "final geometry");
 
     Ok(r_final)
 }