From 47553c22db6781415fcce5d32926477f8556fa91 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Sat, 8 Aug 2026 20:00:13 -0400 Subject: [PATCH] Fix Save As silently corrupting the file on BLE Save As wrote only the changed suffix into the new file, leaving everything before that offset as whatever the freshly allocated cluster happened to contain. Editing `print("Hello World!")` to add ` xxx` and saving as a new file produced 26 bytes of which only the last 7 were correct: 2e 20 20 20 20 20 20 20 20 20 20 10 00 70 44 7d e7 5c e7 | 20 78 78 78 22 29 0a `------------------ 19 bytes of junk -----------------' `--- " xxx")\n" ---' 19 is the length of `print("Hello World!`, the common prefix with the previous contents. Save As with no edits at all was worse: the offset then equals the whole document, so nothing was written and the file was junk end to end. `unchanged` tracks the leading run of bytes known to match the device and is used as the partial-write offset. saveFileContents() reset it when writing a different file, but tested `path !== workflow.currentFilename`, and saveFileAs() (workflow.js:467) assigns currentFilename *before* calling save. The two therefore always matched by then and the reset never fired, so the offset from the previously open file was applied to a brand-new one. Rather than reorder saveFileAs(), track which file the offset describes. `unchanged` is only meaningful for one path, so record it in `unchangedPath` and reset when they disagree. That enforces the actual invariant and also covers routes other than Save As that could leave the offset and the target file out of step. BLE only in practice: partialWrites is true just for BLEWorkflow, and the base class defaults it to false, so the web and USB workflows always write in full. Co-Authored-By: Claude Opus 5 --- js/script.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/js/script.js b/js/script.js index 64906cf..a88f072 100644 --- a/js/script.js +++ b/js/script.js @@ -37,7 +37,11 @@ workflows[CONNTYPE.Usb] = new USBWorkflow(); workflows[CONNTYPE.Web] = new WebWorkflow(); let workflow = null; +// Length of the leading run of bytes known to match what is on the device, used +// as the offset for partial writes. It is only meaningful for one particular +// file, so `unchangedPath` records which -- see saveFileContents(). let unchanged = 0; +let unchangedPath = null; let connectionPromise = null; let debugMessageAnsi = null; @@ -600,6 +604,7 @@ function loadEditorContents(content, path = null) { // can correctly skip a no-op reconfigure. editorLanguagePath = path; unchanged = editor.state.doc.length; + unchangedPath = path; //console.log("doc length", unchanged); } @@ -706,7 +711,14 @@ async function saveFileContents(path) { // If this is a different file, we write everything. The language // plugin is refreshed by setFilename below (it routes through // setEditorLanguageForPath), so no extra dispatch is needed here. - if (path !== workflow.currentFilename) { + // + // Compare against unchangedPath, not workflow.currentFilename: + // saveFileAs() assigns currentFilename *before* calling us, so by this + // point the two always match and the reset never fired. `unchanged` + // then still described the previously open file, and Save As did a + // partial write at that file's offset into a brand-new one, leaving + // everything before the offset as whatever the fresh cluster held. + if (path !== unchangedPath) { unchanged = 0; } let doc = editor.state.doc; @@ -724,7 +736,9 @@ async function saveFileContents(path) { } // Optimistically mark the bytes-being-sent as unchanged. If the // write throws we'll roll back to baseUnchanged for the next try. + // Either way the offset now describes `path`, so record that. unchanged = docLengthAtStart; + unchangedPath = path; try { if (await workflow.writeFile(path, contents, offset)) { setFilename(workflow.currentFilename);