Migrate config load/save to bread_utils::tomlcfg; timeout-guard hyprctl calls
config.rs's config_dir/load_doc/save_doc/atomic_write were a byte-for-byte copy of bos-settings/src/config/mod.rs's versions (own doc comment said as much) introduced in the same fix pass that added them there — now both delegate to bread_utils::tomlcfg/bread_utils::atomic instead of each carrying their own copy (path dependency for now, see the TODO in Cargo.toml). Also: services/hyprland.rs::query_json used a bare Command::output() with no timeout, so an unresponsive hyprctl (Hyprland wedged/reloading) could block clients()/monitors()/layers() indefinitely — switched to bread_utils::proc::run_json with a 3s cap. Builds clean; all 8 existing tests pass.
This commit is contained in:
parent
df7f51281a
commit
7b7fe680fe
4 changed files with 33 additions and 59 deletions
11
Cargo.lock
generated
11
Cargo.lock
generated
|
|
@ -37,12 +37,23 @@ dependencies = [
|
||||||
"serde_json",
|
"serde_json",
|
||||||
]
|
]
|
||||||
|
|
||||||
|
[[package]]
|
||||||
|
name = "bread-utils"
|
||||||
|
version = "0.2.3"
|
||||||
|
dependencies = [
|
||||||
|
"dirs",
|
||||||
|
"serde",
|
||||||
|
"serde_json",
|
||||||
|
"toml_edit 0.22.27",
|
||||||
|
]
|
||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "breadhelp"
|
name = "breadhelp"
|
||||||
version = "0.2.1"
|
version = "0.2.1"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"async-channel",
|
"async-channel",
|
||||||
"bread-theme",
|
"bread-theme",
|
||||||
|
"bread-utils",
|
||||||
"gdk4",
|
"gdk4",
|
||||||
"glib",
|
"glib",
|
||||||
"gtk4",
|
"gtk4",
|
||||||
|
|
|
||||||
|
|
@ -17,3 +17,5 @@ toml = "0.8"
|
||||||
# Non-destructive state editing (mirrors bos-settings/src/config/mod.rs).
|
# Non-destructive state editing (mirrors bos-settings/src/config/mod.rs).
|
||||||
toml_edit = "0.22"
|
toml_edit = "0.22"
|
||||||
async-channel = "2"
|
async-channel = "2"
|
||||||
|
# TODO(owner): switch to tag-pinned git dependency once bread-utils is merged and tagged, matching the bread-theme pattern
|
||||||
|
bread-utils = { path = "../bread-ecosystem-fix-worktree/bread-utils", features = ["toml"] }
|
||||||
|
|
|
||||||
|
|
@ -1,20 +1,16 @@
|
||||||
//! `~/.config/breadhelp/state.toml` — non-destructive TOML editing, same
|
//! `~/.config/breadhelp/state.toml` — non-destructive TOML editing via
|
||||||
//! `load_doc`/`save_doc` discipline as `bos-settings/src/config/mod.rs`: a
|
//! `bread_utils::tomlcfg`, the same `load_doc`/`save_doc` discipline
|
||||||
//! missing file yields defaults, a file that exists but fails to parse is
|
//! `bos-settings/src/config/mod.rs` uses: a missing file yields defaults, a
|
||||||
//! backed up once before falling back, so a bad edit is always recoverable.
|
//! file that exists but fails to parse is backed up once before falling
|
||||||
|
//! back, so a bad edit is always recoverable. This module used to carry its
|
||||||
|
//! own byte-for-byte copy of that logic (introduced in the same fix pass
|
||||||
|
//! that added it to bos-settings) — now both share one implementation.
|
||||||
|
|
||||||
use std::path::{Path, PathBuf};
|
use std::path::{Path, PathBuf};
|
||||||
use toml_edit::{value, DocumentMut};
|
use toml_edit::{value, DocumentMut};
|
||||||
|
|
||||||
pub fn config_dir() -> PathBuf {
|
pub fn config_dir() -> PathBuf {
|
||||||
if let Ok(xdg) = std::env::var("XDG_CONFIG_HOME") {
|
bread_utils::xdg::config_home()
|
||||||
let p = PathBuf::from(xdg);
|
|
||||||
if p.is_absolute() {
|
|
||||||
return p;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
let home = std::env::var("HOME").unwrap_or_else(|_| "/root".to_string());
|
|
||||||
PathBuf::from(home).join(".config")
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn state_path() -> PathBuf {
|
fn state_path() -> PathBuf {
|
||||||
|
|
@ -22,55 +18,15 @@ fn state_path() -> PathBuf {
|
||||||
}
|
}
|
||||||
|
|
||||||
fn load_doc(path: &Path) -> DocumentMut {
|
fn load_doc(path: &Path) -> DocumentMut {
|
||||||
let Ok(text) = std::fs::read_to_string(path) else {
|
bread_utils::tomlcfg::load_doc("breadhelp", path)
|
||||||
return DocumentMut::default();
|
|
||||||
};
|
|
||||||
match text.parse::<DocumentMut>() {
|
|
||||||
Ok(doc) => doc,
|
|
||||||
Err(e) => {
|
|
||||||
let backup = PathBuf::from(format!("{}.bak", path.display()));
|
|
||||||
eprintln!(
|
|
||||||
"breadhelp: {} failed to parse ({e}); backed up to {} before falling back to defaults",
|
|
||||||
path.display(),
|
|
||||||
backup.display()
|
|
||||||
);
|
|
||||||
let _ = std::fs::write(&backup, &text);
|
|
||||||
DocumentMut::default()
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn save_doc(path: &Path, doc: &DocumentMut) {
|
fn save_doc(path: &Path, doc: &DocumentMut) {
|
||||||
if let Err(e) = atomic_write(path, &doc.to_string()) {
|
if let Err(e) = bread_utils::tomlcfg::save_doc(path, doc) {
|
||||||
eprintln!("breadhelp: couldn't write {}: {e}", path.display());
|
eprintln!("breadhelp: couldn't write {}: {e}", path.display());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Write `contents` to `path` atomically, backing up whatever was there
|
|
||||||
/// before overwriting it — same discipline as `bos-settings/src/config/
|
|
||||||
/// mod.rs::atomic_write`. Writing straight to the target path means a
|
|
||||||
/// crash, power loss, or disk-full error mid-write can leave `state.toml`
|
|
||||||
/// 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. Backing up the
|
|
||||||
/// previous file first (best-effort) means even a successful-but-wrong
|
|
||||||
/// write is always recoverable from `<path>.bak`.
|
|
||||||
fn atomic_write(path: &Path, contents: &str) -> std::io::Result<()> {
|
|
||||||
if let Some(parent) = path.parent() {
|
|
||||||
std::fs::create_dir_all(parent)?;
|
|
||||||
}
|
|
||||||
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(())
|
|
||||||
}
|
|
||||||
|
|
||||||
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
|
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
|
||||||
pub enum Mode {
|
pub enum Mode {
|
||||||
Normal,
|
Normal,
|
||||||
|
|
@ -184,17 +140,21 @@ mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn atomic_write_backs_up_previous_contents_and_no_tmp_file_left_behind() {
|
fn state_toml_write_backs_up_previous_contents_and_no_tmp_file_left_behind() {
|
||||||
|
// Exercises the same bread_utils::atomic::write_atomic_backed_up
|
||||||
|
// path save_doc uses (via tomlcfg::save_doc), at the file-layout
|
||||||
|
// breadhelp actually writes to — the deeper backup/no-leftover-tmp
|
||||||
|
// behavior itself is covered by bread-utils' own test suite.
|
||||||
let dir = std::env::temp_dir().join(format!("breadhelp-atomic-write-test-{}", std::process::id()));
|
let dir = std::env::temp_dir().join(format!("breadhelp-atomic-write-test-{}", std::process::id()));
|
||||||
std::fs::create_dir_all(&dir).unwrap();
|
std::fs::create_dir_all(&dir).unwrap();
|
||||||
let path = dir.join("state.toml");
|
let path = dir.join("state.toml");
|
||||||
let backup = dir.join("state.toml.bak");
|
let backup = dir.join("state.toml.bak");
|
||||||
|
|
||||||
atomic_write(&path, "first").unwrap();
|
bread_utils::atomic::write_atomic_backed_up(&path, "first").unwrap();
|
||||||
assert_eq!(std::fs::read_to_string(&path).unwrap(), "first");
|
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");
|
assert!(!backup.exists(), "no backup should be made when there's nothing to back up yet");
|
||||||
|
|
||||||
atomic_write(&path, "second").unwrap();
|
bread_utils::atomic::write_atomic_backed_up(&path, "second").unwrap();
|
||||||
assert_eq!(std::fs::read_to_string(&path).unwrap(), "second");
|
assert_eq!(std::fs::read_to_string(&path).unwrap(), "second");
|
||||||
assert_eq!(std::fs::read_to_string(&backup).unwrap(), "first");
|
assert_eq!(std::fs::read_to_string(&backup).unwrap(), "first");
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -46,8 +46,9 @@ pub struct LayerClient {
|
||||||
}
|
}
|
||||||
|
|
||||||
fn query_json(cmd: &str) -> Option<serde_json::Value> {
|
fn query_json(cmd: &str) -> Option<serde_json::Value> {
|
||||||
let output = Command::new("hyprctl").args(["-j", cmd]).output().ok()?;
|
// Timeout-guarded: an unresponsive hyprctl (Hyprland wedged/reloading)
|
||||||
output.status.success().then(|| serde_json::from_slice(&output.stdout).ok()).flatten()
|
// used to be able to block clients()/monitors()/layers() indefinitely.
|
||||||
|
bread_utils::proc::run_json("hyprctl", &["-j", cmd], std::time::Duration::from_secs(3))
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn clients() -> Vec<Client> {
|
pub fn clients() -> Vec<Client> {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue