fix(config): say "your config is broken" once, not once per pane spawn

`Config::load` runs on every pane spawn and every palette command, and the
parse-failure arm logged a warning each time. One stray comma in config.json
therefore produced a log made of the same line: measured at 23 copies after
opening three tabs and sending one command, growing for as long as the session
lasts and burying every other warning in the file.

The answer was already there. `quarantine` refuses to keep a second copy of
contents it has kept before — that check is what stops a broken config filling
the directory with identical `.corrupt` files — so it already knows whether a
breakage is newly seen. It now says so, and the parse message is a warning the
first time and a debug line afterwards.

Nothing else moves: the file is still kept aside, `save` still refuses to run
against a quarantined config, and the user's file is untouched.

Verified against a running instance with a deliberately malformed config.json:
the same four operations that produced 23 warnings now produce 1, there is
still exactly one `.corrupt` file, and config.json is byte-identical to what
was written before the app started.

2949 tests pass.
This commit is contained in:
l0ng-ai
2026-08-15 18:29:17 +08:00
parent d76ed6df50
commit bfa58af587
+23 -4
View File
@@ -679,11 +679,23 @@ impl Config {
// file held the moment anything — a dragged sidebar divider —
// writes. Park a copy first, the way `WindowViews::load`
// does, and mark the stand-in so `save` refuses to run for it.
log::warn!(
// Said once per breakage, not once per read. This arm runs on
// every pane spawn and every palette command, so a single
// stray comma otherwise fills the log with one repeated line
// and buries everything else in it — measured at 23 copies
// after four CLI operations. `quarantine` already knows
// whether it has kept these bytes before; the first sighting
// is the one worth raising a voice about.
let first_sighting = quarantine(&path);
log::log!(
if first_sighting {
log::Level::Warn
} else {
log::Level::Debug
},
"failed to parse config at {}: {e}; keeping it aside and using defaults",
path.display()
);
quarantine(&path);
let cfg = Config {
quarantined: true,
..Config::default()
@@ -845,7 +857,13 @@ pub fn strip_bom(text: &str) -> &str {
/// Sets a corrupt state file aside (copied, the original left in place) so the
/// caller can fall back to defaults without silently destroying what was there.
pub(crate) fn quarantine(path: &std::path::Path) {
/// Keeps a copy of a file that could not be parsed, and answers whether this
/// breakage is one it had not already kept.
///
/// The answer is worth having because the same broken file is read again and
/// again, and a caller that logs every time turns one stray comma into a log
/// full of the same line.
pub(crate) fn quarantine(path: &std::path::Path) -> bool {
// A broken file is read again and again — `Config::load` alone runs on
// every pane spawn and every palette command — so this is reached over
// and over for the same contents. A sibling already holding those bytes
@@ -855,13 +873,14 @@ pub(crate) fn quarantine(path: &std::path::Path) {
if let Ok(bytes) = std::fs::read(path)
&& already_kept(path, &bytes)
{
return;
return false;
}
let aside = quarantine_path(path);
match std::fs::copy(path, &aside) {
Ok(_) => log::warn!("the previous contents were kept at {}", aside.display()),
Err(e) => log::warn!("could not keep a copy at {}: {e}", aside.display()),
}
true
}
/// Whether an earlier quarantine of `path` already holds exactly `bytes`.