fix(config): say what is wrong with a config file, and where

Three different mistakes, one answer:

    { "shell": "/nonexistent/shell" }  ->  NOT VALID JSON
    { "font_size": 12,, }              ->  NOT VALID JSON
    { "font_size": "big" }             ->  NOT VALID JSON

Two of those *are* valid JSON. They are valid JSON in the wrong shape — a
string where a struct goes, a string where a number goes — which is a
different mistake with a different fix, and telling that reader their file is
not valid JSON sends them hunting for a missing comma that is not missing.

serde has already worked out the answer. It names the field, the type it
wanted, and the line and column. That went to `log::warn!` and nowhere else,
and there is no log unless `TTY7_LOG` is set, so on a default install it went
nowhere at all — which is the same shape as the keybinding faults and the
clamped settings this tree has already fixed.

Now:

    DOES NOT FIT — invalid type: string "big", expected f32 at line 1 column 20
    NOT VALID JSON — key must be a string at line 1 column 19
    NOT VALID JSON — EOF while parsing an object at line 1 column 17

`parse_fault` is a helper rather than a field on `LoadOutcome`, which is
`Copy` and crosses several call sites that only want the verdict; this is
asked once, by a diagnostic, about a file already on disk. The window's
notice appends the same detail on its own line — deliberately after the
translated sentence rather than inside it, because serde's message is English
that is not ours to translate and a placeholder would leave a raw parser
string in the middle of a localized one. The notice's own wording needed no
change: it says "could not be parsed", which was true of all three.

CHANGELOG.md

@@ -153,6 +153,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **A config file that will not load now says what is wrong and where.**
+  `doctor` reported "NOT VALID JSON" for three different mistakes, and two of
+  them were valid JSON: `"font_size": "big"`, or a string where an object goes,
+  is well-formed JSON that does not fit the shape — a reader told their file
+  is not valid JSON goes hunting for a missing comma that is not missing.
+  serde already answers this, naming the field, the type it wanted and the line
+  and column, and that answer went only to a `log::warn!` — which is nowhere,
+  since there is no log unless `TTY7_LOG` is set. The two are now told apart,
+  and both carry the detail: `DOES NOT FIT — invalid type: string "big",
+  expected f32 at line 1 column 20`. The window's notice appends it too, on its
+  own line after the translated sentence.
+
 - **A workspace or tab name is stored as one line.** `tty7 tab rename @1
   $'one\ntwo'` kept the newline verbatim, and a name is only ever drawn as a
   label — the layout breaks a label on a newline whatever its wrapping says,

crates/tty7-cli/src/commands.rs

@@ -1776,11 +1776,27 @@ fn doctor(ctx: &Context, backend: &mut dyn Backend) -> Result<Outcome> {
     let (config_state, config_ok) = match loaded.1 {
         tty7_core::core::config::LoadOutcome::Parsed => ("ok".to_string(), true),
         tty7_core::core::config::LoadOutcome::Absent => ("none yet — the defaults are the config".to_string(), true),
-        tty7_core::core::config::LoadOutcome::Quarantined => (
-            "NOT VALID JSON — kept aside as config.json.corrupt; running on defaults and not saving"
-                .to_string(),
-            false,
-        ),
+        tty7_core::core::config::LoadOutcome::Quarantined => {
+            // What failed and where, not just that something did. serde
+            // already names the field, the type it wanted and the line and
+            // column; that went to a log nobody has switched on. And a file
+            // that is valid JSON in the wrong shape — `"font_size": "big"`, a
+            // string where an object goes — is a different mistake from a
+            // missing comma, so calling both "not valid JSON" sent half the
+            // readers hunting for punctuation that was not wrong.
+            let fault = tty7_core::core::config::parse_fault();
+            let what = match &fault {
+                Some(f) if f.malformed => format!("NOT VALID JSON — {}", f.detail),
+                Some(f) => format!("DOES NOT FIT — {}", f.detail),
+                None => "NOT USABLE".to_string(),
+            };
+            (
+                format!(
+                    "{what}; kept aside as config.json.corrupt, running on defaults and not saving"
+                ),
+                false,
+            )
+        }
         tty7_core::core::config::LoadOutcome::Unreadable => (
             "UNREADABLE — running on defaults and not saving"
                 .to_string(),

crates/tty7-core/src/core/config.rs

@@ -917,7 +917,7 @@ impl Config {
         }
     }
 
-    fn path() -> Option<PathBuf> {
+    pub(crate) fn path() -> Option<PathBuf> {
         config_path("config.json")
     }
 }
@@ -970,6 +970,47 @@ pub fn config_path(file: &str) -> Option<PathBuf> {
     Some(config_dir()?.join(file))
 }
 
+/// Why the config file did not parse, in serde's own words.
+///
+/// [`LoadOutcome::Quarantined`] says only *that* it failed, and that is all
+/// most callers need. `doctor` needs the rest, because the reader is standing
+/// in front of a file they have to fix and the answer is already known: serde
+/// names the field, the type it wanted, and the line and column.
+///
+/// Until now that went to `log::warn!` and nowhere else — and there is no log
+/// unless `TTY7_LOG` is set, so in practice it went nowhere.
+///
+/// Read again rather than carried on the outcome: the outcome is `Copy` and
+/// crosses several call sites that only want the verdict, and this is asked
+/// once, by a diagnostic command, about a file that is already on disk.
+pub fn parse_fault() -> Option<ParseFault> {
+    let path = Config::path()?;
+    let text = std::fs::read_to_string(&path).ok()?;
+    let error = serde_json::from_str::<Config>(strip_bom(&text)).err()?;
+    Some(ParseFault {
+        malformed: matches!(
+            error.classify(),
+            serde_json::error::Category::Syntax | serde_json::error::Category::Eof
+        ),
+        detail: error.to_string(),
+    })
+}
+
+/// What [`parse_fault`] found.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct ParseFault {
+    /// True when the bytes are not JSON at all — a missing comma, an unclosed
+    /// brace. False when they *are* JSON and simply do not fit the shape:
+    /// `"font_size": "big"`, or a string where an object goes. Those are
+    /// different mistakes and they send the reader to different places, so
+    /// calling both "not valid JSON" sends half of them hunting for a comma
+    /// that is not missing.
+    pub malformed: bool,
+    /// serde's message, which already carries the field, the expected type,
+    /// and `at line N column M`.
+    pub detail: String,
+}
+
 pub fn strip_bom(text: &str) -> &str {
     text.strip_prefix('\u{FEFF}').unwrap_or(text)
 }
@@ -2508,6 +2549,52 @@ mod tests {
         assert!(cfg.keybindings.is_empty());
     }
 
+    /// A config that will not load says what is wrong with it and where.
+    ///
+    /// Two mistakes, and they send the reader to different places. A missing
+    /// comma is not JSON. `"font_size": "big"`, or a string where an object
+    /// goes, *is* JSON — it simply does not fit — and telling that reader
+    /// their file is "not valid JSON" sends them hunting for punctuation that
+    /// is not wrong. `doctor` said exactly that for both, and serde's answer,
+    /// which names the field, the type it wanted and the line and column, went
+    /// to a `log::warn!` nobody has switched on.
+    #[test]
+    fn a_config_that_will_not_load_says_what_and_where() {
+        let _guard = lock_config_file();
+        pin_config_dir();
+        let path = Config::path().expect("a pinned config dir");
+
+        std::fs::write(&path, br#"{ "font_size": 12,, }"#).unwrap();
+        let fault = parse_fault().expect("a file that does not parse has a fault");
+        assert!(fault.malformed, "a stray comma is not JSON: {fault:?}");
+        assert!(
+            fault.detail.contains("line 1 column"),
+            "the reader needs the place: {fault:?}"
+        );
+
+        std::fs::write(&path, br#"{ "font_size": "big" }"#).unwrap();
+        let fault = parse_fault().expect("a shape mismatch is a fault too");
+        assert!(
+            !fault.malformed,
+            "this is valid JSON that does not fit, not broken JSON: {fault:?}"
+        );
+        assert!(
+            fault.detail.contains("f32") && fault.detail.contains("line 1 column"),
+            "the reader needs the type and the place: {fault:?}"
+        );
+
+        std::fs::write(&path, br#"{ "font_size": 12"#).unwrap();
+        assert!(
+            parse_fault().is_some_and(|f| f.malformed),
+            "a truncated file is not JSON either"
+        );
+
+        std::fs::write(&path, br#"{ "font_size": 12 }"#).unwrap();
+        assert_eq!(parse_fault(), None, "a file that loads has no fault");
+
+        let _ = std::fs::remove_file(&path);
+    }
+
     fn pin_config_dir() {
         let dir = std::env::temp_dir().join(format!("tty7-covtest-{}", std::process::id()));
         std::fs::create_dir_all(&dir).ok();

src/main.rs

@@ -212,8 +212,22 @@ fn notify_config_load_failed(
     let Some(handle) = crate::ui::windows::WindowRegistry::window_for(cx, workspace) else {
         return;
     };
+    // The sentence says what happened; serde says what to fix. It already
+    // names the field, the type it wanted, and the line and column, and until
+    // now it went only to `log::warn!` — which is to say nowhere, since there
+    // is no log unless `TTY7_LOG` is set. "Fix it and tty7 reloads" is not
+    // much help to somebody who cannot see which line is wrong.
+    //
+    // Appended rather than folded into the translated string: it is serde's
+    // English and it is not ours to translate, and a placeholder would put a
+    // raw parser message in the middle of a localized sentence. On its own
+    // line after it, it reads as what it is — the parser's own words.
+    let body = match crate::core::config::parse_fault() {
+        Some(fault) => format!("{}\n\n{}", crate::ui::i18n::t(key), fault.detail),
+        None => crate::ui::i18n::t(key).to_string(),
+    };
     let _ = handle.update(cx, |_, window, cx| {
-        window.push_notification(crate::ui::i18n::t(key), cx);
+        window.push_notification(body, cx);
     });
 }