From 65df18d8dbbc9fdb435807693096fce8f7038b91 Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:49:50 +0300 Subject: [PATCH] settings: a blanked config.toml is no longer adopted over the loaded document MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A syntactically broken file keeps the loaded document and is moved aside, but a blank file parses fine — so check_disk adopted the empty document before its blankness test ran, and the next save wrote only the known values: comments and unknown sections loaded at startup were deleted silently, with no error and no backup (blank text is never written as a backup either, so on a file's first save the content was unrecoverable). The crate itself treats blank as untrustworthy for the backup (recovery.rs: 'a blank or unparsable file must never replace the previous backup') and the docs promise comments and unknown keys survive saves. Blank text is now rejected up front, before it can replace the document. Regression test: an_emptied_file_is_not_adopted_over_the_loaded_document — fails today (comments and unknown sections missing after the second save), passes with the early return; the existing backup test keeps passing. --- crates/settings/src/store.rs | 7 +++++++ crates/settings/tests/recovery.rs | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/crates/settings/src/store.rs b/crates/settings/src/store.rs index 50369459..43ae721e 100644 --- a/crates/settings/src/store.rs +++ b/crates/settings/src/store.rs @@ -385,6 +385,13 @@ impl SettingsStore { Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(None), Err(e) => return Err(e.into()), }; + // A blank file still parses, so adopting it below would replace this store's document -- + // and with it every comment and unknown key the app does not manage -- with an empty one; + // blank text is never written as a backup either, so the content would be lost. A blank + // file is not an edit to adopt: keep what this store loaded and leave the file alone. + if text.trim().is_empty() { + return Ok(None); + } let Ok(mut doc) = document::parse(&text) else { self.moved_aside.push(recovery::move_aside(path)?); return Ok(None); diff --git a/crates/settings/tests/recovery.rs b/crates/settings/tests/recovery.rs index 4af90bb4..5d719285 100644 --- a/crates/settings/tests/recovery.rs +++ b/crates/settings/tests/recovery.rs @@ -178,3 +178,22 @@ fn outside_file_with_failing_migration_is_not_overwritten() { assert_eq!(read_config(tmp.path()), external, "the outside file is left untouched"); assert!(matches!(store.tick(std::time::Instant::now()).status, craft_settings::SaveStatus::Idle), "no retries once read-only"); } + +#[test] +fn an_emptied_file_is_not_adopted_over_the_loaded_document() { + let tmp = tempfile::tempdir().unwrap(); + write_config(tmp.path(), "schema_version = 2\n\n# my note\n[plugins]\nenabled = true\n\n[general]\nlanguage = \"de\"\n"); + let (mut store, _) = open_in(tmp.path()); + store.update(|p| p.general.language = "fr".into()); + store.flush().unwrap(); + + // Emptied while the app runs: an editor crash, a sync tool, a hand `truncate`. + write_config(tmp.path(), ""); + store.update(|p| p.general.language = "es".into()); + store.flush().unwrap(); + + let text = read_config(tmp.path()); + assert!(text.contains("# my note"), "{text}"); + assert!(text.contains("[plugins]"), "{text}"); + assert!(text.contains("language = \"es\""), "{text}"); +}