From 23a7b36d0d20ef1f809db0b5d69f1416021afb91 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 16 Oct 2026 20:58:18 +0200 Subject: notes: write and commit failures are reported, not assumed A review of the branch found four ways the page could report success for work that never happened, all reachable in ordinary use. Writes: setText is asynchronous unless blockWrites, and its saved()/saveFailed() do not fire for every write (a second setText to the same path loses the signal, which stranded the callback), so a save was reported before it landed and a failed db.json or body write was never seen. Both views now block and carry the outcome, so writeDb and writeBody know the result when they return; a failed create therefore leaves no index entry. The create path also loads the body before the index, so a failed body write cannot leave an entry with no text. Commits: the sync chain swallowed every commit failure with '|| true', so a failing pre-commit hook or a signing failure pushed an unchanged HEAD and reported success. It now stages, commits only when something is staged, and lets 'set -e' fail the chain, with stderr folded in for the whole chain rather than just the push. Duplicate notes: the 'awaiting commit' state lived only in a 3 second timer, so a slower second click re-entered create() and appended a second note. It is now a page property that outlives the timer, and the button follows it. Preview: the shared Switch writes checked itself, dropping the binding, so the switch stopped following page.preview; resynced on change, the way StatusRow.qml does. Also validates a note's file name as digits before read, write or rm, and clears the index error once the index reads again. --- desktop/modules/notes/NotesPage.qml | 72 +++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 35 deletions(-) (limited to 'desktop/modules/notes/NotesPage.qml') diff --git a/desktop/modules/notes/NotesPage.qml b/desktop/modules/notes/NotesPage.qml index a3d11f6..88382fc 100644 --- a/desktop/modules/notes/NotesPage.qml +++ b/desktop/modules/notes/NotesPage.qml @@ -37,12 +37,19 @@ Item { property string bodyError: "" // A write or git command is in flight; the buttons stand down meanwhile. property bool busy: false + // "create" | "edit" | "delete" once that write has landed and the commit + // and push are still owed. It outlives any timer, so a click after a pause + // pushes the note that was already written instead of writing it again. + property string awaiting: "" + + // The shared Switch writes `checked` itself, which drops the binding + // above; StatusRow.qml documents the same and resyncs the same way. + onPreviewChanged: prevSwitch.checked = page.preview function open(note) { page.bodyError = ""; page.preview = note !== null; - confirm.stop(); - confirmDel.stop(); + page.awaiting = ""; titleField.text = note ? note.title : ""; // Clear before the assignment below: that assignment is what retargets // the body FileView, and its load may land synchronously, so clearing @@ -54,8 +61,7 @@ Item { } function toList() { - confirm.stop(); - confirmDel.stop(); + page.awaiting = ""; page.busy = false; page.editing = null; page.bodyError = ""; @@ -64,27 +70,27 @@ Item { function save() { if (page.busy) return; - const done = (ok, msg) => { - page.busy = false; - if (!ok) { page.owner.lastError = msg; return; } - // Git off: there is no second half, so this was the whole save. - if (!page.owner.gitEnabled) { page.toList(); return; } - confirm.start(); - }; - if (confirm.running) { - confirm.stop(); - const kind = page.editing ? "edit" : "create"; - const id = page.editing ? page.editing.id : page.owner.lastCreatedId; + // The note is already on disk and only the push is owed, so this click + // pushes rather than writing a second note. + if (page.awaiting !== "") { + const id = page.awaiting === "create" ? page.owner.lastCreatedId : page.editing.id; page.busy = true; - page.owner.commitPush(kind, id, (ok, msg) => { + page.owner.commitPush(page.awaiting, id, (ok, msg) => { page.busy = false; - if (!ok) page.owner.lastError = msg; + if (!ok) { page.owner.lastError = msg; return; } page.toList(); }); return; } page.owner.lastError = ""; page.busy = true; + const done = (ok, msg) => { + page.busy = false; + if (!ok) { page.owner.lastError = msg; return; } + // Git off: there is no second half, so this was the whole save. + if (!page.owner.gitEnabled) { page.toList(); return; } + page.awaiting = page.editing ? "edit" : "create"; + }; if (page.editing) page.owner.update(page.editing.id, titleField.text, bodyEdit.text, done); else page.owner.create(titleField.text, bodyEdit.text, done); } @@ -92,25 +98,23 @@ Item { function remove() { if (page.busy || page.editing === null) return; const id = page.editing.id; - const done = (ok, msg) => { - page.busy = false; - if (!ok) { page.owner.lastError = msg; return; } - if (!page.owner.gitEnabled) { page.toList(); return; } - confirmDel.start(); - }; - if (confirmDel.running) { - confirmDel.stop(); + if (page.awaiting === "delete") { page.busy = true; page.owner.commitPush("delete", id, (ok, msg) => { page.busy = false; - if (!ok) page.owner.lastError = msg; + if (!ok) { page.owner.lastError = msg; return; } page.toList(); }); return; } page.owner.lastError = ""; page.busy = true; - page.owner.remove(id, done); + page.owner.remove(id, (ok, msg) => { + page.busy = false; + if (!ok) { page.owner.lastError = msg; return; } + if (!page.owner.gitEnabled) { page.toList(); return; } + page.awaiting = "delete"; + }); } function updateList() { @@ -128,7 +132,7 @@ Item { FileView { id: bodyRead path: (page.view === "editor" && page.editing) - ? page.owner.notesDir + "/" + page.editing.file : "" + ? page.owner.bodyPath(page.editing.file) : "" printErrors: false onLoaded: { bodyEdit.text = text(); page.bodyError = ""; } onLoadFailed: { @@ -137,8 +141,6 @@ Item { } } - Timer { id: confirm; interval: 3000 } - Timer { id: confirmDel; interval: 3000 } Timer { id: updated; interval: 2000 } // --- list -------------------------------------------------------------- @@ -276,9 +278,9 @@ Item { Button { id: saveBtn anchors { top: parent.top; right: parent.right } - enabled: !page.busy || confirm.running - text: page.busy && !confirm.running ? "Saving…" - : confirm.running ? "Commit & push?" : "Save" + enabled: !page.busy + text: page.busy ? "Saving…" + : page.awaiting !== "" ? "Commit & push?" : "Save" onClicked: page.save() } @@ -287,8 +289,8 @@ Item { anchors { top: parent.top; right: saveBtn.left; rightMargin: 8 } visible: page.editing !== null danger: true - enabled: !page.busy || confirmDel.running - text: confirmDel.running ? "Confirm?" : "Delete" + enabled: !page.busy + text: page.awaiting === "delete" ? "Confirm?" : "Delete" onClicked: page.remove() } -- cgit v1.2.3