From 53a6051c1d74e48d9a065ce0765dc49a75c9d711 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 16 Oct 2026 20:46:50 +0200 Subject: notes: in-drawer editor with commit-on-save and git pull The editor, and the write and git path behind it. Save is two clicks and each does one thing: the first writes the note and only then arms the button as 'Commit & push?', so a quick second click cannot commit a note the first has not written yet. The write path refuses when db.json cannot be read as the index. That is not hypothetical: git pull --rebase --autostash resolves a remote db.json change against a local uncommitted one, the stash pop conflicts, git still exits 0, and the file is left full of merge markers. Reading that as 'no notes' and writing it back deleted every title in a probe; Notes.valid now distinguishes it from an empty store and the write reports instead. parse stays tolerant for display. Stamping lastpull also only follows a pull that worked, and only when git is on at all. Db reads that a write is based on go through reload() plus waitForJob(): blockLoading does not make reload() blocking, contrary to the note in AGENTS.md, and reading text() straight after reload() returns the old contents. A body is written with a plain setText, which needs no prior load; only reading text() before a load would. --- desktop/modules/notes/Notes.js | 23 +- desktop/modules/notes/NotesModule.qml | 211 +++++++++++- desktop/modules/notes/NotesPage.qml | 382 ++++++++++++++++++---- desktop/modules/notes/test_notes.js | 14 +- docs/superpowers/specs/2026-10-16-notes-design.md | 29 +- 5 files changed, 577 insertions(+), 82 deletions(-) diff --git a/desktop/modules/notes/Notes.js b/desktop/modules/notes/Notes.js index 76c8f95..6bc8c88 100644 --- a/desktop/modules/notes/Notes.js +++ b/desktop/modules/notes/Notes.js @@ -22,17 +22,28 @@ function notesOf(db) { return (db && Array.isArray(db.notes)) ? db.notes : []; } -// db.json, whole. Anything that is not an object carrying a notes array reads -// as an empty store, so a truncated or hand-edited file shows "no notes" -// rather than failing to load the page. -function parse(text) { +// The db object, or null when the text cannot be read as one. An empty or +// missing file is an empty store, not an error. Anything else that does not +// parse, or parses to something without a notes array, is null: a writer must +// refuse on null rather than replace a file it did not understand, which is +// what a conflicted db.json (merge markers from an autostash pop) is. +function valid(text) { + const t = String(text === undefined || text === null ? "" : text); + if (t.trim() === "") return { notes: [] }; let v = null; - try { v = JSON.parse(text); } catch (e) { v = null; } + try { v = JSON.parse(t); } catch (e) { v = null; } if (!v || typeof v !== "object" || Array.isArray(v) || !Array.isArray(v.notes)) - return { notes: [] }; + return null; return v; } +// db.json for display: the object, or an empty store when nothing readable is +// there, so a truncated or hand-edited file shows "no notes" rather than +// failing to load the page. +function parse(text) { + return valid(text) || { notes: [] }; +} + // Pretty, 2-space, trailing newline. notes.sh reads db.json through jq, which // does not care about the layout. function stringify(db) { diff --git a/desktop/modules/notes/NotesModule.qml b/desktop/modules/notes/NotesModule.qml index a2d54e5..94ef41e 100644 --- a/desktop/modules/notes/NotesModule.qml +++ b/desktop/modules/notes/NotesModule.qml @@ -9,6 +9,7 @@ // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // GNU General Public License for more details. +import Quickshell import Quickshell.Io import QtQuick import "../.." @@ -20,9 +21,13 @@ import "Notes.js" as Notes // seconds. The page writes notes in exactly the shape notes.sh reads, so both // tools work on the same notes. // +// Git follows notes.sh: pull before the write, then add, commit, push. The one +// departure is that a save here is two clicks, so an unconfirmed save leaves +// db.json uncommitted, and the pulls therefore carry --autostash. +// // A Module is a plain QtObject and has no default property, so the Process and -// the FileView below are declared as typed properties, as UpdatesModule does -// for its Process. +// FileView objects below are declared as typed properties, as UpdatesModule +// does for its Process. Module { id: mod @@ -42,10 +47,15 @@ Module { property string client: "" // The whole parsed db.json, so a write can replace .notes and carry - // params and git through untouched. + // params and git through untouched. Every read and write goes through this + // rather than through dbView.text(), which is stale right after a reload. property var db: ({ notes: [] }) property bool ready: false property string lastError: "" + property string gitOutput: "" + property bool syncing: false + // So the second click on a brand-new note can name it in the commit. + property string lastCreatedId: "" readonly property var notes: Notes.sorted(mod.db.notes) readonly property int count: Array.isArray(mod.db.notes) ? mod.db.notes.length : 0 @@ -76,24 +86,213 @@ Module { // The store, watched rather than read on open: the tile's count has to // move when a note is added from either side, the CLI included. A missing // file is an empty store, not an error worth logging. + // + // fileChanged means the file was replaced, and text() still holds the old + // contents until the reload lands, which is why the reparse lives on + // onLoaded and not on fileChanged (the same split Prefs.qml uses). property FileView dbView: FileView { path: mod.dbPath atomicWrites: true watchChanges: true printErrors: false - onFileChanged: mod.reparse() + onFileChanged: reload() onLoaded: mod.reparse() onLoadFailed: mod.reparse() } // Nothing to read until the config has resolved, so the load failed on the - // empty path does not mark the store ready. + // empty path does not mark the store ready. An unreadable db.json keeps + // the last good parse and says so rather than showing an empty store. function reparse() { if (mod.dbPath === "") return; - mod.db = Notes.parse(mod.dbView.text()); + const v = Notes.valid(mod.dbView.text()); + if (v !== null) mod.db = v; + else mod.lastError = mod.indexError(); mod.ready = true; } + function indexError() { + return "db.json could not be read; resolve it in " + mod.baseDir; + } + + // db.json as it is on disk right now, or null when it cannot be read. + // reload() does not block even with blockLoading set, so waitForJob() is + // what makes text() the new contents; without it a create would append to + // the db.json from before the pull. null is load-bearing: a conflicted + // db.json (merge markers an autostash pop left behind) must stop the write, + // not be replaced by an empty one. + function readDbNow() { + mod.dbView.reload(); + mod.dbView.waitForJob(); + return Notes.valid(mod.dbView.text()); + } + + // db.json, written atomically. mod.db is assigned first so the page does + // not wait for the watch to come back round. + function writeDb(db) { + mod.db = db; + mod.dbView.setText(Notes.stringify(db)); + } + + // A note body. notes.sh keeps the title out of the file, so this is the + // body and nothing else. setText writes the path directly and needs no + // prior load; reading the file back is what would need one, and nothing + // here reads it back. + property FileView bodyView: FileView { + printErrors: false + } + + function writeBody(path, text, done) { + mod.bodyView.path = path; + mod.bodyView.setText(text); + if (done) done(true, ""); + } + + function fileIn(db, id) { + for (const n of (db.notes || [])) if (n.id === id) return n.file || ""; + return ""; + } + + // --- git --------------------------------------------------------------- + // + // Both shells fold stderr into stdout with 2>&1, so one collector holds + // the reason. onExited must not read the collector directly: its + // onStreamFinished can fire after the process exits. The collector stores + // the text in mod.gitOutput and onExited reads that and the exit code. + // The two processes never run at once, so sharing gitOutput is safe. + + property Process gitPull: Process { + // "create" resumes the create this pull was for; "stamp" is Update list. + property string after: "" + property string title: "" + property string body: "" + property var done: null + + onExited: code => { + mod.lastError = code === 0 ? "" : mod.gitOutput; + const a = mod.gitPull.after; + mod.gitPull.after = ""; + if (a === "create") mod.finishCreate(code, mod.gitPull.title, mod.gitPull.body, mod.gitPull.done); + else if (a === "stamp") mod.stampPull(code, mod.gitPull.done); + } + stdout: StdioCollector { onStreamFinished: mod.gitOutput = this.text } + } + + property Process gitSync: Process { + property var done: null + + onExited: code => { + mod.syncing = false; + mod.lastError = code === 0 ? "" : mod.gitOutput; + const d = mod.gitSync.done; + mod.gitSync.done = null; + if (d) d(code === 0, code === 0 ? "" : mod.gitOutput); + } + stdout: StdioCollector { onStreamFinished: mod.gitOutput = this.text } + } + + function startPull() { + mod.lastError = ""; + mod.gitPull.command = ["sh", "-c", 'git -C "$1" pull --rebase --autostash 2>&1', "sh", mod.baseDir]; + // Reassigning running on a running Process does nothing, so clear it + // first (Status.qml does the same for its breaktimer). + mod.gitPull.running = false; + mod.gitPull.running = true; + } + + // Update list: pull, then stamp lastpull so notes.sh -l does not pull again + // straight away. Stamped only on a pull that worked: claiming to have + // pulled after a failure would suppress the CLI's own pull for an hour. + function pull(done) { + if (!mod.gitEnabled) { if (done) done(false, "git is disabled"); return; } + mod.gitPull.done = done; + mod.gitPull.after = "stamp"; + mod.startPull(); + } + + function stampPull(code, done) { + if (code !== 0) { if (done) done(false, mod.gitOutput); return; } + const db = mod.readDbNow(); + if (db === null) { if (done) done(false, mod.indexError()); return; } + mod.writeDb(Notes.setLastpull(db, Math.floor(Date.now() / 1000))); + if (done) done(true, ""); + } + + // Create writes the note after the pull, on the db.json the pull brought + // in: notes.sh reads the id after its own gitsync for the same reason. + function create(title, body, done) { + if (!mod.ready) { if (done) done(false, "notes not loaded yet"); return; } + if (!mod.gitEnabled) { mod.finishCreate(0, title, body, done); return; } + mod.gitPull.title = title; + mod.gitPull.body = body; + mod.gitPull.done = done; + mod.gitPull.after = "create"; + mod.startPull(); + } + + function finishCreate(code, title, body, done) { + const db = mod.readDbNow(); + if (db === null) { if (done) done(false, mod.indexError()); return; } + const now = Math.floor(Date.now() / 1000); + const file = String(now); + let next = Notes.addNote(db, title, file); + // Only when a pull actually happened: with git off there was none, and + // claiming one would suppress notes.sh's own pull later. + if (code === 0 && mod.gitEnabled) next = Notes.setLastpull(next, now); + mod.lastCreatedId = next.notes[next.notes.length - 1].id; + mod.writeDb(next); + mod.writeBody(mod.notesDir + "/" + file, body, () => { if (done) done(true, ""); }); + // shortcut: second-granularity filenames, as notes.sh takes date +%s, + // so two notes in one second would share a file. Upgrade to a fresh + // second or a suffix if that is ever seen. + } + + // Edit keeps the file name and the id, replacing the title in db.json and + // the body in notes/. notes.sh pulls on add only. + function update(id, title, body, done) { + if (!mod.ready) { if (done) done(false, "notes not loaded yet"); return; } + const db = mod.readDbNow(); + if (db === null) { if (done) done(false, mod.indexError()); return; } + const file = mod.fileIn(db, id); + if (file === "") { if (done) done(false, "note not found"); return; } + mod.writeDb(Notes.updateNote(db, id, title)); + mod.writeBody(mod.notesDir + "/" + file, body, () => { if (done) done(true, ""); }); + } + + // Delete drops the entry and removes the body file. FileView has no + // delete, so this is the module's one filesystem call. + function remove(id, done) { + if (!mod.ready) { if (done) done(false, "notes not loaded yet"); return; } + const db = mod.readDbNow(); + if (db === null) { if (done) done(false, mod.indexError()); return; } + const file = mod.fileIn(db, id); + mod.writeDb(Notes.removeNote(db, id)); + if (file !== "") Quickshell.execDetached(["rm", "--", mod.notesDir + "/" + file]); + if (done) done(true, ""); + } + + function msg(kind, id) { + if (kind === "create") return "quickshell - adding note from " + mod.client; + if (kind === "edit") return "quickshell - " + mod.client + " note edited."; + return "quickshell - " + mod.client + " removing note ID " + id + "."; + } + + // The second click of Save or Delete: add everything, commit (a commit + // with nothing staged is not a failure), push. No pull here; creating + // already pulled. + function commitPush(kind, id, done) { + if (!mod.gitEnabled) { if (done) done(true, ""); return; } + if (mod.syncing) return; + mod.syncing = true; + mod.lastError = ""; + mod.gitSync.done = done; + mod.gitSync.command = ["sh", "-c", + 'git -C "$1" add . && { git -C "$1" commit -m "$2" || true; } && git -C "$1" push origin HEAD 2>&1', + "sh", mod.baseDir, mod.msg(kind, id)]; + mod.gitSync.running = false; + mod.gitSync.running = true; + } + tileContent: Component { Text { width: parent ? parent.width : implicitWidth diff --git a/desktop/modules/notes/NotesPage.qml b/desktop/modules/notes/NotesPage.qml index d73bea2..a3d11f6 100644 --- a/desktop/modules/notes/NotesPage.qml +++ b/desktop/modules/notes/NotesPage.qml @@ -9,27 +9,148 @@ // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // GNU General Public License for more details. +import Quickshell.Io import QtQuick import QtQuick.Controls import "../.." import "Notes.js" as Notes -// The notes list, newest first, with a title search. Reads the module's parsed -// db.json; nothing here touches the store or git. +// The notes page: a list, and an editor over it. +// +// Save is two clicks and each click does one thing. The first writes the note +// and, when that has finished, arms the button as "Commit & push?"; the second +// commits and pushes and returns to the list. Arming the second click only +// after the write completes is what stops a quick second click from committing +// a note the first click has not written yet. +// +// Nothing is remembered between visits: leaving the editor with text that was +// not saved drops it, which is the clipboard page's own rule. Item { id: page - // The notes module: the store, the resolved paths and the writes. + // The notes module: the store, the resolved paths, the writes and git. required property QtObject owner - Row { - id: bar - anchors { top: parent.top; left: parent.left; right: parent.right } - spacing: 8 + property string view: "list" // "list" | "editor" + property var editing: null // the note being edited, null for new + property bool preview: false // editor shows rendered Markdown + property string bodyError: "" + // A write or git command is in flight; the buttons stand down meanwhile. + property bool busy: false + + function open(note) { + page.bodyError = ""; + page.preview = note !== null; + confirm.stop(); + confirmDel.stop(); + 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 + // after would wipe the body it just read. + bodyEdit.text = ""; + page.view = "editor"; + page.editing = note; + Qt.callLater(() => titleField.forceActiveFocus()); + } + + function toList() { + confirm.stop(); + confirmDel.stop(); + page.busy = false; + page.editing = null; + page.bodyError = ""; + page.view = "list"; + } + + 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; + page.busy = true; + page.owner.commitPush(kind, id, (ok, msg) => { + page.busy = false; + if (!ok) page.owner.lastError = msg; + page.toList(); + }); + return; + } + page.owner.lastError = ""; + page.busy = true; + if (page.editing) page.owner.update(page.editing.id, titleField.text, bodyEdit.text, done); + else page.owner.create(titleField.text, bodyEdit.text, done); + } + + 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(); + page.busy = true; + page.owner.commitPush("delete", id, (ok, msg) => { + page.busy = false; + if (!ok) page.owner.lastError = msg; + page.toList(); + }); + return; + } + page.owner.lastError = ""; + page.busy = true; + page.owner.remove(id, done); + } + + function updateList() { + if (page.busy) return; + page.owner.lastError = ""; + page.busy = true; + page.owner.pull((ok, msg) => { + page.busy = false; + if (ok) updated.restart(); else page.owner.lastError = msg; + }); + } + + // The open note's body. Nothing watches it: it is read when the editor + // opens and written on save. + FileView { + id: bodyRead + path: (page.view === "editor" && page.editing) + ? page.owner.notesDir + "/" + page.editing.file : "" + printErrors: false + onLoaded: { bodyEdit.text = text(); page.bodyError = ""; } + onLoadFailed: { + bodyEdit.text = ""; + page.bodyError = page.editing ? "the note's file is missing" : ""; + } + } + + Timer { id: confirm; interval: 3000 } + Timer { id: confirmDel; interval: 3000 } + Timer { id: updated; interval: 2000 } + + // --- list -------------------------------------------------------------- + + Item { + id: listView + anchors.fill: parent + visible: page.view === "list" TextField { id: search - width: parent.width + anchors { top: parent.top; left: parent.left; right: newBtn.left; rightMargin: 8 } height: 32 placeholderText: "Search" color: Theme.text @@ -40,82 +161,219 @@ Item { border.width: 1 border.color: Qt.alpha(Theme.text, 0.15) } - Component.onCompleted: forceActiveFocus() onTextChanged: list.currentIndex = 0 } - } - ListView { - id: list - anchors { - top: bar.bottom; topMargin: 10 - left: parent.left; right: parent.right - bottom: errorLine.top; bottomMargin: 6 + Button { + id: updateBtn + anchors { top: parent.top; right: parent.right } + visible: page.owner.gitEnabled + enabled: !page.busy + text: updated.running ? "Updated" : "Update list" + onClicked: page.updateList() + } + + Button { + id: newBtn + anchors { top: parent.top; right: updateBtn.visible ? updateBtn.left : parent.right + rightMargin: updateBtn.visible ? 8 : 0 } + enabled: page.owner.ready + text: "New note" + onClicked: page.open(null) } - clip: true - spacing: 4 - model: Notes.filter(page.owner.notes, search.text) - boundsBehavior: Flickable.StopAtBounds - ScrollBar.vertical: ScrollBar {} Text { - anchors.centerIn: parent + id: listEmpty + anchors.centerIn: list visible: list.count === 0 text: page.owner.ready ? "No notes" : "Loading…" color: Theme.subtext font { family: Theme.fontFamily; pixelSize: Theme.fontSize - 2 } } - delegate: Rectangle { - id: row - required property var modelData - required property int index + ListView { + id: list + anchors { + top: search.bottom; topMargin: 10 + left: parent.left; right: parent.right + bottom: listError.top; bottomMargin: 6 + } + clip: true + spacing: 4 + model: Notes.filter(page.owner.notes, search.text) + boundsBehavior: Flickable.StopAtBounds + ScrollBar.vertical: ScrollBar {} - width: list.width - height: 36 - radius: 8 - color: rowArea.containsMouse || ListView.isCurrentItem - ? Qt.alpha(Theme.accent, 0.18) : Qt.alpha(Theme.surface, 0.35) + delegate: Rectangle { + id: row + required property var modelData + required property int index - MouseArea { - id: rowArea - anchors.fill: parent - hoverEnabled: true - cursorShape: Qt.PointingHandCursor - } + width: list.width + height: 36 + radius: 8 + color: rowArea.containsMouse || ListView.isCurrentItem + ? Qt.alpha(Theme.accent, 0.18) : Qt.alpha(Theme.surface, 0.35) - Text { - anchors { - left: parent.left; leftMargin: 10 - right: when.left; rightMargin: 8 - verticalCenter: parent.verticalCenter + MouseArea { + id: rowArea + anchors.fill: parent + hoverEnabled: true + cursorShape: Qt.PointingHandCursor + onClicked: page.open(row.modelData) } - elide: Text.ElideRight - maximumLineCount: 1 - textFormat: Text.PlainText - text: row.modelData.title - color: Theme.text - font { family: Theme.fontFamily; pixelSize: Theme.fontSize - 2 } + + Text { + anchors { + left: parent.left; leftMargin: 10 + right: when.left; rightMargin: 8 + verticalCenter: parent.verticalCenter + } + elide: Text.ElideRight + maximumLineCount: 1 + textFormat: Text.PlainText + text: row.modelData.title + color: Theme.text + font { family: Theme.fontFamily; pixelSize: Theme.fontSize - 2 } + } + + Text { + id: when + anchors { right: parent.right; rightMargin: 10; verticalCenter: parent.verticalCenter } + text: Notes.formatDate(row.modelData.file) + color: Theme.overlay + font { family: Theme.fontFamily; pixelSize: Theme.fontSize - 3 } + } + } + } + + Text { + id: listError + anchors { left: parent.left; right: parent.right; bottom: parent.bottom } + height: visible ? implicitHeight : 0 + visible: page.owner.lastError !== "" + text: page.owner.lastError + wrapMode: Text.WordWrap + color: Theme.red + font { family: Theme.fontFamily; pixelSize: Theme.fontSize - 3 } + } + } + + // --- editor ------------------------------------------------------------ + + Item { + id: editorView + anchors.fill: parent + visible: page.view === "editor" + + Button { + id: cancelBtn + anchors { top: parent.top; left: parent.left } + text: "Cancel" + onClicked: page.toList() + } + + 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" + onClicked: page.save() + } + + Button { + id: delBtn + 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" + onClicked: page.remove() + } + + // Edit (raw Markdown) on the left, Preview (rendered) on the right. + // The switch that owns the state is Switch.qml, as elsewhere. + Item { + id: previewToggle + anchors { + top: parent.top; right: delBtn.visible ? delBtn.left : saveBtn.left + rightMargin: 12 } + width: prevLabel.implicitWidth + 8 + prevSwitch.implicitWidth + height: Math.max(prevLabel.implicitHeight, prevSwitch.implicitHeight) Text { - id: when - anchors { right: parent.right; rightMargin: 10; verticalCenter: parent.verticalCenter } - text: Notes.formatDate(row.modelData.file) - color: Theme.overlay + id: prevLabel + anchors { left: parent.left; verticalCenter: parent.verticalCenter } + text: "Preview" + color: Theme.subtext font { family: Theme.fontFamily; pixelSize: Theme.fontSize - 3 } } + + Switch { + id: prevSwitch + anchors { right: parent.right; verticalCenter: parent.verticalCenter } + checked: page.preview + onToggled: page.preview = checked + } + } + + TextField { + id: titleField + anchors { top: parent.top; topMargin: 44; left: parent.left; right: parent.right } + height: 32 + placeholderText: "Title" + color: Theme.text + font { family: Theme.fontFamily; pixelSize: Theme.fontSize - 1 } + background: Rectangle { + radius: 6 + color: Qt.alpha(Theme.surface, 0.6) + border.width: 1 + border.color: Qt.alpha(Theme.text, 0.15) + } } - } - Text { - id: errorLine - anchors { left: parent.left; right: parent.right; bottom: parent.bottom } - height: visible ? implicitHeight : 0 - visible: page.owner.lastError !== "" - text: page.owner.lastError - wrapMode: Text.WordWrap - color: Theme.red - font { family: Theme.fontFamily; pixelSize: Theme.fontSize - 3 } + Text { + id: bodyErr + anchors { top: titleField.bottom; topMargin: 6; left: parent.left; right: parent.right } + height: visible ? implicitHeight : 0 + visible: page.bodyError !== "" + text: page.bodyError + color: Theme.yellow + font { family: Theme.fontFamily; pixelSize: Theme.fontSize - 3 } + } + + TextEdit { + id: bodyEdit + anchors { + top: bodyErr.bottom; topMargin: 6 + left: parent.left; right: parent.right + bottom: editError.top; bottomMargin: 6 + } + readOnly: page.preview + selectByMouse: true + textFormat: page.preview ? TextEdit.MarkdownText : TextEdit.PlainText + wrapMode: TextEdit.Wrap + color: Theme.text + selectionColor: Qt.alpha(Theme.accent, 0.4) + font { family: Theme.fontFamily; pixelSize: Theme.fontSize - 2 } + // Web and mail links only, as Segments.qml does: a note is the + // user's own, but a file: or custom scheme link would still reach + // xdg-open. + onLinkActivated: link => { if (/^(https?|mailto):/i.test(link)) Qt.openUrlExternally(link); } + HoverHandler { cursorShape: bodyEdit.hoveredLink ? Qt.PointingHandCursor : Qt.IBeamCursor } + } + + Text { + id: editError + anchors { left: parent.left; right: parent.right; bottom: parent.bottom } + height: visible ? implicitHeight : 0 + visible: page.owner.lastError !== "" + text: page.owner.lastError + wrapMode: Text.WordWrap + color: Theme.red + font { family: Theme.fontFamily; pixelSize: Theme.fontSize - 3 } + } } } diff --git a/desktop/modules/notes/test_notes.js b/desktop/modules/notes/test_notes.js index 955369c..6a316ce 100644 --- a/desktop/modules/notes/test_notes.js +++ b/desktop/modules/notes/test_notes.js @@ -18,7 +18,7 @@ const assert = require("assert"); const src = fs.readFileSync(__dirname + "/Notes.js", "utf8") .replace(/^\.pragma library$/m, ""); -const N = new Function(src + "\nreturn { parse, stringify, sorted, formatDate, nextId," +const N = new Function(src + "\nreturn { parse, valid, stringify, sorted, formatDate, nextId," + " filter, addNote, updateNote, removeNote, setLastpull };")(); const db = { @@ -51,6 +51,18 @@ assert.deepStrictEqual(N.parse(N.stringify(db)), db); assert.deepStrictEqual(N.parse("not json"), { notes: [] }); assert.deepStrictEqual(N.parse('{"a":1}'), { notes: [] }); +// valid(): the object, or null when the file cannot be read. A write must +// refuse on null rather than replace an unreadable index, which is what a +// conflicted db.json is: merge markers from an autostash pop over a remote +// change. An empty or missing file is an empty store, not an error. +assert.deepStrictEqual(N.valid(N.stringify(db)), db); +assert.deepStrictEqual(N.valid(""), { notes: [] }); +assert.deepStrictEqual(N.valid(" \n"), { notes: [] }); +assert.strictEqual(N.valid("not json"), null); +assert.strictEqual(N.valid('{"a":1}'), null); +assert.strictEqual(N.valid("<<<<<<< HEAD\n{}\n=======\n{}\n>>>>>>> other\n"), null); +assert.deepStrictEqual(N.parse("not json"), { notes: [] }); // display stays tolerant + // pure transforms preserve everything else and do not mutate const added = N.addNote(db, "T", "1750"); assert.strictEqual(added.notes.length, 4); diff --git a/docs/superpowers/specs/2026-10-16-notes-design.md b/docs/superpowers/specs/2026-10-16-notes-design.md index 9b460a8..a9f5420 100644 --- a/docs/superpowers/specs/2026-10-16-notes-design.md +++ b/docs/superpowers/specs/2026-10-16-notes-design.md @@ -111,6 +111,15 @@ All index writes go through the module's `db.json` `FileView` with `db.json` for `notes.sh` to trip on. A body is written with a `FileView` on its own path, which creates the file. +**A write is refused when `db.json` cannot be read as the index.** That is not +hypothetical: `git pull --rebase --autostash` resolves a remote `db.json` +change against a local uncommitted one, the stash pop conflicts, git still +exits 0, and the file is left full of merge markers. Reading that as "no notes" +and writing it back would delete every title, so the write instead reports +`db.json could not be read; resolve it in ` and leaves the file for +git to resolve. `Notes.valid` is the check; `Notes.parse` stays tolerant for +display, returning an empty store for the same input. + - **Create**: the first click pulls first (see Git), then re-reads `db.json` so the append is based on what the pull brought in, not on what was read before it; `notes.sh` reads the id after its own pull for the same reason. @@ -163,12 +172,16 @@ clipboard page keeps for its own untrusted text. `git add .` runs from that changed nothing, is not a failure: git has nothing to record and the push still runs. -`git.lastpull` is stamped after the pull, not before. `notes.sh` stamps first -and then pulls, which leaves its own `db.json` dirty for the pull it is about -to run, and it ignores the resulting failure; stamping on return keeps the -field truthful without that. The field is what `notes.sh -l` reads to decide -whether to pull again inside `GITSYNCDELAY`, so a page pull that did not stamp -would make every CLI list pull again. +`git.lastpull` is stamped after the pull, not before, and only when that pull +worked. `notes.sh` stamps first and then pulls, which leaves its own `db.json` +dirty for the pull it is about to run, and it ignores the resulting failure; +stamping on return keeps the field truthful without that. The field is what +`notes.sh -l` reads to decide whether to pull again inside `GITSYNCDELAY`, so a +page pull that did not stamp would make every CLI list pull again. + +A pull that fails without leaving a conflict does not stop a note being saved: +the note is written and the failure is shown, as a note taker that refused to +work offline would be worse. Only an unreadable index stops the write (above). The commit message follows `notes.sh`'s wording but names the page, so history says where a change came from: `quickshell - adding note from @@ -224,7 +237,9 @@ Registering: `import "modules/notes"` and a `NotesModule {}` in `shell.qml`'s `params` and `git.lastpull` intact, and adding does not mutate the input object. - A malformed or truncated `db.json` yields an empty list rather than a - throw, so the page renders "no notes" instead of failing to load. + throw, so the page renders "no notes" instead of failing to load, while + `valid` returns null for the same input so a write refuses instead of + replacing it. Merge markers read the same way. The tests must be shown to fail on a broken input before they are trusted. -- cgit v1.2.3