From 82f305cea69daaefae6b3adf0273deed71513208 Mon Sep 17 00:00:00 2001
From: Julien Calixte <julien.calixte@theodo.com>
Date: Sun, 12 Jul 2026 01:11:28 +0200
Subject: [PATCH] feat(persistence): end saved files with a trailing newline

Add the POSIX line terminator on save and strip it on load, so files
written by the editor no longer trip git's "No newline at end of file".
Done at the persistence choke point, not in :fmt: the editor buffer is
newline-free by design (rows = #\n + 1), so a trailing '\n' in the buffer
would render a phantom blank last line. save_path adds exactly one '\n'
(guarded against doubling); load_path strips one back off so the buffer
stays newline-free and round-trips byte-stable.

Update the sd_fat spike payload to be newline-free so its byte-identity
round-trip assertion holds under the new normalizing contract.
---
 firmware/src/bin/sd_fat.rs  |  5 ++++-
 firmware/src/persistence.rs | 25 ++++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/firmware/src/bin/sd_fat.rs b/firmware/src/bin/sd_fat.rs
index ea2c2aa..84971da 100644
--- a/firmware/src/bin/sd_fat.rs
+++ b/firmware/src/bin/sd_fat.rs
@@ -94,7 +94,10 @@ fn write_test(storage: &Storage) -> Result<()> {
         log::info!("{REPO_DIR} missing — creating it (bench setup) so the write test can run");
         fs::create_dir_all(REPO_DIR).with_context(|| format!("create {REPO_DIR}"))?;
     }
-    let payload = format!("typoena spike 3\n{BUILD_TAG}\ndedicated SPI3: SCK14 MOSI15 MISO13 CS10\n");
+    // Newline-free, matching a real editor buffer: `save`/`load` normalize the
+    // trailing terminator (add on write, strip on read), so a payload that ended
+    // in '\n' would read back one byte shorter. This still round-trips identically.
+    let payload = format!("typoena spike 3\n{BUILD_TAG}\ndedicated SPI3: SCK14 MOSI15 MISO13 CS10");
     storage.save(&payload).context("Storage::save")?;
     let back = storage.load().context("Storage::load after save")?;
     if back != payload {
diff --git a/firmware/src/persistence.rs b/firmware/src/persistence.rs
index 5e42bc2..a05036a 100644
--- a/firmware/src/persistence.rs
+++ b/firmware/src/persistence.rs
@@ -269,7 +269,21 @@ impl Storage {
                 m.len() / 1024,
                 MAX_FILE_BYTES / 1024
             ),
-            Ok(_) => fs::read_to_string(path).with_context(|| format!("reading {path}")),
+            Ok(_) => {
+                let mut s =
+                    fs::read_to_string(path).with_context(|| format!("reading {path}"))?;
+                // Drop the single POSIX line terminator that `save_path` adds so the
+                // editor buffer stays newline-free (it counts rows as `#\n + 1`, so a
+                // trailing '\n' would render a phantom blank last line). Also handles a
+                // file authored elsewhere: it loads without that phantom line.
+                if s.ends_with('\n') {
+                    s.pop();
+                    if s.ends_with('\r') {
+                        s.pop(); // tolerate a CRLF terminator
+                    }
+                }
+                Ok(s)
+            }
             Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(String::new()),
             Err(e) => Err(e).with_context(|| format!("stat {path}")),
         }
@@ -294,6 +308,15 @@ impl Storage {
                 .with_context(|| format!("create {tmp} (does its directory exist?)"))?;
             f.write_all(contents.as_bytes())
                 .with_context(|| format!("write {tmp}"))?;
+            // End the file with exactly one newline (POSIX text convention; keeps git
+            // from flagging "No newline at end of file"). The editor buffer is
+            // newline-free by design, so this is the single place the terminator is
+            // added; `load_path` strips it back off on the way in. Guarded so an
+            // already-terminated buffer (e.g. an unformatted external file) isn't
+            // doubled.
+            if !contents.ends_with('\n') {
+                f.write_all(b"\n").with_context(|| format!("write final newline to {tmp}"))?;
+            }
             // FatFS f_sync — flush the tmp fully before it can replace the target.
             f.sync_all().with_context(|| format!("fsync {tmp}"))?;
         }