settings: a blanked config.toml is no longer adopted over the loaded document

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.
This commit is contained in:
midasmatterhorn committed 2026-10-15 17:49:50 +03:00
1 parent 8f5a283161
commit 65df18d8db
2 files changed
+26

No files matched your search

+7
View File
@@ -385,6 +385,13 @@ impl<T: Settings> SettingsStore<T> {
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);
+19
View File
@@ -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}");
}