diff options
| -rw-r--r-- | crates/core/src/manager/lock.rs | 16 | ||||
| -rw-r--r-- | crates/core/src/manager/mod.rs | 24 | ||||
| -rw-r--r-- | crates/platform/src/ipc/dispatch.rs | 38 | ||||
| -rw-r--r-- | crates/platform/src/ipc/mod.rs | 1 | ||||
| -rw-r--r-- | crates/platform/src/ipc/tests.rs | 39 | ||||
| -rw-r--r-- | crates/platform/src/lib.rs | 2 | ||||
| -rw-r--r-- | crates/srdwm/src/main.rs | 13 | ||||
| -rw-r--r-- | docs/DEFAULTS.md | 19 | ||||
| -rw-r--r-- | docs/TODO.md | 32 |
9 files changed, 169 insertions, 15 deletions
diff --git a/crates/core/src/manager/lock.rs b/crates/core/src/manager/lock.rs index 0ffe0ca..15c0a9f 100644 --- a/crates/core/src/manager/lock.rs +++ b/crates/core/src/manager/lock.rs @@ -51,6 +51,22 @@ impl WindowManager { pub fn drain_refresh_request(&mut self) -> bool { std::mem::take(&mut self.refresh_requested) } + + /// Records that `key` was set live to `value_json`. See + /// `live_settings`' own doc comment. + /// + /// Last write wins, so setting the same key twice leaves one entry and + /// the replay applies each key exactly once. + pub fn record_live_setting(&mut self, key: &str, value_json: String) { + self.live_settings.insert(key.to_string(), value_json); + } + + /// Every live setting recorded so far, for replay after a config + /// reload. Cloned rather than borrowed: the replay mutates the same + /// `WindowManager` this came from. + pub fn live_settings(&self) -> Vec<(String, String)> { + self.live_settings.iter().map(|(k, v)| (k.clone(), v.clone())).collect() + } } #[cfg(test)] diff --git a/crates/core/src/manager/mod.rs b/crates/core/src/manager/mod.rs index 7454ce2..e778a32 100644 --- a/crates/core/src/manager/mod.rs +++ b/crates/core/src/manager/mod.rs @@ -437,6 +437,29 @@ pub struct WindowManager { /// Set by `request_refresh`, drained by the main loop. Same /// cross-boundary queued-request shape as `lock_requested`. refresh_requested: bool, + /// Every setting changed live since startup, as `srd set` key -> the + /// raw JSON text of its value, in insertion-independent key order. + /// + /// Exists so a config reload does not silently undo a change the user + /// just made by hand. `apply_general_settings` rebuilds the whole + /// `ThemeConfig` and general-settings block from the config file, which + /// is the correct precedence for a *file* edit - but it also wiped + /// every live `srd set`, and the titlebar right-click menu's own + /// "Customize" section is built entirely out of live `srd set`s. So + /// changing a button style from that menu and then saving `init.lua` + /// for any unrelated reason silently reverted it. + /// + /// That was survivable while reloads only happened on an explicit + /// `Mod4+Ctrl+r`. `general.config_reload_on_write` makes a reload + /// happen on every save, which turns a rare surprise into a reliable + /// one - a control that silently reverts is worse than no control. + /// + /// Raw JSON text rather than a typed value because this crate has no + /// serde dependency and no business gaining one for this; the platform + /// crate parses it back and replays it through the same `handle_set` + /// that recorded it, so a replayed setting cannot diverge from a real + /// one. `BTreeMap` for a deterministic replay order. + live_settings: std::collections::BTreeMap<String, String>, drag: Option<DragState>, resize: Option<ResizeState>, rules: Vec<WindowRule>, @@ -601,6 +624,7 @@ impl WindowManager { theme: ThemeConfig::default(), lock: LockConfig::default(), refresh_requested: false, + live_settings: std::collections::BTreeMap::new(), drag: None, resize: None, rules: Vec::new(), diff --git a/crates/platform/src/ipc/dispatch.rs b/crates/platform/src/ipc/dispatch.rs index d2232c8..749b2aa 100644 --- a/crates/platform/src/ipc/dispatch.rs +++ b/crates/platform/src/ipc/dispatch.rs @@ -437,6 +437,44 @@ pub(crate) fn handle_request(line: &[u8], wm: &std::rc::Rc<std::cell::RefCell<Wi /// predicate is what the two colour/width arms below walk existing /// windows with, rather than touching every window unconditionally. fn handle_set(req: &serde_json::Value, wm: &std::rc::Rc<std::cell::RefCell<WindowManager>>) -> (Vec<u8>, bool) { + let (response, changed) = apply_set(req, wm); + // Recorded only on success, so a rejected value is never replayed -- + // and only for a real client call, not for the replay itself (see + // `replay_live_settings`, which calls `apply_set` directly and would + // otherwise rewrite what it is currently reading). + if changed { + if let (Some(key), Some(value)) = (req.get("key").and_then(|v| v.as_str()), req.get("value")) { + wm.borrow_mut().record_live_setting(key, value.to_string()); + } + } + (response, changed) +} + +/// Re-applies every setting changed live since startup. +/// +/// Called after a config reload, which rebuilds the theme and general +/// settings from the config file and would otherwise silently undo them -- +/// see `WindowManager::live_settings`' own doc comment for why that matters +/// more now that a reload happens on every save. +/// +/// Replayed through `apply_set`, the same function that applied them +/// originally, so a replayed setting cannot behave differently from a real +/// one. Failures are ignored: a value that no longer applies (a monitor +/// that has gone away, say) should not stop the rest being restored. +pub fn replay_live_settings(wm: &std::rc::Rc<std::cell::RefCell<WindowManager>>) -> usize { + let recorded = wm.borrow().live_settings(); + let mut applied = 0; + for (key, raw) in recorded { + let Ok(value) = serde_json::from_str::<serde_json::Value>(&raw) else { continue }; + let req = serde_json::json!({ "cmd": "set", "key": key, "value": value }); + if apply_set(&req, wm).1 { + applied += 1; + } + } + applied +} + +fn apply_set(req: &serde_json::Value, wm: &std::rc::Rc<std::cell::RefCell<WindowManager>>) -> (Vec<u8>, bool) { let key = req.get("key").and_then(|v| v.as_str()).unwrap_or(""); let value = req.get("value"); match key { diff --git a/crates/platform/src/ipc/mod.rs b/crates/platform/src/ipc/mod.rs index 796c459..7094f6e 100644 --- a/crates/platform/src/ipc/mod.rs +++ b/crates/platform/src/ipc/mod.rs @@ -37,6 +37,7 @@ use std::path::PathBuf; use srdwm_core::WindowManager; mod dispatch; +pub use dispatch::replay_live_settings; mod types; #[cfg(test)] mod tests; diff --git a/crates/platform/src/ipc/tests.rs b/crates/platform/src/ipc/tests.rs index c2a678c..2e61761 100644 --- a/crates/platform/src/ipc/tests.rs +++ b/crates/platform/src/ipc/tests.rs @@ -26,6 +26,45 @@ fn read_line(reader: &mut std::io::BufReader<UnixStream>) -> String { } #[test] +fn a_live_setting_is_recorded_and_survives_a_replay() { + // The concrete regression: a config reload rebuilds the theme from the + // file, so a value changed by hand (the titlebar menu's own Customize + // rows are all live `srd set`s) was silently reverted. + let wm = std::rc::Rc::new(std::cell::RefCell::new(WindowManager::new())); + let req = serde_json::json!({"cmd": "set", "key": "button_side", "value": "left"}); + let (_, changed) = super::dispatch::handle_request(serde_json::to_vec(&req).unwrap().as_slice(), &wm); + assert!(changed); + assert!(wm.borrow().theme.buttons_left); + + // Stand in for what a reload does to the theme. + wm.borrow_mut().theme = srdwm_core::ThemeConfig::default(); + assert!(!wm.borrow().theme.buttons_left, "the rebuild really did drop it"); + + assert_eq!(super::replay_live_settings(&wm), 1); + assert!(wm.borrow().theme.buttons_left, "the live change must come back"); +} + +#[test] +fn a_rejected_value_is_never_recorded_for_replay() { + let wm = std::rc::Rc::new(std::cell::RefCell::new(WindowManager::new())); + let req = serde_json::json!({"cmd": "set", "key": "button_side", "value": "sideways"}); + let (_, changed) = super::dispatch::handle_request(serde_json::to_vec(&req).unwrap().as_slice(), &wm); + assert!(!changed, "an invalid value must be rejected"); + assert_eq!(super::replay_live_settings(&wm), 0, "nothing should have been recorded"); +} + +#[test] +fn setting_the_same_key_twice_replays_only_the_last_value() { + let wm = std::rc::Rc::new(std::cell::RefCell::new(WindowManager::new())); + for value in ["left", "right"] { + let req = serde_json::json!({"cmd": "set", "key": "button_side", "value": value}); + super::dispatch::handle_request(serde_json::to_vec(&req).unwrap().as_slice(), &wm); + } + assert_eq!(super::replay_live_settings(&wm), 1, "one entry per key, not one per call"); + assert!(!wm.borrow().theme.buttons_left, "the last value written is the one that survives"); +} + +#[test] fn subscribe_gets_an_immediate_snapshot() { let dir = tempfile::tempdir().unwrap(); let mut server = IpcServer::bind_in(dir.path(), "test").unwrap(); diff --git a/crates/platform/src/lib.rs b/crates/platform/src/lib.rs index 5de701f..b69aa02 100644 --- a/crates/platform/src/lib.rs +++ b/crates/platform/src/lib.rs @@ -12,7 +12,7 @@ mod appmenu_registrar; pub use appmenu_registrar::{AppmenuRegistrarState, RegistrarEvent}; mod ipc; -pub use ipc::IpcServer; +pub use ipc::{replay_live_settings, IpcServer}; #[cfg(unix)] mod pam_auth; diff --git a/crates/srdwm/src/main.rs b/crates/srdwm/src/main.rs index e21e585..94da20c 100644 --- a/crates/srdwm/src/main.rs +++ b/crates/srdwm/src/main.rs @@ -798,6 +798,15 @@ fn main() -> Result<(), Box<dyn std::error::Error>> { Err(e) => report_config_error(&format!("Config edit not applied, keeping the last working one.\n{e}")), } apply_general_settings(&engine, &wm); + // After `apply_general_settings`, which rebuilds the + // theme from the config file - see + // `WindowManager::live_settings` for why a hand-made + // change has to win over that on a *reload*, even + // though the file wins at startup. + let replayed = srdwm_platform::replay_live_settings(&wm); + if replayed > 0 { + log::info!("re-applied {replayed} live setting(s) after the reload"); + } } } } @@ -810,6 +819,8 @@ fn main() -> Result<(), Box<dyn std::error::Error>> { Ok(()) => log::info!("config reloaded (desktop refresh)"), Err(e) => report_config_error(&format!("Config reload failed, keeping the last working one.\n{e}")), } + apply_general_settings(&engine, &wm); + srdwm_platform::replay_live_settings(&wm); // After the reload, so a handler edited in the config since // startup is the one that runs. engine.dispatch_event("refresh"); @@ -825,6 +836,8 @@ fn main() -> Result<(), Box<dyn std::error::Error>> { Ok(()) => log::info!("config reloaded"), Err(e) => report_config_error(&format!("Config reload failed, keeping the last working one.\n{e}")), } + apply_general_settings(&engine, &wm); + srdwm_platform::replay_live_settings(&wm); } else if !engine.dispatch_keybinding(&combo) { log::debug!("no binding for '{combo}'"); } diff --git a/docs/DEFAULTS.md b/docs/DEFAULTS.md index a130434..3497410 100644 --- a/docs/DEFAULTS.md +++ b/docs/DEFAULTS.md @@ -867,10 +867,17 @@ checks the config directory's `.lua` modification times once a second and reloads when one changes. Set it to `false` for a config that does expensive work at load time. `Mod4+Ctrl+r` still reloads on demand in either case. -Two limits to know. A reload does not re-register key *grabs* with the +**A hand-made change is not undone by a reload.** A reload rebuilds the +theme and general settings from the config file, which is right for a file +edit but would also wipe every live `srd set` - and the titlebar +right-click menu's "Customize" rows are all live `srd set`s. Every setting +changed live is recorded and re-applied after each reload, so changing a +button style from that menu and then saving `init.lua` for an unrelated +reason keeps the change. A live value stays until it is changed again or the +session ends; it is a session override, not a persisted setting, so write it +into the config to keep it across restarts. + +One limit to know: a reload does not re-register key *grabs* with the backend, so a brand new key combination needs a restart before the -compositor sees that key at all; an existing combination picks up its new -action immediately. And a reload rebuilds the theme from the config file, so -it discards live `srd set` theme changes - including titlebar settings -changed through the right-click menu. That is the correct precedence, but -with reload-on-write it now happens every time the config is saved. +compositor sees that key at all. An existing combination picks up its new +action immediately. diff --git a/docs/TODO.md b/docs/TODO.md index 49f7312..d24f9b8 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -104,14 +104,30 @@ investigations in one day started from a screenshot that was quietly lying -- draw (border strips and the desktop icon grid), and `winit/render.rs` carries a pointer to it at the place a new tier gets added. -**One interaction worth knowing.** A config reload rebuilds `ThemeConfig` from -the config file, so it discards live `srd set` theme changes - correct -precedence, and pre-existing, but auto-reload makes it happen on every save -rather than only when the reload key is pressed. A titlebar customised live -through the right-click menu reverts the next time `init.lua` is saved. - -Full workspace build/test/clippy clean: 512 tests (262 core / 160 wayland / -43 platform / 34 config / 13 ctl), 0 failed, 0 clippy warnings. +**A follow-on defect, found and fixed rather than documented away.** A config +reload rebuilds `ThemeConfig` from the config file, so it discarded every +live `srd set` - and the titlebar right-click menu's "Customize" rows are +built entirely out of live `srd set`s. That was survivable while reloads only +happened on `Mod4+Ctrl+r`; reload-on-write turned a rare surprise into a +reliable one, and a control that silently reverts is worse than no control. +Flagged independently by the AGS peer session while deciding whether to build +Settings controls against these values, which is the same conclusion from the +other side. + +Every setting changed live is now recorded (`WindowManager::live_settings`, +key -> raw JSON text) and replayed after each reload through the very same +`handle_set` that applied it, so a replayed setting cannot behave differently +from a real one. Recorded only on success, so a rejected value is never +replayed; last write wins per key. Verified live: set `button_side left` and +`button_mode fixed`, saved an unrelated config edit, both survived and the +log reported "re-applied 2 live setting(s) after the reload". + +A live value is still a session override rather than a persisted setting -- +it lasts until changed again or the session ends. That distinction is now +documented in `DEFAULTS.md` instead of being a trap. + +Full workspace build/test/clippy clean: 515 tests (262 core / 160 wayland / +46 platform / 34 config / 13 ctl), 0 failed, 0 clippy warnings. ## Nemo's right-click menu: confirmed working, and two real bugs found doing it (2026-08-28) |