Some refactorings and improvements

Cargo.lock

@@ -78,9 +78,9 @@ dependencies = [
 
 [[package]]
 name = "clap"
-version = "3.1.16"
+version = "3.1.17"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f52d4f8e4a1419219935762e32913b4430f37cb0c0200ad17a89ee18c0188a9f"
+checksum = "47582c09be7c8b32c0ab3a6181825ababb713fde6fff20fc573a3870dd45c6a0"
 dependencies = [
  "atty",
  "bitflags",

swayrbar/src/module.rs

@@ -24,7 +24,7 @@ pub mod pactl;
 pub mod sysinfo;
 pub mod window;
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq)]
 pub enum RefreshReason {
     ClickEvent,
     SwayEvent,
@@ -32,25 +32,17 @@ pub enum RefreshReason {
 
 pub type NameInstanceAndReason = (String, String, RefreshReason);
 
-fn should_refresh(
-    m: &dyn BarModuleFn,
-    nai: &Option<NameInstanceAndReason>,
-) -> bool {
-    let cfg = m.get_config();
-    match nai {
-        None => true,
-        Some((n, i, _)) => n == &cfg.name && i == &cfg.instance,
-    }
-}
-
 pub trait BarModuleFn: Sync + Send {
     fn create(config: config::ModuleConfig) -> Box<dyn BarModuleFn>
     where
         Self: Sized;
+
     fn default_config(instance: String) -> config::ModuleConfig
     where
         Self: Sized;
+
     fn get_config(&self) -> &config::ModuleConfig;
+
     fn get_on_click_map(
         &self,
         name: &str,
@@ -63,6 +55,25 @@ pub trait BarModuleFn: Sync + Send {
             None
         }
     }
+
     fn build(&self, nai: &Option<NameInstanceAndReason>) -> s::Block;
+
+    fn should_refresh(
+        &self,
+        nai: &Option<NameInstanceAndReason>,
+        periodic: bool,
+        reasons: &[RefreshReason],
+    ) -> bool {
+        let cfg = self.get_config();
+        match nai {
+            None => periodic,
+            Some((n, i, r)) => {
+                n == &cfg.name
+                    && i == &cfg.instance
+                    && reasons.iter().any(|x| x == r)
+            }
+        }
+    }
+
     fn subst_args<'a>(&'a self, _cmd: &'a [String]) -> Option<Vec<String>>;
 }

swayrbar/src/module/battery.rs

@@ -16,7 +16,7 @@
 //! The date `swayrbar` module.
 
 use crate::config;
-use crate::module::{should_refresh, BarModuleFn, NameInstanceAndReason};
+use crate::module::{BarModuleFn, NameInstanceAndReason};
 use crate::shared::fmt::subst_placeholders;
 use battery as bat;
 use std::collections::HashSet;
@@ -29,6 +29,7 @@ struct State {
     state_of_charge: f32,
     state_of_health: f32,
     state: String,
+    cached_text: String,
 }
 
 pub struct BarModuleBattery {
@@ -50,7 +51,7 @@ fn get_refreshed_batteries(
     Ok(bats)
 }
 
-fn refresh_state(state: &mut State) {
+fn refresh_state(state: &mut State, fmt_str: &str, html_escape: bool) {
     // FIXME: Creating the Manager on every refresh is bad but internally
     // it uses an Rc so if I keep it as a field of BarModuleBattery, that
     // cannot be Sync.
@@ -86,7 +87,8 @@ fn refresh_state(state: &mut State) {
                     comma_sep_string += "]";
                     comma_sep_string
                 }
-            }
+            };
+            state.cached_text = subst_placeholders(fmt_str, html_escape, state);
         }
         Err(err) => {
             log::error!("Could not update battery state: {}", err);
@@ -94,7 +96,7 @@ fn refresh_state(state: &mut State) {
     }
 }
 
-fn get_text(fmt: &str, html_escape: bool, state: &State) -> String {
+fn subst_placeholders(fmt: &str, html_escape: bool, state: &State) -> String {
     subst_placeholders!(fmt, html_escape, {
         "state_of_charge" => state.state_of_charge,
         "state_of_health" => state.state_of_health,
@@ -110,6 +112,7 @@ impl BarModuleFn for BarModuleBattery {
                 state_of_charge: 0.0,
                 state_of_health: 0.0,
                 state: "Unknown".to_owned(),
+                cached_text: String::new(),
             }),
         })
     }
@@ -131,16 +134,18 @@ impl BarModuleFn for BarModuleBattery {
     fn build(&self, nai: &Option<NameInstanceAndReason>) -> s::Block {
         let mut state = self.state.lock().expect("Could not lock state.");
 
-        if should_refresh(self, nai) {
+        if self.should_refresh(nai, true, &[]) {
-            refresh_state(&mut state);
+            refresh_state(
+                &mut state,
+                &self.config.format,
+                self.get_config().is_html_escape(),
+            );
         }
 
-        let text =
-            get_text(&self.config.format, self.config.is_html_escape(), &state);
         s::Block {
             name: Some(NAME.to_owned()),
             instance: Some(self.config.instance.clone()),
-            full_text: text,
+            full_text: state.cached_text.to_owned(),
             align: Some(s::Align::Left),
             markup: Some(s::Markup::Pango),
             short_text: None,
@@ -160,6 +165,10 @@ impl BarModuleFn for BarModuleBattery {
 
     fn subst_args<'a>(&'a self, cmd: &'a [String]) -> Option<Vec<String>> {
         let state = self.state.lock().expect("Could not lock state.");
-        Some(cmd.iter().map(|arg| get_text(arg, false, &state)).collect())
+        Some(
+            cmd.iter()
+                .map(|arg| subst_placeholders(arg, false, &state))
+                .collect(),
+        )
     }
 }

swayrbar/src/module/date.rs

@@ -15,14 +15,21 @@
 
 //! The date `swayrbar` module.
 
+use std::sync::Mutex;
+
 use crate::module::config;
 use crate::module::{BarModuleFn, NameInstanceAndReason};
 use swaybar_types as s;
 
 const NAME: &str = "date";
 
+struct State {
+    cached_text: String,
+}
+
 pub struct BarModuleDate {
     config: config::ModuleConfig,
+    state: Mutex<State>,
 }
 
 fn chrono_format(s: &str) -> String {
@@ -31,7 +38,12 @@ fn chrono_format(s: &str) -> String {
 
 impl BarModuleFn for BarModuleDate {
     fn create(cfg: config::ModuleConfig) -> Box<dyn BarModuleFn> {
-        Box::new(BarModuleDate { config: cfg })
+        Box::new(BarModuleDate {
+            config: cfg,
+            state: Mutex::new(State {
+                cached_text: String::new(),
+            }),
+        })
     }
 
     fn default_config(instance: String) -> config::ModuleConfig {
@@ -48,25 +60,17 @@ impl BarModuleFn for BarModuleDate {
         &self.config
     }
 
-    fn get_on_click_map(
+    fn build(&self, nai: &Option<NameInstanceAndReason>) -> s::Block {
-        &self,
+        let mut state = self.state.lock().expect("Could not lock state.");
-        name: &str,
+
-        instance: &str,
+        if self.should_refresh(nai, true, &[]) {
-    ) -> Option<&std::collections::HashMap<String, Vec<String>>> {
+            state.cached_text = chrono_format(&self.config.format);
-        let cfg = self.get_config();
-        if name == cfg.name && instance == cfg.instance {
-            cfg.on_click.as_ref()
-        } else {
-            None
-        }
         }
 
-    fn build(&self, _nai: &Option<NameInstanceAndReason>) -> s::Block {
-        let text = chrono_format(&self.config.format);
         s::Block {
             name: Some(NAME.to_owned()),
             instance: Some(self.config.instance.clone()),
-            full_text: text,
+            full_text: state.cached_text.to_owned(),
             align: Some(s::Align::Left),
             markup: Some(s::Markup::Pango),
             short_text: None,

swayrbar/src/module/pactl.rs

@@ -16,7 +16,7 @@
 //! The pactl `swayrbar` module.
 
 use crate::config;
-use crate::module::{should_refresh, BarModuleFn, NameInstanceAndReason};
+use crate::module::{BarModuleFn, NameInstanceAndReason};
 use crate::shared::fmt::subst_placeholders;
 use once_cell::sync::Lazy;
 use regex::Regex;
@@ -25,11 +25,14 @@ use std::process::Command;
 use std::sync::Mutex;
 use swaybar_types as s;
 
+use super::RefreshReason;
+
 const NAME: &str = "pactl";
 
 struct State {
     volume: u8,
     muted: bool,
+    cached_text: String,
 }
 
 pub static VOLUME_RX: Lazy<Regex> =
@@ -62,12 +65,13 @@ pub struct BarModulePactl {
     state: Mutex<State>,
 }
 
-fn refresh_state(state: &mut State) {
+fn refresh_state(state: &mut State, fmt_str: &str, html_escape: bool) {
     state.volume = get_volume();
     state.muted = get_mute_state();
+    state.cached_text = subst_placeholders(fmt_str, html_escape, state);
 }
 
-fn get_text(fmt: &str, html_escape: bool, state: &State) -> String {
+fn subst_placeholders(fmt: &str, html_escape: bool, state: &State) -> String {
     subst_placeholders!(fmt, html_escape, {
         "volume" => {
             state.volume
@@ -92,6 +96,7 @@ impl BarModuleFn for BarModulePactl {
             state: Mutex::new(State {
                 volume: 255_u8,
                 muted: false,
+                cached_text: String::new(),
             }),
         })
     }
@@ -145,16 +150,18 @@ impl BarModuleFn for BarModulePactl {
     fn build(&self, nai: &Option<NameInstanceAndReason>) -> s::Block {
         let mut state = self.state.lock().expect("Could not lock state.");
 
-        if should_refresh(self, nai) {
+        if self.should_refresh(nai, true, &[RefreshReason::ClickEvent]) {
-            refresh_state(&mut state);
+            refresh_state(
+                &mut state,
+                &self.config.format,
+                self.config.is_html_escape(),
+            );
         }
 
-        let text =
-            get_text(&self.config.format, self.config.is_html_escape(), &state);
         s::Block {
             name: Some(NAME.to_owned()),
             instance: Some(self.config.instance.clone()),
-            full_text: text,
+            full_text: state.cached_text.to_owned(),
             align: Some(s::Align::Left),
             markup: Some(s::Markup::Pango),
             short_text: None,
@@ -174,6 +181,10 @@ impl BarModuleFn for BarModulePactl {
 
     fn subst_args<'a>(&'a self, cmd: &'a [String]) -> Option<Vec<String>> {
         let state = self.state.lock().expect("Could not lock state.");
-        Some(cmd.iter().map(|arg| get_text(arg, false, &state)).collect())
+        Some(
+            cmd.iter()
+                .map(|arg| subst_placeholders(arg, false, &state))
+                .collect(),
+        )
     }
 }

swayrbar/src/module/sysinfo.rs

@@ -16,7 +16,7 @@
 //! The date `swayrbar` module.
 
 use crate::config;
-use crate::module::{should_refresh, BarModuleFn, NameInstanceAndReason};
+use crate::module::{BarModuleFn, NameInstanceAndReason};
 use crate::shared::fmt::subst_placeholders;
 use std::collections::HashMap;
 use std::sync::Mutex;
@@ -34,6 +34,7 @@ struct State {
     load_avg_1: f64,
     load_avg_5: f64,
     load_avg_15: f64,
+    cached_text: String,
 }
 
 pub struct BarModuleSysInfo {
@@ -95,16 +96,22 @@ fn get_load_average(
     }
 }
 
-fn refresh_state(sys: &mut si::System, state: &mut State) {
+fn refresh_state(
+    sys: &mut si::System,
+    state: &mut State,
+    fmt_str: &str,
+    html_escape: bool,
+) {
     let updater = OnceRefresher::new();
     state.cpu_usage = get_cpu_usage(sys, &updater);
     state.mem_usage = get_memory_usage(sys, &updater);
     state.load_avg_1 = get_load_average(sys, LoadAvg::One, &updater);
     state.load_avg_5 = get_load_average(sys, LoadAvg::Five, &updater);
     state.load_avg_15 = get_load_average(sys, LoadAvg::Fifteen, &updater);
+    state.cached_text = subst_placeholders(fmt_str, html_escape, state);
 }
 
-fn get_text(fmt: &str, html_escape: bool, state: &State) -> String {
+fn subst_placeholders(fmt: &str, html_escape: bool, state: &State) -> String {
     subst_placeholders!(fmt, html_escape, {
         "cpu_usage" => state.cpu_usage,
         "mem_usage" => state.mem_usage,
@@ -125,6 +132,7 @@ impl BarModuleFn for BarModuleSysInfo {
                 load_avg_1: 0.0,
                 load_avg_5: 0.0,
                 load_avg_15: 0.0,
+                cached_text: String::new(),
             }),
         })
     }
@@ -149,18 +157,19 @@ impl BarModuleFn for BarModuleSysInfo {
         let mut sys = self.system.lock().expect("Could not lock state.");
         let mut state = self.state.lock().expect("Could not lock state.");
 
-        if should_refresh(self, nai) {
+        if self.should_refresh(nai, true, &[]) {
-            refresh_state(&mut sys, &mut state);
+            refresh_state(
+                &mut sys,
+                &mut state,
+                &self.config.format,
+                self.config.is_html_escape(),
+            );
         }
 
         s::Block {
             name: Some(NAME.to_owned()),
             instance: Some(self.config.instance.clone()),
-            full_text: get_text(
+            full_text: state.cached_text.to_owned(),
-                &self.config.format,
-                self.config.is_html_escape(),
-                &state,
-            ),
             align: Some(s::Align::Left),
             markup: Some(s::Markup::Pango),
             short_text: None,
@@ -180,6 +189,10 @@ impl BarModuleFn for BarModuleSysInfo {
 
     fn subst_args<'a>(&'a self, cmd: &'a [String]) -> Option<Vec<String>> {
         let state = self.state.lock().expect("Could not lock state.");
-        Some(cmd.iter().map(|arg| get_text(arg, false, &state)).collect())
+        Some(
+            cmd.iter()
+                .map(|arg| subst_placeholders(arg, false, &state))
+                .collect(),
+        )
     }
 }

swayrbar/src/module/window.rs

@@ -19,12 +19,14 @@ use std::collections::HashMap;
 use std::sync::Mutex;
 
 use crate::config;
-use crate::module::{should_refresh, BarModuleFn, NameInstanceAndReason};
+use crate::module::{BarModuleFn, NameInstanceAndReason};
 use crate::shared::fmt::subst_placeholders;
 use crate::shared::ipc;
 use crate::shared::ipc::NodeMethods;
 use swaybar_types as s;
 
+use super::RefreshReason;
+
 pub const NAME: &str = "window";
 
 const INITIAL_PID: i32 = -128;
@@ -119,7 +121,7 @@ impl BarModuleFn for BarModuleWindow {
         // initially at startup and when explicitly named by `nai` (caused by a
         // window or workspace event).
         if state.pid == INITIAL_PID
-            || (nai.is_some() && should_refresh(self, nai))
+            || (self.should_refresh(nai, false, &[RefreshReason::SwayEvent]))
         {
             refresh_state(
                 &mut state,
@@ -149,19 +151,6 @@ impl BarModuleFn for BarModuleWindow {
         }
     }
 
-    fn get_on_click_map(
-        &self,
-        name: &str,
-        instance: &str,
-    ) -> Option<&HashMap<String, Vec<String>>> {
-        let cfg = self.get_config();
-        if name == cfg.name && instance == cfg.instance {
-            cfg.on_click.as_ref()
-        } else {
-            None
-        }
-    }
-
     fn subst_args<'b>(&'b self, cmd: &'b [String]) -> Option<Vec<String>> {
         let state = self.state.lock().expect("Could not lock state.");
         let cmd = cmd