diff --git a/src/config/mod.rs b/src/config/mod.rs index 4cc3266..e87a7c8 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -41,10 +41,38 @@ pub fn load_doc(path: &Path) -> DocumentMut { /// Write the document back to disk, creating parent dirs as needed. pub fn save_doc(path: &Path, doc: &DocumentMut) -> Result<(), Box> { + atomic_write(path, &doc.to_string())?; + Ok(()) +} + +/// Write `contents` to `path` atomically, backing up whatever was there +/// before overwriting it. +/// +/// Every config-writing view in this app (TOML via `save_doc` above, and the +/// plain-JSON views — keybinds, autostart, appearance/settings.json, +/// monitors.json, breadbar's CSS) should go through this instead of a bare +/// `std::fs::write`: writing straight to the target path means a crash, +/// power loss, or disk-full error mid-write can leave the file truncated or +/// corrupted with no way back. Writing to a temp file in the *same* +/// directory first, then `rename`-ing it over the target, avoids that — a +/// rename within one filesystem is atomic, so the target either has the old +/// complete contents or the new complete contents, never a partial write. +/// Backing up the previous file first (best-effort — the write can still +/// proceed if the backup fails, e.g. read-only source) means even a +/// successful-but-wrong write is always recoverable from `.bak`. +pub fn atomic_write(path: &Path, contents: &str) -> std::io::Result<()> { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?; } - std::fs::write(path, doc.to_string())?; + if path.exists() { + let backup = PathBuf::from(format!("{}.bak", path.display())); + let _ = std::fs::copy(path, &backup); + } + let dir = path.parent().map(Path::to_path_buf).unwrap_or_else(|| PathBuf::from(".")); + let file_name = path.file_name().and_then(|n| n.to_str()).unwrap_or("config"); + let tmp_path = dir.join(format!(".{file_name}.tmp.{}", std::process::id())); + std::fs::write(&tmp_path, contents)?; + std::fs::rename(&tmp_path, path)?; Ok(()) } @@ -210,4 +238,30 @@ password = \"secret\" # keep me set_str_list(&mut doc, &["modules", "disable"], &items); assert_eq!(get_str_list(&doc, &["modules", "disable"]), items); } + + #[test] + fn atomic_write_backs_up_previous_contents_and_no_tmp_file_left_behind() { + let dir = std::env::temp_dir().join(format!("bos-settings-atomic-write-test-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("config.toml"); + let backup = dir.join("config.toml.bak"); + + atomic_write(&path, "first").unwrap(); + assert_eq!(std::fs::read_to_string(&path).unwrap(), "first"); + assert!(!backup.exists(), "no backup should be made when there's nothing to back up yet"); + + atomic_write(&path, "second").unwrap(); + assert_eq!(std::fs::read_to_string(&path).unwrap(), "second"); + assert_eq!(std::fs::read_to_string(&backup).unwrap(), "first"); + + let leftover_tmp: Vec<_> = std::fs::read_dir(&dir) + .unwrap() + .filter_map(|e| e.ok()) + .map(|e| e.file_name().to_string_lossy().into_owned()) + .filter(|n| n.contains(".tmp.")) + .collect(); + assert!(leftover_tmp.is_empty(), "temp file should be renamed away, not left behind: {leftover_tmp:?}"); + + let _ = std::fs::remove_dir_all(&dir); + } } diff --git a/src/ui/views/appearance.rs b/src/ui/views/appearance.rs index 6f9092f..419ac94 100644 --- a/src/ui/views/appearance.rs +++ b/src/ui/views/appearance.rs @@ -81,10 +81,7 @@ fn load() -> Appearance { fn save(a: &Appearance) -> std::io::Result<()> { let path = config_path(); - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent)?; - } - std::fs::write(path, serde_json::to_string_pretty(a).unwrap_or_default()) + crate::config::atomic_write(&path, &serde_json::to_string_pretty(a).unwrap_or_default()) } /// "rgba(RRGGBBAA)" (Hyprland's format) <-> gdk::RGBA, so the color fields diff --git a/src/ui/views/autostart.rs b/src/ui/views/autostart.rs index ce58ae3..fe2870c 100644 --- a/src/ui/views/autostart.rs +++ b/src/ui/views/autostart.rs @@ -56,11 +56,8 @@ fn load() -> Vec { fn save(entries: &[Entry_]) -> std::io::Result<()> { let path = config_path(); - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent)?; - } let file = AutostartFile { extra: entries.to_vec() }; - std::fs::write(path, serde_json::to_string_pretty(&file).unwrap_or_default()) + crate::config::atomic_write(&path, &serde_json::to_string_pretty(&file).unwrap_or_default()) } fn rebuild(list: &ListBox, model: &Rc>>) { diff --git a/src/ui/views/breadbar.rs b/src/ui/views/breadbar.rs index 1f49c43..3caaeea 100644 --- a/src/ui/views/breadbar.rs +++ b/src/ui/views/breadbar.rs @@ -57,10 +57,7 @@ pub fn build() -> GBox { save_btn.connect_clicked(move |_| { let (start, end) = buf.bounds(); let text = buf.text(&start, &end, false); - if let Some(parent) = path.parent() { - let _ = std::fs::create_dir_all(parent); - } - match std::fs::write(&path, text.as_str()) { + match crate::config::atomic_write(&path, text.as_str()) { Ok(()) => { // breadbar has no systemd unit (it's launched directly by // hyprland.lua's exec-once) — SIGHUP is its own documented diff --git a/src/ui/views/hyprland.rs b/src/ui/views/hyprland.rs index c37e19c..e0936d5 100644 --- a/src/ui/views/hyprland.rs +++ b/src/ui/views/hyprland.rs @@ -64,11 +64,8 @@ fn load() -> Vec { fn save(rules: &[MonitorRule]) -> std::io::Result<()> { let path = config_path(); - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent)?; - } let file = MonitorsFile { monitors: rules.to_vec() }; - std::fs::write(path, serde_json::to_string_pretty(&file).unwrap_or_default()) + crate::config::atomic_write(&path, &serde_json::to_string_pretty(&file).unwrap_or_default()) } fn get_live_monitors() -> Vec<(String, String)> { diff --git a/src/ui/views/keybinds.rs b/src/ui/views/keybinds.rs index 0ee9def..ab5a0dc 100644 --- a/src/ui/views/keybinds.rs +++ b/src/ui/views/keybinds.rs @@ -1,20 +1,41 @@ //! hypr/binds.json — Hyprland keybind editor, read by -//! `scripts/ui/binds.lua` on the Hyprland side (see hyprland.lua). The -//! schema has four kinds of bind lists (`globals`, `common`, and one per -//! keyboard `layouts` entry) and each bind's shape varies by `action` -//! (`exec` needs `command`, `move_dir` needs `direction`, workspace-focus -//! needs `workspace`, mouse binds need `options.mouse`, ...). Rather than -//! modelling every action's field set as its own row layout — which would -//! mean a combinatorial explosion of widgets and silently dropping any -//! action shape this editor doesn't already know about — `action`/`key`/ -//! `mods` get real fields (the ones every bind has) and everything else -//! round-trips through `#[serde(flatten)]` into a small inline-JSON column, -//! same trade-off the other Hyprland JSON editors (appearance.rs, -//! hyprland.rs, autostart.rs) already make: no comments to preserve, so this -//! is a whole-file round trip, not the `toml_edit`/`Doc` path-based pattern. +//! `scripts/ui/binds.lua` on the Hyprland side (see hyprland.lua). +//! +//! This file has TWO real on-disk shapes, and which one applies depends on +//! the machine: +//! +//! - **Flat** (`default_mods` + a single `bindings` array) — what BOS itself +//! ships (`iso/airootfs/etc/skel/.config/hypr/binds.json`, read by the +//! BOS-shipped `scripts/input/binds.lua`). No layouts. Each bind carries +//! `label`/`category`/`demo_cmd` fields breadhelp depends on for its +//! cheatsheet and guided tour. +//! - **MultiLayout** (`globals`/`common`/one `layouts` entry per keyboard +//! layout) — a personal, per-machine schema some dev setups use instead, +//! read by a different, personal `binds.lua`. +//! +//! This editor was originally built only against the MultiLayout shape. +//! Loading a real BOS (Flat) file into that model, then saving, silently +//! dropped the `bindings` key entirely — still valid JSON, so the Lua +//! `pcall` failsafes on the reading side never caught it. `SchemaKind` +//! detects which shape is actually on disk (from the top-level key set) and +//! `save()` always emits that SAME shape back — see `SchemaKind::detect` +//! and `save_to`. +//! +//! Each bind's shape also varies by `action` (`exec` needs `command`, +//! `move_dir` needs `direction`, workspace-focus needs `workspace`, mouse +//! binds need `options.mouse`, ...). Rather than modelling every action's +//! field set as its own row layout — which would mean a combinatorial +//! explosion of widgets and silently dropping any action shape this editor +//! doesn't already know about — `action`/`key`/`mods` get real fields (the +//! ones every bind has) and everything else round-trips through +//! `#[serde(flatten)]` into a small inline-JSON column, same trade-off the +//! other Hyprland JSON editors (appearance.rs, hyprland.rs, autostart.rs) +//! already make: no comments to preserve, so this is a whole-file round +//! trip, not the `toml_edit`/`Doc` path-based pattern. use std::cell::RefCell; use std::collections::BTreeMap; +use std::path::Path; use std::rc::Rc; use gtk4::prelude::*; @@ -27,17 +48,65 @@ use serde_json::{Map, Value}; use crate::ui::widgets as w; +/// Which on-disk shape `binds.json` was loaded as. Detected once at load +/// time from the top-level key set present in the JSON, then pinned for the +/// lifetime of the editor session so `save()` always writes back the same +/// shape it read, regardless of what the in-memory model happens to have +/// populated. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +enum SchemaKind { + /// `{ "default_mods": [...], "bindings": [...] }` — BOS's real shipped + /// shape. No layout-switching UI applies; there's nothing to switch. + Flat, + /// `{ "active_layout", "default_mods", "globals", "common", "layouts" }` + /// — the personal, multi-keyboard-layout schema this editor was + /// originally built against. + MultiLayout, + /// Neither key set matched — an empty file, a totally different shape, + /// or unparsable JSON. Loading still renders (empty), but `save()` + /// refuses outright rather than guessing a shape and risking silently + /// destroying whatever the real file's actual schema was. + Unknown, +} + +impl SchemaKind { + fn detect(top_level: &Map) -> Self { + if top_level.contains_key("bindings") { + SchemaKind::Flat + } else if top_level.contains_key("globals") + || top_level.contains_key("common") + || top_level.contains_key("layouts") + { + SchemaKind::MultiLayout + } else { + SchemaKind::Unknown + } + } +} + #[derive(Clone, Serialize, Deserialize, Default)] #[serde(default)] struct Bind { action: String, #[serde(skip_serializing_if = "Option::is_none")] key: Option, - #[serde(skip_serializing_if = "Vec::is_empty")] - mods: Vec, + /// `None` (key omitted) means "fall back to `default_mods`"; `Some(_)` + /// — including `Some(vec![])` — means "use exactly this, even if that's + /// no modifiers at all." Real BOS binds rely on that distinction (e.g. + /// media keys pin `"mods": []` on purpose so they never inherit + /// `default_mods`), so this can't collapse both cases to "omit the + /// key" the way a bare `Vec` with `skip_serializing_if` would — + /// that would silently turn an explicit "no mods" into "use the + /// default" the next time this editor saves the file. + #[serde(skip_serializing_if = "Option::is_none")] + mods: Option>, /// Everything else a bind can carry — `command`, `direction`, - /// `workspace`, `x`, `y`, `layout`, `options`, and any action shape not - /// yet invented. Edited as compact inline JSON (see `extra_field`). + /// `workspace`, `x`, `y`, `layout`, `options`, `label`, `category`, + /// `demo_cmd`, and any action shape not yet invented. Edited as compact + /// inline JSON (see `extra_field`). This flatten is what keeps + /// breadhelp's `label`/`category`/`demo_cmd` fields — which this + /// editor's UI has no dedicated widgets for — alive across a full + /// load/save round trip instead of being silently dropped. #[serde(flatten)] extra: Map, } @@ -45,6 +114,7 @@ struct Bind { #[derive(Serialize, Deserialize, Default)] #[serde(default)] struct BindsFile { + #[serde(skip_serializing_if = "String::is_empty")] active_layout: String, #[serde(skip_serializing_if = "Vec::is_empty")] default_mods: Vec, @@ -57,22 +127,83 @@ struct BindsFile { /// the rest of this file's JSON-config siblings. #[serde(skip_serializing_if = "BTreeMap::is_empty")] layouts: BTreeMap>, + /// Flat-schema bind list — BOS's real shipped shape. Only ever populated + /// when `SchemaKind::Flat` was detected at load time; stays empty (and + /// so omitted, see `to_json`) for a MultiLayout file. + #[serde(skip_serializing_if = "Vec::is_empty")] + bindings: Vec, +} + +/// The editor's full in-memory state: which shape was loaded, plus the data +/// itself. Kept together so a stray code path can't accidentally serialize +/// `file` without knowing which shape it's supposed to come back out as. +struct Model { + kind: SchemaKind, + file: BindsFile, } fn config_path() -> std::path::PathBuf { crate::config::config_dir().join("hypr/binds.json") } -fn load() -> BindsFile { - std::fs::read_to_string(config_path()).ok().and_then(|s| serde_json::from_str(&s).ok()).unwrap_or_default() +fn load_from(path: &Path) -> (BindsFile, SchemaKind) { + let Ok(text) = std::fs::read_to_string(path) else { + // No file yet (fresh install/environment) — nothing on disk to + // misdetect or destroy. BOS itself ships the flat schema, so a new + // file defaults to Flat rather than the personal MultiLayout schema + // this editor originally assumed. + return (BindsFile::default(), SchemaKind::Flat); + }; + let kind = match serde_json::from_str::(&text) { + Ok(Value::Object(top_level)) => SchemaKind::detect(&top_level), + // Unparsable JSON, or valid JSON that isn't even an object — treat + // as Unknown so save() refuses rather than silently overwriting + // whatever this file actually was with an empty default. + _ => SchemaKind::Unknown, + }; + let file: BindsFile = serde_json::from_str(&text).unwrap_or_default(); + (file, kind) } -fn save(f: &BindsFile) -> std::io::Result<()> { - let path = config_path(); +fn load() -> (BindsFile, SchemaKind) { + load_from(&config_path()) +} + +/// Serialize `f` in exactly the shape `kind` implies: +/// - `Flat` -> `{ "default_mods": [...], "bindings": [...] }`, nothing else +/// — no `active_layout`/`globals`/`common`/`layouts` keys, even if the +/// struct happens to carry empty values for them. +/// - `MultiLayout` -> today's existing shape (whatever fields are +/// non-empty), via `BindsFile`'s own `Serialize` impl. +fn to_json(f: &BindsFile, kind: SchemaKind) -> Value { + match kind { + SchemaKind::Flat => serde_json::json!({ + "default_mods": f.default_mods, + "bindings": f.bindings, + }), + SchemaKind::MultiLayout => serde_json::to_value(f).unwrap_or(Value::Null), + SchemaKind::Unknown => Value::Null, + } +} + +fn save_to(path: &Path, f: &BindsFile, kind: SchemaKind) -> std::io::Result<()> { + if kind == SchemaKind::Unknown { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "binds.json's schema wasn't recognized (expected a \"bindings\" key, or one of \ + \"globals\"/\"common\"/\"layouts\") — refusing to save so nothing gets silently \ + overwritten. Fix or remove the file, then reopen this panel.", + )); + } if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?; } - std::fs::write(path, serde_json::to_string_pretty(f).unwrap_or_default()) + let text = serde_json::to_string_pretty(&to_json(f, kind)).unwrap_or_default(); + crate::config::atomic_write(path, &text) +} + +fn save(f: &BindsFile, kind: SchemaKind) -> std::io::Result<()> { + save_to(&config_path(), f, kind) } fn mods_to_text(mods: &[String]) -> String { @@ -84,22 +215,26 @@ fn text_to_mods(s: &str) -> Vec { } /// A `Vec` accessor that always finds the right list regardless of -/// whether it's `globals`, `common`, or a named entry in `layouts` — lets one -/// row-builder work for every section instead of three near-duplicates. -type SectionAccessor = Rc &mut Vec>; +/// whether it's `globals`, `common`, a named entry in `layouts`, or the flat +/// `bindings` list — lets one row-builder work for every section instead of +/// near-duplicates per schema. +type SectionAccessor = Rc &mut Vec>; fn globals_accessor() -> SectionAccessor { - Rc::new(|f| &mut f.globals) + Rc::new(|m| &mut m.file.globals) } fn common_accessor() -> SectionAccessor { - Rc::new(|f| &mut f.common) + Rc::new(|m| &mut m.file.common) } fn layout_accessor(name: String) -> SectionAccessor { - Rc::new(move |f| f.layouts.entry(name.clone()).or_default()) + Rc::new(move |m| m.file.layouts.entry(name.clone()).or_default()) +} +fn bindings_accessor() -> SectionAccessor { + Rc::new(|m| &mut m.file.bindings) } fn bind_row( - model: &Rc>, + model: &Rc>, accessor: &SectionAccessor, idx: usize, rerender: &Rc, @@ -116,7 +251,7 @@ fn bind_row( let mut m = model.borrow_mut(); let bind = &accessor(&mut m)[idx]; ( - mods_to_text(&bind.mods), + mods_to_text(bind.mods.as_deref().unwrap_or(&[])), bind.key.clone().unwrap_or_default(), bind.action.clone(), if bind.extra.is_empty() { String::new() } else { serde_json::to_string(&bind.extra).unwrap_or_default() }, @@ -152,7 +287,11 @@ fn bind_row( mods.connect_changed(move |e| { let mut m = model.borrow_mut(); if let Some(b) = accessor(&mut m).get_mut(idx) { - b.mods = text_to_mods(&e.text()); + // Explicitly setting this field (even to an empty string, + // which `text_to_mods` turns into `vec![]`) always records + // `Some(_)` — "use exactly these mods" — never falls back + // to inferring "key omitted" from an empty result. + b.mods = Some(text_to_mods(&e.text())); } }); } @@ -219,10 +358,10 @@ fn bind_row( } /// A section = a title, an "Add bind" button, and the section's bind rows — -/// shared by Globals, Common, and every named layout. +/// shared by Globals, Common, every named layout, and the flat Bindings list. fn section( title: Option<&str>, - model: &Rc>, + model: &Rc>, accessor: SectionAccessor, rerender: &Rc, ) -> GBox { @@ -256,21 +395,51 @@ fn section( wrapper } -fn rerender(content: &GBox, model: &Rc>, status: &Label) { +fn rerender(content: &GBox, model: &Rc>, status: &Label) { while let Some(child) = content.first_child() { content.remove(&child); } populate(content, model, status); } -fn populate(content: &GBox, model: &Rc>, status: &Label) { - let rerender: Rc = { - let content = content.clone(); - let model = model.clone(); - let status = status.clone(); - Rc::new(move || rerender(&content, &model, &status)) - }; +fn populate_unknown(content: &GBox, status: &Label) { + content.append(&w::hint( + "binds.json's schema wasn't recognized (expected a \"bindings\" key, or one of \ + \"globals\"/\"common\"/\"layouts\"). Nothing below is editable, and Save is disabled, \ + so the file on disk isn't at risk of being silently overwritten with the wrong shape. \ + Fix or remove the file by hand, then reopen this panel.", + )); + status.set_text("binds.json schema not recognized — editing disabled"); +} +fn populate_flat(content: &GBox, model: &Rc>, status: &Label, rerender: &Rc) { + content.append(&w::hint( + "Mods/Key/Action are the fields every bind needs. The last column holds action-specific \ + extras as inline JSON — e.g. {\"command\": \"kitty\"}, {\"direction\": \"left\"}, \ + {\"workspace\": \"e+1\"}, {\"label\": \"...\", \"category\": \"...\"} — leave it blank \ + for actions with none (close, exit, fullscreen, ...). This machine's binds.json uses \ + BOS's flat schema (no keyboard-layout switching), so that's all there is. Applies on \ + next login/reload.", + )); + + let default_mods = Entry::new(); + default_mods.set_text(&mods_to_text(&model.borrow().file.default_mods)); + default_mods.set_hexpand(true); + default_mods.set_width_chars(20); + { + let model = model.clone(); + default_mods.connect_changed(move |e| { + model.borrow_mut().file.default_mods = text_to_mods(&e.text()); + }); + } + content.append(&w::row("Default mods", &default_mods)); + + content.append(§ion(Some("Bindings"), model, bindings_accessor(), rerender)); + + let _ = status; +} + +fn populate_multi_layout(content: &GBox, model: &Rc>, rerender: &Rc) { content.append(&w::hint( "Mods/Key/Action are the fields every bind needs. The last column \ holds action-specific extras as inline JSON — e.g. \ @@ -280,7 +449,7 @@ fn populate(content: &GBox, model: &Rc>, status: &Label) { Applies on next login/reload.", )); - let layout_names: Vec = model.borrow().layouts.keys().cloned().collect(); + let layout_names: Vec = model.borrow().file.layouts.keys().cloned().collect(); let top_row = GBox::new(Orientation::Horizontal, 12); top_row.append(&{ @@ -288,13 +457,13 @@ fn populate(content: &GBox, model: &Rc>, status: &Label) { Some(StringList::new(&layout_names.iter().map(String::as_str).collect::>())), Expression::NONE, ); - let cur = model.borrow().active_layout.clone(); + let cur = model.borrow().file.active_layout.clone(); dd.set_selected(layout_names.iter().position(|n| *n == cur).unwrap_or(0) as u32); let model = model.clone(); let layout_names = layout_names.clone(); dd.connect_selected_notify(move |dd| { if let Some(name) = layout_names.get(dd.selected() as usize) { - model.borrow_mut().active_layout = name.clone(); + model.borrow_mut().file.active_layout = name.clone(); } }); w::row("Active layout", &dd) @@ -302,19 +471,19 @@ fn populate(content: &GBox, model: &Rc>, status: &Label) { content.append(&top_row); let default_mods = Entry::new(); - default_mods.set_text(&mods_to_text(&model.borrow().default_mods)); + default_mods.set_text(&mods_to_text(&model.borrow().file.default_mods)); default_mods.set_hexpand(true); default_mods.set_width_chars(20); { let model = model.clone(); default_mods.connect_changed(move |e| { - model.borrow_mut().default_mods = text_to_mods(&e.text()); + model.borrow_mut().file.default_mods = text_to_mods(&e.text()); }); } content.append(&w::row("Default mods", &default_mods)); - content.append(§ion(Some("Media & function keys (globals)"), model, globals_accessor(), &rerender)); - content.append(§ion(Some("Common (every layout)"), model, common_accessor(), &rerender)); + content.append(§ion(Some("Media & function keys (globals)"), model, globals_accessor(), rerender)); + content.append(§ion(Some("Common (every layout)"), model, common_accessor(), rerender)); for name in &layout_names { let header = GBox::new(Orientation::Horizontal, 8); @@ -345,9 +514,9 @@ fn populate(content: &GBox, model: &Rc>, status: &Label) { dialog.choose(window.as_ref(), gtk4::gio::Cancellable::NONE, move |result| { if result == Ok(1) { let mut m = model.borrow_mut(); - m.layouts.remove(&name); - if m.active_layout == name { - m.active_layout = m.layouts.keys().next().cloned().unwrap_or_default(); + m.file.layouts.remove(&name); + if m.file.active_layout == name { + m.file.active_layout = m.file.layouts.keys().next().cloned().unwrap_or_default(); } drop(m); rerender(); @@ -360,7 +529,7 @@ fn populate(content: &GBox, model: &Rc>, status: &Label) { // No section title here — the "Layout: X" header above (with its // own Remove button) already covers it. - content.append(§ion(None, model, layout_accessor(name.clone()), &rerender)); + content.append(§ion(None, model, layout_accessor(name.clone()), rerender)); } let add_layout_row = GBox::new(Orientation::Horizontal, 8); @@ -377,7 +546,7 @@ fn populate(content: &GBox, model: &Rc>, status: &Label) { if name.is_empty() { return; } - model.borrow_mut().layouts.entry(name).or_default(); + model.borrow_mut().file.layouts.entry(name).or_default(); entry.set_text(""); rerender(); }); @@ -387,9 +556,25 @@ fn populate(content: &GBox, model: &Rc>, status: &Label) { content.append(&add_layout_row); } +fn populate(content: &GBox, model: &Rc>, status: &Label) { + let rerender: Rc = { + let content = content.clone(); + let model = model.clone(); + let status = status.clone(); + Rc::new(move || rerender(&content, &model, &status)) + }; + + match model.borrow().kind { + SchemaKind::Unknown => populate_unknown(content, status), + SchemaKind::Flat => populate_flat(content, model, status, &rerender), + SchemaKind::MultiLayout => populate_multi_layout(content, model, &rerender), + } +} + pub fn build() -> GBox { let (outer, content) = w::view_scaffold("Keybinds"); - let model = Rc::new(RefCell::new(load())); + let (file, kind) = load(); + let model = Rc::new(RefCell::new(Model { kind, file })); let status = Label::new(None); status.add_css_class("dim-label"); @@ -403,16 +588,19 @@ pub fn build() -> GBox { { let model = model.clone(); let status = status.clone(); - save_btn.connect_clicked(move |_| match save(&model.borrow()) { - Ok(()) => { - status.set_text("Saved"); - let lbl = status.clone(); - glib::timeout_add_seconds_local(3, move || { - lbl.set_text(""); - glib::ControlFlow::Break - }); + save_btn.connect_clicked(move |_| { + let m = model.borrow(); + match save(&m.file, m.kind) { + Ok(()) => { + status.set_text("Saved"); + let lbl = status.clone(); + glib::timeout_add_seconds_local(3, move || { + lbl.set_text(""); + glib::ControlFlow::Break + }); + } + Err(e) => status.set_text(&format!("Error: {e}")), } - Err(e) => status.set_text(&format!("Error: {e}")), }); } btn_row.append(&save_btn); @@ -421,3 +609,144 @@ pub fn build() -> GBox { outer } + +#[cfg(test)] +mod tests { + use super::*; + + /// A representative slice of BOS's real shipped `binds.json` + /// (`iso/airootfs/etc/skel/.config/hypr/binds.json`, flat schema) — + /// chosen to exercise the extra-field variety breadhelp reads (`label`, + /// `category`, `demo_cmd`), an explicit `mods: []` override, a nested + /// `options` object, and both integer and string `workspace` values. + /// This is the fixture that would have caught the original bug: the + /// editor mis-detecting this shape as MultiLayout and silently dropping + /// the whole `bindings` array on save. + const REAL_BOS_FLAT_FIXTURE: &str = r#"{ + "default_mods": ["SUPER"], + "bindings": [ + { "action": "exec", "command": "kitty", "key": "RETURN", "label": "Open a terminal", "category": "apps" }, + { "action": "close", "key": "BACKSPACE", "label": "Close the focused window", "category": "windows" }, + { "action": "exec", "command": "breadbox", "key": "SPACE", "label": "Open the app launcher (breadbox)", "category": "apps", "demo_cmd": "breadbox" }, + { "action": "exec", "command": "wpctl set-volume -l 1 @DEFAULT_AUDIO_SINK@ 5%+", "key": "XF86AudioRaiseVolume", "mods": [], "options": { "locked": true, "repeating": true }, "label": "Volume up", "category": "media" }, + { "action": "focus", "workspace": 1, "key": "1", "label": "Switch to workspace 1", "category": "workspaces" }, + { "action": "focus", "workspace": "e+1", "key": "bracketright", "label": "Next workspace", "category": "workspaces" }, + { "action": "resize_dir", "x": 30, "y": 0, "key": "right", "mods": ["SUPER", "SHIFT"], "options": { "repeating": true }, "label": "Resize the focused window (grow right)", "category": "focus" }, + { "action": "drag", "key": "mouse:272", "options": { "mouse": true }, "label": "Move a window (drag)", "category": "mouse" } + ] +}"#; + + fn parse(text: &str) -> (BindsFile, SchemaKind) { + let kind = match serde_json::from_str::(text) { + Ok(Value::Object(top)) => SchemaKind::detect(&top), + _ => SchemaKind::Unknown, + }; + let file: BindsFile = serde_json::from_str(text).unwrap_or_default(); + (file, kind) + } + + #[test] + fn detects_flat_schema_from_real_bos_binds_json() { + let (_, kind) = parse(REAL_BOS_FLAT_FIXTURE); + assert_eq!(kind, SchemaKind::Flat); + } + + #[test] + fn round_trips_real_bos_flat_binds_json_through_load_and_save() { + let (file, kind) = parse(REAL_BOS_FLAT_FIXTURE); + assert_eq!(kind, SchemaKind::Flat); + + let original: Value = serde_json::from_str(REAL_BOS_FLAT_FIXTURE).unwrap(); + let saved = to_json(&file, kind); + + // Flat save must emit EXACTLY {default_mods, bindings} — no + // active_layout/globals/common/layouts keys leaking in. + let saved_obj = saved.as_object().expect("flat save must be a JSON object"); + assert_eq!( + saved_obj.keys().cloned().collect::>(), + ["default_mods", "bindings"].into_iter().map(String::from).collect(), + "Flat schema must round-trip as exactly {{default_mods, bindings}}" + ); + + // The `bindings` array — and every per-bind extra field (label, + // category, demo_cmd, mods, options, integer vs string workspace, + // ...) — must survive the round trip semantically untouched. + assert_eq!(saved["bindings"], original["bindings"]); + assert_eq!(saved["default_mods"], original["default_mods"]); + } + + #[test] + fn round_trip_via_files_preserves_bindings_key_and_extras() { + let dir = std::env::temp_dir().join(format!("bos-settings-keybinds-test-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("binds.json"); + std::fs::write(&path, REAL_BOS_FLAT_FIXTURE).unwrap(); + + let (file, kind) = load_from(&path); + assert_eq!(kind, SchemaKind::Flat); + save_to(&path, &file, kind).unwrap(); + + let saved_text = std::fs::read_to_string(&path).unwrap(); + let saved: Value = serde_json::from_str(&saved_text).unwrap(); + let original: Value = serde_json::from_str(REAL_BOS_FLAT_FIXTURE).unwrap(); + + assert!(saved.get("bindings").is_some(), "bindings key must survive a load -> save round trip"); + assert_eq!(saved["bindings"], original["bindings"]); + assert_eq!(saved["default_mods"], original["default_mods"]); + + // Backup safety net: a second save must leave `.bak` holding the + // prior contents. + save_to(&path, &file, kind).unwrap(); + let backup_path = dir.join("binds.json.bak"); + assert!(backup_path.exists(), "save must back up the previous file"); + let backup: Value = serde_json::from_str(&std::fs::read_to_string(&backup_path).unwrap()).unwrap(); + assert_eq!(backup["bindings"], original["bindings"]); + + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn detects_and_round_trips_multi_layout_schema() { + let text = r#"{ + "active_layout": "qwerty", + "default_mods": ["SUPER"], + "globals": [{ "action": "exec", "command": "kitty", "key": "RETURN" }], + "common": [], + "layouts": { "qwerty": [{ "action": "close", "key": "BACKSPACE" }] } + }"#; + let (file, kind) = parse(text); + assert_eq!(kind, SchemaKind::MultiLayout); + + let saved = to_json(&file, kind); + assert!(saved.get("bindings").is_none(), "MultiLayout save must not emit a flat `bindings` key"); + assert_eq!(saved["active_layout"], "qwerty"); + assert_eq!(saved["layouts"]["qwerty"][0]["action"], "close"); + assert_eq!(saved["globals"][0]["command"], "kitty"); + } + + #[test] + fn unknown_schema_is_detected_and_refuses_to_save() { + let text = r#"{ "some_other_shape": true }"#; + let (file, kind) = parse(text); + assert_eq!(kind, SchemaKind::Unknown); + + let dir = std::env::temp_dir().join(format!("bos-settings-keybinds-unknown-test-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("binds.json"); + + let result = save_to(&path, &file, kind); + assert!(result.is_err(), "save() must refuse when schema kind is Unknown"); + assert!(!path.exists(), "refusing to save must not create/touch the target file"); + + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn missing_file_defaults_to_flat_not_multi_layout() { + let dir = std::env::temp_dir().join(format!("bos-settings-keybinds-missing-test-{}", std::process::id())); + // Don't create the file at all. + let path = dir.join("binds.json"); + let (_, kind) = load_from(&path); + assert_eq!(kind, SchemaKind::Flat); + } +}