config_writer: emit pretty multi-line sections, drop json5 crate

Previously when append_kvp created a new section or added a key, it
stuffed the "\n    " separator into the new kvp's wsc.0 (the whitespace
between its own key and colon) instead of the prior kvp's wsc.3 (the
whitespace after the prior trailing comma). Result looked like:

    lsp_servers: [...],
    learn

        : {generate_alternates
            : true,},}

The writer also didn't set any interior whitespace on the new section's
JSONObjectContext, so everything crammed onto one line — `{key: val,}`
compact, not `{\n    key: val,\n}` multi-line.

Rewrote the appender as append_kvp_pretty(object, key, value,
inner_indent, outer_indent):
- separator between kvps goes in the prior kvp's wsc.3, or if we're the
  first kvp in a fresh object, in the object's own wsc.0 (after its
  opening `{`)
- new kvp's wsc.3 carries `,\n<outer_indent>` so the parent's closing
  `}` lands correctly indented
- interior indent vs outer indent are both explicit, so we don't have
  to rewrite this logic every time we add another nesting level

New tests: new_section_exact_multiline_layout asserts byte-exact
output shape; new_section_and_key_format_cleanly verifies no key wraps
to the next line. Prior tests just substring-matched and happily passed
on the broken output — that's why this shipped in the first place.

Also: dropped the json5 crate dependency. json-five's serde feature
(default) provides the same from_str / to_string API. One fewer
dependency, and the two were doing the same job.

Co-Authored-By: Proof of Concept <poc@bcachefs.org>

Cargo.lock

@@ -493,7 +493,6 @@ dependencies = [
  "hyper",
  "hyper-util",
  "json-five",
- "json5",
  "libc",
  "log",
  "memchr",

Cargo.toml

@@ -29,7 +29,6 @@ log = "0.4"
 
 serde = { version = "1", features = ["derive"] }
 serde_json = "1"
-json5 = "1.3"
 json-five = "0.3"
 
 ratatui = { version = "0.30", features = ["unstable-rendered-line-info"] }

src/config.rs

@@ -175,7 +175,7 @@ impl Config {
     /// API settings resolved from models + backend configuration.
     fn try_load_shared() -> Option<Self> {
         let content = std::fs::read_to_string(config_path()).ok()?;
-        let root: serde_json::Value = json5::from_str(&content).ok()?;
+        let root: serde_json::Value = json_five::from_str(&content).ok()?;
         let mem_value = root.get("memory")?;
 
         let mut config: Config = serde_json::from_value(mem_value.clone()).ok()?;
@@ -545,7 +545,7 @@ impl Provider for Json5File {
     fn data(&self) -> figment::Result<figment::value::Map<figment::Profile, figment::value::Dict>> {
         match std::fs::read_to_string(&self.0) {
             Ok(content) => {
-                let value: figment::value::Value = json5::from_str(&content)
+                let value: figment::value::Value = json_five::from_str(&content)
                     .map_err(|e| figment::Error::from(format!("{}: {}", self.0.display(), e)))?;
                 Serialized::defaults(value).data()
             }

src/config_writer.rs

@@ -52,46 +52,94 @@ fn key_matches(key: &JSONValue, name: &str) -> bool {
 
 /// Find (or create) a child object under `parent`, returning a mutable borrow
 /// of its key_value_pairs vector.
-fn get_or_create_object<'a>(
+/// Append a new kvp to `object`, setting whitespace so the output is
-    parent: &'a mut JSONValue,
+/// multi-line with the given indentation:
-    section: &str,
+///
-) -> Result<&'a mut Vec<JSONKeyValuePair>> {
+/// ```text
-    let pairs = match parent {
+/// {<newline><inner_indent>first_key: first_val,<newline><outer_indent>}
-        JSONValue::JSONObject { key_value_pairs, .. } => key_value_pairs,
+/// ```
-        _ => return Err(anyhow!("config root is not an object")),
+///
+/// If `object` already has kvps, the separator between the last one and
+/// ours goes in the prior kvp's wsc.3. If we're the first kvp, the
+/// lead-in after `{` goes in the object's own wsc.0.
+fn append_kvp_pretty(
+    object: &mut JSONValue,
+    key: JSONValue,
+    value: JSONValue,
+    inner_indent: &str,
+    outer_indent: &str,
+) -> Result<()> {
+    let (pairs, ctx) = match object {
+        JSONValue::JSONObject { key_value_pairs, context } => {
+            let ctx = context.get_or_insert_with(|| JSONObjectContext {
+                wsc: (String::new(),),
+            });
+            (key_value_pairs, ctx)
+        }
+        _ => return Err(anyhow!("not an object")),
     };
 
-    // Separate the lookup from the mutable borrow we return — needed to
+    if pairs.is_empty() {
-    // satisfy the borrow checker when we create a new entry.
+        ctx.wsc.0 = format!("\n{}", inner_indent);
-    let idx = pairs.iter().position(|kvp| key_matches(&kvp.key, section));
+    } else {
+        let prev = pairs.last_mut().unwrap();
+        let prev_ctx = prev.context.get_or_insert_with(|| KeyValuePairContext {
+            wsc: (String::new(), String::from(" "), String::new(), None),
+        });
+        prev_ctx.wsc.3 = Some(format!("\n{}", inner_indent));
+    }
 
-    let idx = match idx {
-        Some(i) => i,
-        None => {
     pairs.push(JSONKeyValuePair {
-                key: JSONValue::Identifier(section.to_string()),
+        key,
-                value: JSONValue::JSONObject {
+        value,
-                    key_value_pairs: Vec::new(),
-                    context: Some(JSONObjectContext {
-                        wsc: (String::new(),),
-                    }),
-                },
         context: Some(KeyValuePairContext {
             wsc: (
-                        String::from("\n\n    "), // whitespace before ':'
+                String::new(),
-                        String::from(" "),         // whitespace after ':'
+                String::from(" "),
-                        String::new(),             // whitespace after value
+                String::new(),
-                        Some(String::new()),       // whitespace after trailing comma
+                Some(format!("\n{}", outer_indent)),
             ),
         }),
     });
-            pairs.len() - 1
+
+    Ok(())
+}
+
+/// Find or create a child object under `parent`. Returns the index of
+/// the kvp in parent's key_value_pairs so the caller can re-borrow
+/// afterward.
+fn get_or_create_object_idx(
+    parent: &mut JSONValue,
+    section: &str,
+    inner_indent: &str,
+    outer_indent: &str,
+) -> Result<usize> {
+    let existing = match parent {
+        JSONValue::JSONObject { key_value_pairs, .. } => {
+            key_value_pairs.iter()
+                .position(|kvp| key_matches(&kvp.key, section))
         }
+        _ => return Err(anyhow!("config root is not an object")),
     };
 
-    match &mut pairs[idx].value {
+    if let Some(i) = existing {
-        JSONValue::JSONObject { key_value_pairs, .. } => Ok(key_value_pairs),
+        return Ok(i);
-        _ => Err(anyhow!("config key '{}' is not an object", section)),
+    }
+
+    append_kvp_pretty(
+        parent,
+        JSONValue::Identifier(section.to_string()),
+        JSONValue::JSONObject {
+            key_value_pairs: Vec::new(),
+            context: Some(JSONObjectContext { wsc: (String::new(),) }),
+        },
+        inner_indent,
+        outer_indent,
+    )?;
+
+    match parent {
+        JSONValue::JSONObject { key_value_pairs, .. } => Ok(key_value_pairs.len() - 1),
+        _ => unreachable!(),
     }
 }
 
@@ -100,26 +148,36 @@ fn get_or_create_object<'a>(
 pub fn set_scalar(section: &str, key: &str, literal: &str) -> Result<()> {
     let value = parse_scalar_literal(literal)?;
     edit_config(|root| {
-        let pairs = get_or_create_object(root, section)?;
+        // New top-level sections sit at column 4 (inside root `{`),
+        // and the root's closing `}` sits at column 0.
+        let section_idx = get_or_create_object_idx(root, section, "    ", "")?;
 
-        if let Some(kvp) = pairs.iter_mut().find(|k| key_matches(&k.key, key)) {
+        let section_value = match root {
+            JSONValue::JSONObject { key_value_pairs, .. } => {
+                &mut key_value_pairs[section_idx].value
+            }
+            _ => unreachable!(),
+        };
+
+        // Update in place if the key already exists.
+        if let JSONValue::JSONObject { key_value_pairs, .. } = section_value {
+            if let Some(kvp) = key_value_pairs.iter_mut()
+                .find(|k| key_matches(&k.key, key))
+            {
                 kvp.value = value;
                 return Ok(());
             }
+        }
 
-        pairs.push(JSONKeyValuePair {
+        // Append a new kvp. Inner keys sit at column 8, the section's
-            key: JSONValue::Identifier(key.to_string()),
+        // closing `}` sits at column 4.
+        append_kvp_pretty(
+            section_value,
+            JSONValue::Identifier(key.to_string()),
             value,
-            context: Some(KeyValuePairContext {
+            "        ",
-                wsc: (
+            "    ",
-                    String::from("\n        "),
+        )
-                    String::from(" "),
-                    String::new(),
-                    Some(String::new()),
-                ),
-            }),
-        });
-        Ok(())
     })
 }
 
@@ -166,24 +224,28 @@ mod tests {
         literal: &str,
     ) -> Result<()> {
         let value = parse_scalar_literal(literal)?;
-        let pairs = get_or_create_object(root, section)?;
+        let section_idx = get_or_create_object_idx(root, section, "    ", "")?;
-        if let Some(kvp) = pairs.iter_mut().find(|k| key_matches(&k.key, key)) {
+        let section_value = match root {
+            JSONValue::JSONObject { key_value_pairs, .. } => {
+                &mut key_value_pairs[section_idx].value
+            }
+            _ => unreachable!(),
+        };
+        if let JSONValue::JSONObject { key_value_pairs, .. } = section_value {
+            if let Some(kvp) = key_value_pairs.iter_mut()
+                .find(|k| key_matches(&k.key, key))
+            {
                 kvp.value = value;
                 return Ok(());
             }
-        pairs.push(JSONKeyValuePair {
+        }
-            key: JSONValue::Identifier(key.to_string()),
+        append_kvp_pretty(
+            section_value,
+            JSONValue::Identifier(key.to_string()),
             value,
-            context: Some(KeyValuePairContext {
+            "        ",
-                wsc: (
+            "    ",
-                    String::from("\n        "),
+        )
-                    String::from(" "),
-                    String::new(),
-                    Some(String::new()),
-                ),
-            }),
-        });
-        Ok(())
     }
 
     fn edit_str<F: FnOnce(&mut JSONValue) -> Result<()>>(src: &str, f: F) -> Result<String> {
@@ -302,7 +364,7 @@ mod tests {
         assert!(out.contains("1e-7"));
 
         // Parse result should parse back without error (real json5 parser).
-        let reparsed: serde_json::Value = json5::from_str(&out)
+        let reparsed: serde_json::Value = json_five::from_str(&out)
             .expect("mutated output must be valid JSON5");
         let threshold = reparsed.pointer("/learn/threshold").expect("learn.threshold exists");
         assert_eq!(threshold.as_f64(), Some(1e-7));
@@ -324,10 +386,55 @@ mod tests {
         assert!(!out.contains("0.001"));
         assert!(out.contains("// The divergence threshold"));
 
-        let reparsed: serde_json::Value = json5::from_str(&out).unwrap();
+        let reparsed: serde_json::Value = json_five::from_str(&out).unwrap();
         assert_eq!(reparsed.pointer("/learn/threshold").and_then(|v| v.as_f64()), Some(5e-8));
     }
 
+    #[test]
+    fn new_section_exact_multiline_layout() {
+        let src = "{\n    a: 1,\n}";
+        let out = edit_str(src, |root| {
+            set_scalar_inline(root, "learn", "generate_alternates", "true")?;
+            set_scalar_inline(root, "learn", "threshold", "1e-7")
+        }).unwrap();
+
+        let expected = "\
+{
+    a: 1,
+    learn: {
+        generate_alternates: true,
+        threshold: 1e-7,
+    },
+}";
+        assert_eq!(out, expected, "\n--- got ---\n{}\n--- want ---\n{}\n", out, expected);
+    }
+
+    #[test]
+    fn new_section_and_key_format_cleanly() {
+        // The kind of config we actually have in ~/.consciousness
+        // (top-level sections separated by blank lines, 4-space indent
+        // for keys within each section). Appending a fresh `learn`
+        // section with one key should land cleanly, not as
+        // `learn\n\n    :{key\n        :value}`.
+        let src = "{\n    memory: {\n        user_name: \"Kent\",\n    },\n}";
+        let out = edit_str(src, |root| {
+            set_scalar_inline(root, "learn", "generate_alternates", "true")
+        }).unwrap();
+
+        // No stray key-to-colon-on-next-line anywhere.
+        assert!(!out.contains("learn\n"), "learn key wraps: {}", out);
+        assert!(!out.contains("generate_alternates\n"),
+            "inner key wraps: {}", out);
+
+        // The output should reparse.
+        let v: serde_json::Value = json_five::from_str(&out).unwrap();
+        assert_eq!(
+            v.pointer("/learn/generate_alternates").and_then(|x| x.as_bool()),
+            Some(true),
+            "output: {}", out,
+        );
+    }
+
     #[test]
     fn roundtrip_stable_without_change() {
         let src = r#"{