From fbcd6cdd7aa675c88def07d10331de7977b921d6 Mon Sep 17 00:00:00 2001 From: Rodrigo Couto Date: Sun, 16 Aug 2026 11:32:26 -0300 Subject: [PATCH 1/8] Take the name of a file that is not there yet Backend::open only reads: a path with nothing behind it fails the ReadOnly open, reports "Could not open", and returns before setFileUrl, so the document keeps no name at all. Opening a file that has yet to be written -- `omawrite new_document.md`, the way every editor is asked to start one -- therefore left Ctrl+S with nothing to save to, and the portal's Save As dialog asked for a name the writer had already given. Claim the URL for a blank document when the path is not there, and let the first save land on it. A path that exists but cannot be read is still an error, as it should be: this is only about the file that is not there yet. Fixes #6 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TyQRJCyR76uk7XaNAB8jMC --- src/backend.cpp | 14 ++++++++++ tests/tst_omawrite.cpp | 63 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/src/backend.cpp b/src/backend.cpp index 90e279e..a4969c5 100644 --- a/src/backend.cpp +++ b/src/backend.cpp @@ -206,6 +206,20 @@ void Backend::open(const QUrl &url) { const QString targetName = QFileInfo(url.toLocalFile()).fileName(); QFile file(url.toLocalFile()); + // A path that is not there yet is a file the writer means to start, so + // take the name for a blank document. The first save then lands where + // they said it should, instead of asking them again. + if (!file.exists()) { + loadDocumentText(QString()); + clearRecovery(); + m_lastKnownFileContents.clear(); + m_hasKnownFileContents = false; + setFileUrl(url); + setModified(false); + setStatus(QStringLiteral("New file %1").arg(fileName())); + return; + } + if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) { setStatus(QStringLiteral("Could not open %1.").arg(targetName)); return; diff --git a/tests/tst_omawrite.cpp b/tests/tst_omawrite.cpp index 5c3306a..4441872 100644 --- a/tests/tst_omawrite.cpp +++ b/tests/tst_omawrite.cpp @@ -162,6 +162,69 @@ private slots: QCOMPARE(editor->property("wrappedSelectionEnd").toInt(), 12); } + void startsANewFileFromAPathThatIsNotThereYet() { + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const QString newPath = directory.filePath(QStringLiteral("new_document.md")); + const QString existingPath = directory.filePath(QStringLiteral("already-there.md")); + QFile existing(existingPath); + QVERIFY(existing.open(QIODevice::WriteOnly | QIODevice::Text)); + existing.write("on disk already"); + existing.close(); + + const QString mainQmlPath = QFINDTESTDATA("../src/Main.qml"); + QVERIFY(!mainQmlPath.isEmpty()); + + Backend backend; + QQmlEngine engine; + engine.rootContext()->setContextProperty(QStringLiteral("backend"), &backend); + QQmlComponent component(&engine, QUrl::fromLocalFile(mainQmlPath)); + QVERIFY2(component.isReady(), qPrintable(component.errorString())); + QScopedPointer window(component.create()); + QVERIFY2(window, qPrintable(component.errorString())); + + QObject *editor = window->findChild(QStringLiteral("sourceEditor")); + QVERIFY(editor); + + // A name from the command line that is not on disk yet is still this + // document's name, blank as the document is. + QSignalSpy saveDialogSpy(&backend, &Backend::saveDialogRequested); + backend.open(QUrl::fromLocalFile(newPath)); + QCOMPARE(backend.fileUrl(), QUrl::fromLocalFile(newPath)); + QCOMPARE(backend.fileName(), QStringLiteral("new_document.md")); + QCOMPARE(backend.status(), QStringLiteral("New file new_document.md")); + QCOMPARE(editor->property("text").toString(), QString()); + QVERIFY(!backend.modified()); + + // Opening it wrote nothing: the file appears when the writer saves. + QVERIFY(!QFileInfo::exists(newPath)); + + editor->setProperty("text", QStringLiteral("first words")); + QVERIFY(backend.modified()); + backend.save(); + QCOMPARE(saveDialogSpy.count(), 0); + QVERIFY(!backend.modified()); + + QFile written(newPath); + QVERIFY(written.open(QIODevice::ReadOnly | QIODevice::Text)); + QCOMPARE(written.readAll(), QByteArray("first words")); + written.close(); + + // A file that is there still opens and reads. + backend.open(QUrl::fromLocalFile(existingPath)); + QCOMPARE(backend.fileName(), QStringLiteral("already-there.md")); + QCOMPARE(editor->property("text").toString(), QStringLiteral("on disk already")); + + // A path that is there but cannot be read is still an error, and + // leaves the document it could not replace alone. + backend.open(QUrl::fromLocalFile(directory.path())); + QCOMPARE(backend.status(), + QStringLiteral("Could not open %1.") + .arg(QFileInfo(directory.path()).fileName())); + QCOMPARE(backend.fileUrl(), QUrl::fromLocalFile(existingPath)); + QCOMPARE(editor->property("text").toString(), QStringLiteral("on disk already")); + } + void savesAndOpensFromFooterButtons() { const QString mainQmlPath = QFINDTESTDATA("../src/Main.qml"); QVERIFY(!mainQmlPath.isEmpty()); From 770f7144d15dbfe2aad67b8c7b480e1ae037daf2 Mon Sep 17 00:00:00 2001 From: Omabot Date: Thu, 20 Aug 2026 04:15:16 -0700 Subject: [PATCH 2/8] Only take a name a file could be written to Taking any absent path meant a name under a directory that is not there was accepted too, and the failure moved from the open to the first save: `omawrite ~/notes/2026/draft.md` with no `2026/` directory opened a document titled `draft.md`, and Ctrl+S then answered with `Could not save draft.md.` in the footer and no dialog, because the URL was valid enough to skip the Save As fallback that used to catch this. Same for a name in a directory the writer cannot write, and for a trailing-slash path, which is a directory name that `fileName()` reports as `Untitled.md`. Claim the name only where the file could actually be created. Everything else keeps master's answer: `Could not open`, and the first save still offers somewhere to put the text. Co-Authored-By: Claude Opus 5 (1M context) --- src/backend.cpp | 8 ++++++++ tests/tst_omawrite.cpp | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/src/backend.cpp b/src/backend.cpp index a4969c5..7677bdc 100644 --- a/src/backend.cpp +++ b/src/backend.cpp @@ -210,6 +210,14 @@ void Backend::open(const QUrl &url) { // take the name for a blank document. The first save then lands where // they said it should, instead of asking them again. if (!file.exists()) { + // Only where it could be written: a name under a directory that is not + // there leaves the first save with nowhere to land and no dialog. + const QFileInfo parentDirectory(QFileInfo(url.toLocalFile()).absolutePath()); + if (!parentDirectory.isDir() || !parentDirectory.isWritable()) { + setStatus(QStringLiteral("Could not open %1.").arg(targetName)); + return; + } + loadDocumentText(QString()); clearRecovery(); m_lastKnownFileContents.clear(); diff --git a/tests/tst_omawrite.cpp b/tests/tst_omawrite.cpp index 4441872..959ec92 100644 --- a/tests/tst_omawrite.cpp +++ b/tests/tst_omawrite.cpp @@ -223,6 +223,14 @@ private slots: .arg(QFileInfo(directory.path()).fileName())); QCOMPARE(backend.fileUrl(), QUrl::fromLocalFile(existingPath)); QCOMPARE(editor->property("text").toString(), QStringLiteral("on disk already")); + + // A name under a directory that is not there is not a file anyone can + // start, so it stays an error rather than a document that cannot save. + backend.open(QUrl::fromLocalFile( + directory.filePath(QStringLiteral("not-there/child.md")))); + QCOMPARE(backend.status(), QStringLiteral("Could not open child.md.")); + QCOMPARE(backend.fileUrl(), QUrl::fromLocalFile(existingPath)); + QCOMPARE(editor->property("text").toString(), QStringLiteral("on disk already")); } void savesAndOpensFromFooterButtons() { From f4be8dce9df95a3f9ba6bbcef74932d3ef4dc525 Mon Sep 17 00:00:00 2001 From: Rodrigo Couto Date: Mon, 24 Aug 2026 13:06:45 -0300 Subject: [PATCH 3/8] Do not let a reload that lost its file start a new one reloadFromDisk() calls open(), and open() now takes a path with nothing behind it as a file the writer means to start. Reload is not that: it is asked for a file we already read. The "File changed" dialog leaves the Reload button enabled whenever the file was still there when the watcher fired, so a deletion in the seconds between the dialog opening and the click reaches reloadFromDisk() with the file gone -- and the new-file branch answered by blanking the editor, clearing the recovery snapshot and reporting "New file". That is the writer's only remaining copy, and master reported "Could not open" and left it alone. Route both callers through a private openPath() that carries whether an absent path may become a new document. open() still says yes; reload says no and falls through to the ReadOnly open that fails, which is master's answer, word for word. Checking existence in reloadFromDisk() first would leave the same race one function further along; the flag closes it rather than narrowing it. keepsTheDocumentWhenReloadRacesADeletion drives the real document: open a file, type over it, remove the file, reload. Without the flag the status reads "New file racing.md" and the typed text is gone; with it the status is "Could not open racing.md.", the text, the URL and the modified flag are all untouched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UXHc91hBNDVXyoaCpqXDae --- src/backend.cpp | 13 +++++++++++-- src/backend.h | 1 + tests/tst_omawrite.cpp | 40 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/src/backend.cpp b/src/backend.cpp index 7677bdc..b558e85 100644 --- a/src/backend.cpp +++ b/src/backend.cpp @@ -199,6 +199,10 @@ void Backend::openDialog() { } void Backend::open(const QUrl &url) { + openPath(url, true); +} + +void Backend::openPath(const QUrl &url, bool mayStartNewFile) { if (!url.isLocalFile()) { setStatus(QStringLiteral("Only local files can be opened.")); return; @@ -209,7 +213,7 @@ void Backend::open(const QUrl &url) { // A path that is not there yet is a file the writer means to start, so // take the name for a blank document. The first save then lands where // they said it should, instead of asking them again. - if (!file.exists()) { + if (mayStartNewFile && !file.exists()) { // Only where it could be written: a name under a directory that is not // there leaves the first save with nowhere to land and no dialog. const QFileInfo parentDirectory(QFileInfo(url.toLocalFile()).absolutePath()); @@ -280,8 +284,13 @@ void Backend::discardRecovery() { } void Backend::reloadFromDisk() { + // Reload is asked for a file we already have, so it must not go down the + // path that takes an absent name for a new document: if the file goes away + // between the "File changed" dialog opening and the click, blanking the + // editor and clearing recovery would throw away the only copy left. Say + // it could not be opened and leave the text where it is. if (m_fileUrl.isLocalFile()) - open(m_fileUrl); + openPath(m_fileUrl, false); } void Backend::keepExternalVersion() { diff --git a/src/backend.h b/src/backend.h index 2429590..e940510 100644 --- a/src/backend.h +++ b/src/backend.h @@ -90,6 +90,7 @@ class Backend : public QObject { void externalChangeDetected(bool deleted, bool locallyModified); private: + void openPath(const QUrl &url, bool mayStartNewFile); void loadDocumentText(const QString &text); void setFileUrl(const QUrl &url); void setModified(bool modified); diff --git a/tests/tst_omawrite.cpp b/tests/tst_omawrite.cpp index 959ec92..6ad2966 100644 --- a/tests/tst_omawrite.cpp +++ b/tests/tst_omawrite.cpp @@ -233,6 +233,46 @@ private slots: QCOMPARE(editor->property("text").toString(), QStringLiteral("on disk already")); } + void keepsTheDocumentWhenReloadRacesADeletion() { + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const QString path = directory.filePath(QStringLiteral("racing.md")); + QFile onDisk(path); + QVERIFY(onDisk.open(QIODevice::WriteOnly | QIODevice::Text)); + onDisk.write("what was there"); + onDisk.close(); + + const QString mainQmlPath = QFINDTESTDATA("../src/Main.qml"); + QVERIFY(!mainQmlPath.isEmpty()); + + Backend backend; + QQmlEngine engine; + engine.rootContext()->setContextProperty(QStringLiteral("backend"), &backend); + QQmlComponent component(&engine, QUrl::fromLocalFile(mainQmlPath)); + QVERIFY2(component.isReady(), qPrintable(component.errorString())); + QScopedPointer window(component.create()); + QVERIFY2(window, qPrintable(component.errorString())); + + QObject *editor = window->findChild(QStringLiteral("sourceEditor")); + QVERIFY(editor); + + backend.open(QUrl::fromLocalFile(path)); + QCOMPARE(editor->property("text").toString(), QStringLiteral("what was there")); + editor->setProperty("text", QStringLiteral("words only I have")); + QVERIFY(backend.modified()); + + // The "File changed" dialog leaves Reload enabled for a file that was + // still there when it opened. If the file goes away before the click, + // the reload has nothing to read: it must say so, not take the missing + // path for a new document and blank the only copy of this text. + QVERIFY(QFile::remove(path)); + backend.reloadFromDisk(); + QCOMPARE(backend.status(), QStringLiteral("Could not open racing.md.")); + QCOMPARE(editor->property("text").toString(), QStringLiteral("words only I have")); + QCOMPARE(backend.fileUrl(), QUrl::fromLocalFile(path)); + QVERIFY(backend.modified()); + } + void savesAndOpensFromFooterButtons() { const QString mainQmlPath = QFINDTESTDATA("../src/Main.qml"); QVERIFY(!mainQmlPath.isEmpty()); From bf00e9115f999403ea044b76accbb41ef6f160ce Mon Sep 17 00:00:00 2001 From: Rodrigo Couto Date: Mon, 24 Aug 2026 15:13:35 -0300 Subject: [PATCH 4/8] Ask before the first save replaces a file that turned up Taking the name of a file that is not there leaves the path unguarded until the first save. watchCurrentFile() can only watch a file that exists and no directory is watched, so nothing can report a file appearing on that path -- and save() went straight to QSaveFile, whose commit() replaces whatever it finds. A `git pull` or a sync client landing that file between the open and Ctrl+S was overwritten in silence. Before this branch the same case had no name to save to and went through the Save As dialog, which at least asks. The state that says so needs its own flag. m_hasKnownFileContents answers a different question -- whether we hold a copy to compare a watcher event against -- and reading "we have never looked at this path" out of it does not work, because keepExternalVersion() sets it back to false whenever the disk read fails. Keying the guard on it therefore trapped the writer: if what turned up could not be read, a directory or a file they may write but not read, Keep Mine reported "Kept your version", changed nothing, and every Ctrl+S after it raised the same dialog again, with no answer that ended it. m_pathNeverRead is set only where this document takes a name without having read what is on it -- the new-file open, and a recovery snapshot naming a file that was never written -- and cleared by every outcome that settles the question: a successful read in openPath(), a successful write in saveTo(), a recovery snapshot whose file did read, and keepExternalVersion() on both of its branches, the failing one included. Failing to read is still an answer. The next save then goes to the filesystem and the filesystem's answer ends it, which is the only thing that can. The guard reports the appeared file with its own signal rather than externalChangeDetected. The two cases need different words and, more than that, different defaults: ExternalChangeDialog focuses and highlights Reload for an ordinary outside edit, which is right when the file on disk is a second copy of the work, and catastrophic here, where the writer's only copy is in the editor and reloadFromDisk() clears the recovery snapshot on its way past. Enter on that dialog would have taken the whole draft. Keep Mine now leads whenever the safe answer is to keep, deleted and appeared alike, and the text says a file appeared and that reloading discards everything typed. The guard is in save() rather than saveTo() on purpose. saveAs() reaches saveTo() from the portal's Save File dialog, which has already asked about replacing whatever the writer picked; asking a second time in a different dialog would be worse than not asking at all. Three tests. asksBeforeAFirstSaveReplacesAFileThatAppeared: without the guard the file on disk reads "my draft" where "arrived from elsewhere" was. asksOnlyOnceWhenWhatAppearedCannotBeRead puts a directory on the path; without the clear in keepExternalVersion() the dialog is raised twice instead of once and the readable case never saves at all. putsKeepMineForwardWhenAFileAppeared holds the button rule; under the old one Keep Mine is not primary for an appeared file, so Reload takes the focus. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UXHc91hBNDVXyoaCpqXDae --- src/ExternalChangeDialog.qml | 29 +++++-- src/Main.qml | 8 ++ src/backend.cpp | 26 +++++++ src/backend.h | 7 ++ tests/tst_omawrite.cpp | 142 +++++++++++++++++++++++++++++++++++ 5 files changed, 204 insertions(+), 8 deletions(-) diff --git a/src/ExternalChangeDialog.qml b/src/ExternalChangeDialog.qml index 2bf59c5..d20b972 100644 --- a/src/ExternalChangeDialog.qml +++ b/src/ExternalChangeDialog.qml @@ -5,7 +5,12 @@ Dialog { id: root property bool deleted: false + // A file turned up on a path this document took while it was empty and + // never read. Unlike the other two cases there is no copy of this text on + // disk, so Reload is the destructive answer here, not the safe one. + property bool appeared: false property bool locallyModified: false + readonly property bool keepIsSafer: deleted || appeared property bool darkMode: true property color textColor: darkMode ? "#d0d0d0" : "#42464c" property color strongTextColor: darkMode ? "#eeeeee" : "#222324" @@ -25,7 +30,7 @@ Dialog { y: Math.round((containerHeight - height) / 2) padding: 20 - onOpened: (deleted ? keepButton : reloadButton).forceActiveFocus() + onOpened: (keepIsSafer ? keepButton : reloadButton).forceActiveFocus() background: Rectangle { color: root.darkMode ? "#1a1a1a" : "#ffffff" @@ -37,7 +42,10 @@ Dialog { spacing: 12 Label { - text: root.deleted ? "File removed" : "File changed" + objectName: "externalChangeHeading" + text: root.deleted + ? "File removed" + : (root.appeared ? "File appeared" : "File changed") color: root.strongTextColor font.family: "iA Writer Mono S" font.pixelSize: Math.round(16 * root.textScale) @@ -45,12 +53,15 @@ Dialog { } Label { + objectName: "externalChangeMessage" width: parent.width text: root.deleted ? "This file was removed outside Omawrite. Keep your text as an unsaved document?" - : (root.locallyModified - ? "This file changed outside Omawrite. Reloading will discard your changes." - : "This file changed outside Omawrite.") + : (root.appeared + ? "Something else created this file after Omawrite took the name. None of your text has been written yet, so reloading will discard everything you have typed." + : (root.locallyModified + ? "This file changed outside Omawrite. Reloading will discard your changes." + : "This file changed outside Omawrite.")) color: root.textColor wrapMode: Text.Wrap font.family: "iA Writer Mono S" @@ -70,11 +81,12 @@ Dialog { SquareDialogButton { id: keepButton + objectName: "keepMineButton" text: "Keep Mine" darkMode: root.darkMode textScale: root.textScale - labelColor: root.deleted ? "#ffffff" : root.textColor - primary: root.deleted + labelColor: root.keepIsSafer ? "#ffffff" : root.textColor + primary: root.keepIsSafer activeColor: root.activeButtonColor KeyNavigation.left: reloadButton KeyNavigation.right: reloadButton @@ -88,9 +100,10 @@ Dialog { SquareDialogButton { id: reloadButton + objectName: "reloadButton" text: "Reload" enabled: !root.deleted - primary: true + primary: !root.keepIsSafer darkMode: root.darkMode textScale: root.textScale activeColor: root.activeButtonColor diff --git a/src/Main.qml b/src/Main.qml index cdcbc3e..22b2bc8 100644 --- a/src/Main.qml +++ b/src/Main.qml @@ -262,6 +262,14 @@ ApplicationWindow { function onExternalChangeDetected(deleted, locallyModified) { externalChangeDialog.deleted = deleted; + externalChangeDialog.appeared = false; + externalChangeDialog.locallyModified = locallyModified; + externalChangeDialog.open(); + } + + function onExternalFileAppeared(locallyModified) { + externalChangeDialog.deleted = false; + externalChangeDialog.appeared = true; externalChangeDialog.locallyModified = locallyModified; externalChangeDialog.open(); } diff --git a/src/backend.cpp b/src/backend.cpp index b558e85..a0931ba 100644 --- a/src/backend.cpp +++ b/src/backend.cpp @@ -226,6 +226,7 @@ void Backend::openPath(const QUrl &url, bool mayStartNewFile) { clearRecovery(); m_lastKnownFileContents.clear(); m_hasKnownFileContents = false; + m_pathNeverRead = true; setFileUrl(url); setModified(false); setStatus(QStringLiteral("New file %1").arg(fileName())); @@ -242,6 +243,7 @@ void Backend::openPath(const QUrl &url, bool mayStartNewFile) { clearRecovery(); m_lastKnownFileContents = contents; m_hasKnownFileContents = true; + m_pathNeverRead = false; setFileUrl(url); watchCurrentFile(); setModified(false); @@ -254,6 +256,20 @@ void Backend::save() { return; } + // Nothing can watch a file that is not there, so a name taken for a file + // that has yet to be written is unguarded until this save: a `git pull` or + // a sync client can put something on that path in the meantime and + // QSaveFile::commit() would replace it without a word. Ask once, and only + // once -- the flag is cleared by every answer the dialog can give, so a + // file that turns out to be unreadable cannot leave the writer trapped in + // a question they have already answered. + if (m_pathNeverRead && m_fileUrl.isLocalFile() + && QFileInfo::exists(m_fileUrl.toLocalFile())) { + m_closeAfterSave = false; + emit externalFileAppeared(m_modified); + return; + } + saveTo(m_fileUrl); } @@ -302,6 +318,11 @@ void Backend::keepExternalVersion() { m_lastKnownFileContents.clear(); m_hasKnownFileContents = false; } + // Answered, whether or not the file could be read. Failing to read it is + // not a reason to ask again: the writer said to keep their version, and + // the next save must be allowed to try, so the filesystem gets to give + // the answer instead of the dialog asking the same question forever. + m_pathNeverRead = false; setModified(true); scheduleRecovery(); watchCurrentFile(); @@ -531,6 +552,7 @@ void Backend::saveTo(const QUrl &url) { m_closeAfterSave = false; m_lastKnownFileContents = contents; m_hasKnownFileContents = true; + m_pathNeverRead = false; setFileUrl(url); watchCurrentFile(); QSettings().setValue(lastSaveDirectorySetting, @@ -582,9 +604,13 @@ void Backend::restoreRecovery() { if (recoveredUrl.isLocalFile() && diskFile.open(QIODevice::ReadOnly)) { m_lastKnownFileContents = diskFile.readAll(); m_hasKnownFileContents = true; + m_pathNeverRead = false; } else { m_lastKnownFileContents.clear(); m_hasKnownFileContents = false; + // A snapshot can name a file that was never written -- the crash came + // first. That is the same unverified path a new file starts on. + m_pathNeverRead = true; } setFileUrl(recoveredUrl); setModified(true); diff --git a/src/backend.h b/src/backend.h index e940510..31b8b7d 100644 --- a/src/backend.h +++ b/src/backend.h @@ -88,6 +88,7 @@ class Backend : public QObject { void saveDialogRequested(const QUrl &suggestedUrl); void saveSucceeded(); void externalChangeDetected(bool deleted, bool locallyModified); + void externalFileAppeared(bool locallyModified); private: void openPath(const QUrl &url, bool mayStartNewFile); @@ -133,6 +134,12 @@ class Backend : public QObject { QString m_lastDocumentText; QByteArray m_lastKnownFileContents; bool m_hasKnownFileContents = false; + // Set where this document takes a name without having read what is on it, + // and cleared the moment anything settles the question -- a read, a write, + // or the writer answering the dialog. It is not the same question as + // m_hasKnownFileContents, which asks whether we hold a copy to compare + // against; a path we have never looked at is one nothing can watch. + bool m_pathNeverRead = false; QString m_recoveryPath; std::unique_ptr m_recoveryLock; diff --git a/tests/tst_omawrite.cpp b/tests/tst_omawrite.cpp index 6ad2966..650ea24 100644 --- a/tests/tst_omawrite.cpp +++ b/tests/tst_omawrite.cpp @@ -233,6 +233,148 @@ private slots: QCOMPARE(editor->property("text").toString(), QStringLiteral("on disk already")); } + void asksBeforeAFirstSaveReplacesAFileThatAppeared() { + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const QString path = directory.filePath(QStringLiteral("arriving.md")); + + const QString mainQmlPath = QFINDTESTDATA("../src/Main.qml"); + QVERIFY(!mainQmlPath.isEmpty()); + + Backend backend; + QQmlEngine engine; + engine.rootContext()->setContextProperty(QStringLiteral("backend"), &backend); + QQmlComponent component(&engine, QUrl::fromLocalFile(mainQmlPath)); + QVERIFY2(component.isReady(), qPrintable(component.errorString())); + QScopedPointer window(component.create()); + QVERIFY2(window, qPrintable(component.errorString())); + + QObject *editor = window->findChild(QStringLiteral("sourceEditor")); + QVERIFY(editor); + + backend.open(QUrl::fromLocalFile(path)); + QCOMPARE(backend.status(), QStringLiteral("New file arriving.md")); + editor->setProperty("text", QStringLiteral("my draft")); + QVERIFY(backend.modified()); + + // A file that is not there cannot be watched, so nothing tells us when + // a `git pull` or a sync client puts one on that path. The first save + // is the first look, and it must not replace a file it has never read. + QFile arrived(path); + QVERIFY(arrived.open(QIODevice::WriteOnly | QIODevice::Text)); + arrived.write("arrived from elsewhere"); + arrived.close(); + + QSignalSpy appearedSpy(&backend, &Backend::externalFileAppeared); + QSignalSpy saveDialogSpy(&backend, &Backend::saveDialogRequested); + backend.save(); + QCOMPARE(appearedSpy.count(), 1); + QCOMPARE(appearedSpy.takeFirst().constFirst().toBool(), true); + + // Asked, not answered: the file on disk is whole and the draft is + // still unsaved. The name is not in question, so no Save As dialog. + QCOMPARE(saveDialogSpy.count(), 0); + QVERIFY(backend.modified()); + QFile untouched(path); + QVERIFY(untouched.open(QIODevice::ReadOnly | QIODevice::Text)); + QCOMPARE(untouched.readAll(), QByteArray("arrived from elsewhere")); + untouched.close(); + + // Keeping your version is what the dialog offers, and the save that + // follows it goes through: the guard asks once, it does not lock the + // writer out of the name they gave. + backend.keepExternalVersion(); + backend.save(); + QVERIFY(!backend.modified()); + QFile written(path); + QVERIFY(written.open(QIODevice::ReadOnly | QIODevice::Text)); + QCOMPARE(written.readAll(), QByteArray("my draft")); + written.close(); + } + + void asksOnlyOnceWhenWhatAppearedCannotBeRead() { + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const QString path = directory.filePath(QStringLiteral("blocked.md")); + + const QString mainQmlPath = QFINDTESTDATA("../src/Main.qml"); + QVERIFY(!mainQmlPath.isEmpty()); + + Backend backend; + QQmlEngine engine; + engine.rootContext()->setContextProperty(QStringLiteral("backend"), &backend); + QQmlComponent component(&engine, QUrl::fromLocalFile(mainQmlPath)); + QVERIFY2(component.isReady(), qPrintable(component.errorString())); + QScopedPointer window(component.create()); + QVERIFY2(window, qPrintable(component.errorString())); + + QObject *editor = window->findChild(QStringLiteral("sourceEditor")); + QVERIFY(editor); + + backend.open(QUrl::fromLocalFile(path)); + editor->setProperty("text", QStringLiteral("my draft")); + QVERIFY(backend.modified()); + + // What turns up on the path need not be a readable file. A directory + // is the plainest case: keepExternalVersion() cannot read it, so it + // has no contents to remember afterwards. + QVERIFY(QDir().mkpath(path)); + + QSignalSpy appearedSpy(&backend, &Backend::externalFileAppeared); + backend.save(); + QCOMPARE(appearedSpy.count(), 1); + + // Keeping your version answers the question, and an answer that could + // not be read is still an answer. Asking again would put the writer in + // a dialog with no way out of it, every Ctrl+S for the rest of the + // session. The second save goes to the filesystem and reports what the + // filesystem says, which is the only thing that can end this. + backend.keepExternalVersion(); + QCOMPARE(backend.status(), QStringLiteral("Kept your version")); + backend.save(); + QCOMPARE(appearedSpy.count(), 1); + QCOMPARE(backend.status(), QStringLiteral("Could not save blocked.md.")); + } + + void putsKeepMineForwardWhenAFileAppeared() { + const QString dialogPath = QFINDTESTDATA("../src/ExternalChangeDialog.qml"); + QVERIFY(!dialogPath.isEmpty()); + + QQmlEngine engine; + QQmlComponent component(&engine, QUrl::fromLocalFile(dialogPath)); + QVERIFY2(component.isReady(), qPrintable(component.errorString())); + QScopedPointer dialog(component.create()); + QVERIFY2(dialog, qPrintable(component.errorString())); + + QObject *keep = dialog->findChild(QStringLiteral("keepMineButton")); + QObject *reload = dialog->findChild(QStringLiteral("reloadButton")); + QObject *message = dialog->findChild(QStringLiteral("externalChangeMessage")); + QObject *heading = dialog->findChild(QStringLiteral("externalChangeHeading")); + QVERIFY(keep); + QVERIFY(reload); + QVERIFY(message); + QVERIFY(heading); + + // For an ordinary outside edit the file on disk is a second copy of + // the work, so Reload is the safe answer and leads, as it always has. + QVERIFY(!dialog->property("keepIsSafer").toBool()); + QVERIFY(reload->property("primary").toBool()); + QVERIFY(!keep->property("primary").toBool()); + + // For a file that appeared there is no second copy: every word the + // writer has is in the editor, and reloading throws all of it away, + // recovery snapshot included. The button that does that must not be + // the one Enter presses, and the text must say what is at stake. + dialog->setProperty("appeared", true); + QVERIFY(dialog->property("keepIsSafer").toBool()); + QVERIFY(keep->property("primary").toBool()); + QVERIFY(!reload->property("primary").toBool()); + QCOMPARE(heading->property("text").toString(), QStringLiteral("File appeared")); + const QString message_ = message->property("text").toString(); + QVERIFY2(message_.contains(QStringLiteral("created this file")), qPrintable(message_)); + QVERIFY2(message_.contains(QStringLiteral("discard everything")), qPrintable(message_)); + } + void keepsTheDocumentWhenReloadRacesADeletion() { QTemporaryDir directory; QVERIFY(directory.isValid()); From 98f0c3fa56b205bec337d48cfa5e4288d75db02f Mon Sep 17 00:00:00 2001 From: Rodrigo Couto Date: Mon, 24 Aug 2026 15:15:14 -0300 Subject: [PATCH 5/8] Guard the path a refused reload leaves behind Refusing the reload keeps the writer's text, which is what that change was for, but it leaves the document in a state nothing is looking after: a name, no file behind it, and a watcher that dropped the path when the file went. If the file comes back -- the pull that removed it landing the next commit, the sync client finishing what it started -- the save had nothing to stop it, and QSaveFile replaced the returned file unread. That is the same hazard the appeared-file guard exists to close, one step further along the same flow. It is the same state, so it gets the same flag. A read that fails on a path with nothing on it, reached from reloadFromDisk() rather than open(), sets m_pathNeverRead: this document has a name it has not looked behind. The open() side is deliberately left out. Its failure leaves a different document loaded under a different URL, and marking that one unread would be a lie about a file we did read. This is master's hazard too, not something the branch introduced, but the reload path is one this branch rewrote and it is a line away from the guard that closes it. keepsTheDocumentWhenReloadRacesADeletion now runs on past the refused reload: the file returns with different contents and the save must ask. Without the flag no dialog is raised at all, and the returned file is overwritten. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UXHc91hBNDVXyoaCpqXDae --- src/backend.cpp | 7 +++++++ tests/tst_omawrite.cpp | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/backend.cpp b/src/backend.cpp index a0931ba..c3b1218 100644 --- a/src/backend.cpp +++ b/src/backend.cpp @@ -234,6 +234,13 @@ void Backend::openPath(const QUrl &url, bool mayStartNewFile) { } if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) { + // A reload with nothing left to read leaves this document holding a + // name and no file, and the watcher let the path go when it went. + // That is the state a new file starts in, so say so: if the file + // comes back, the next save asks rather than replacing it unseen. + if (!mayStartNewFile && !file.exists()) + m_pathNeverRead = true; + setStatus(QStringLiteral("Could not open %1.").arg(targetName)); return; } diff --git a/tests/tst_omawrite.cpp b/tests/tst_omawrite.cpp index 650ea24..ff14b7d 100644 --- a/tests/tst_omawrite.cpp +++ b/tests/tst_omawrite.cpp @@ -413,6 +413,24 @@ private slots: QCOMPARE(editor->property("text").toString(), QStringLiteral("words only I have")); QCOMPARE(backend.fileUrl(), QUrl::fromLocalFile(path)); QVERIFY(backend.modified()); + + // Refusing the reload leaves this document where a new file starts: + // a name, nothing behind it, and a watcher that let the path go when + // the file did. So the same thing can happen again from here -- the + // pull that removed the file landing the next commit -- and the save + // has to ask about it just the same. + QFile returned(path); + QVERIFY(returned.open(QIODevice::WriteOnly | QIODevice::Text)); + returned.write("came back different"); + returned.close(); + + QSignalSpy appearedSpy(&backend, &Backend::externalFileAppeared); + backend.save(); + QCOMPARE(appearedSpy.count(), 1); + QFile intact(path); + QVERIFY(intact.open(QIODevice::ReadOnly | QIODevice::Text)); + QCOMPARE(intact.readAll(), QByteArray("came back different")); + intact.close(); } void savesAndOpensFromFooterButtons() { From b0bf1cc58258fb504fe974419f726330a09c745f Mon Sep 17 00:00:00 2001 From: Omabot Date: Tue, 25 Aug 2026 05:33:12 -0700 Subject: [PATCH 6/8] Drop the pending close when the first save asks instead of writing Closing a window with unsaved changes leaves `pendingAction` at "close" while the unsaved-changes dialog's Save runs, and `onSaveSucceeded` is what completes it. The appeared-file guard ends that save with a question rather than a write, so nothing completes it and the intent stays stored: the window the writer asked to close stays open, and then the next successful save -- an ordinary Ctrl+S minutes later, asked for on its own -- closes it on the earlier request. The question is raised where the save would have gone, so that is where the intent is dropped. It covers both answers and Escape, which leaves the question unanswered on purpose and must not carry a close along with it. This is issue #23's shape rather than its fix. A save that fails for any other reason still leaves the close standing, and the general repair belongs there; this closes the one route into it that taking the name early opens. Co-Authored-By: Claude Opus 5 (1M context) --- src/Main.qml | 5 +++++ tests/tst_omawrite.cpp | 47 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/src/Main.qml b/src/Main.qml index 22b2bc8..cc8e3f8 100644 --- a/src/Main.qml +++ b/src/Main.qml @@ -268,6 +268,11 @@ ApplicationWindow { } function onExternalFileAppeared(locallyModified) { + // This save is not going to happen, so whatever it was for cannot + // follow it. Leaving the intent standing lets an unrelated save + // minutes later close the window or open another document. + win.awaitingPendingSave = false; + win.pendingAction = ""; externalChangeDialog.deleted = false; externalChangeDialog.appeared = true; externalChangeDialog.locallyModified = locallyModified; diff --git a/tests/tst_omawrite.cpp b/tests/tst_omawrite.cpp index ff14b7d..da6093d 100644 --- a/tests/tst_omawrite.cpp +++ b/tests/tst_omawrite.cpp @@ -336,6 +336,53 @@ private slots: QCOMPARE(backend.status(), QStringLiteral("Could not save blocked.md.")); } + void dropsThePendingCloseWhenTheSaveIsRefused() { + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const QString path = directory.filePath(QStringLiteral("closing.md")); + + const QString mainQmlPath = QFINDTESTDATA("../src/Main.qml"); + QVERIFY(!mainQmlPath.isEmpty()); + + Backend backend; + QQmlEngine engine; + engine.rootContext()->setContextProperty(QStringLiteral("backend"), &backend); + QQmlComponent component(&engine, QUrl::fromLocalFile(mainQmlPath)); + QVERIFY2(component.isReady(), qPrintable(component.errorString())); + QScopedPointer window(component.create()); + QVERIFY2(window, qPrintable(component.errorString())); + + QObject *editor = window->findChild(QStringLiteral("sourceEditor")); + QVERIFY(editor); + + backend.open(QUrl::fromLocalFile(path)); + editor->setProperty("text", QStringLiteral("my draft")); + QVERIFY(backend.modified()); + + QFile arrived(path); + QVERIFY(arrived.open(QIODevice::WriteOnly | QIODevice::Text)); + arrived.write("arrived from elsewhere"); + arrived.close(); + + // Closing the window with unsaved changes leaves "close" standing + // while the unsaved-changes dialog's Save runs. The guard turns that + // save into a question, so the close it was for cannot follow. + window->setProperty("pendingAction", QStringLiteral("close")); + window->setProperty("awaitingPendingSave", true); + QSignalSpy appearedSpy(&backend, &Backend::externalFileAppeared); + backend.save(); + QCOMPARE(appearedSpy.count(), 1); + QCOMPARE(window->property("pendingAction").toString(), QString()); + QVERIFY(!window->property("awaitingPendingSave").toBool()); + + // Otherwise the next successful save -- this one, minutes later and + // asked for on its own -- closes the window on the earlier request. + backend.keepExternalVersion(); + backend.save(); + QVERIFY(!backend.modified()); + QVERIFY(!window->property("closeConfirmed").toBool()); + } + void putsKeepMineForwardWhenAFileAppeared() { const QString dialogPath = QFINDTESTDATA("../src/ExternalChangeDialog.qml"); QVERIFY(!dialogPath.isEmpty()); From df3274cc81b531e22f62bd6df73886b806eec4d8 Mon Sep 17 00:00:00 2001 From: Omabot Date: Tue, 25 Aug 2026 05:33:20 -0700 Subject: [PATCH 7/8] Carry the never-read path through a recovered snapshot A snapshot names a path and holds the text; restoring it reads whatever is on that path now and takes that for the baseline. For a path this document had never read, what is there now can be a file that arrived while Omawrite was not running, and clearing m_pathNeverRead on the strength of that read told the first save it had already looked. Ctrl+S then replaced the arrived file without asking -- the same loss the guard was added for, reached over a crash instead of over a `git pull`. So the snapshot carries the flag, and only the snapshot can answer the question. One written before this key existed reads as false, which is what it meant: before the branch, nothing could hold a name it had not written to. The unreadable branch keeps setting it true rather than consulting the key. A path that cannot be read is unverified whatever the snapshot said about it. Co-Authored-By: Claude Opus 5 (1M context) Co-Authored-By: Codex XHigh --- src/backend.cpp | 7 ++++- tests/tst_omawrite.cpp | 58 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/backend.cpp b/src/backend.cpp index c3b1218..d37e985 100644 --- a/src/backend.cpp +++ b/src/backend.cpp @@ -592,6 +592,7 @@ void Backend::writeRecovery() { if (!file.open(QIODevice::WriteOnly)) return; const QJsonObject recovery{{QStringLiteral("fileUrl"), m_fileUrl.toString()}, + {QStringLiteral("pathNeverRead"), m_pathNeverRead}, {QStringLiteral("text"), currentDocumentText()}}; file.write(QJsonDocument(recovery).toJson(QJsonDocument::Compact)); file.commit(); @@ -611,7 +612,11 @@ void Backend::restoreRecovery() { if (recoveredUrl.isLocalFile() && diskFile.open(QIODevice::ReadOnly)) { m_lastKnownFileContents = diskFile.readAll(); m_hasKnownFileContents = true; - m_pathNeverRead = false; + // Reading it now says what is on the path, not that this document ever + // looked: the file can have arrived while Omawrite was gone. Only the + // snapshot knows, so a snapshot without the key predates the flag and + // names a path something was written to. + m_pathNeverRead = recovery.value(QStringLiteral("pathNeverRead")).toBool(); } else { m_lastKnownFileContents.clear(); m_hasKnownFileContents = false; diff --git a/tests/tst_omawrite.cpp b/tests/tst_omawrite.cpp index da6093d..a0eae14 100644 --- a/tests/tst_omawrite.cpp +++ b/tests/tst_omawrite.cpp @@ -480,6 +480,64 @@ private slots: intact.close(); } + void remembersANeverReadPathAcrossRecovery() { + QTemporaryDir homeDirectory; + QVERIFY(homeDirectory.isValid()); + const QByteArray originalHome = qgetenv("HOME"); + struct HomeRestorer { + QByteArray value; + ~HomeRestorer() { qputenv("HOME", value); } + } restoreHome{originalHome}; + QVERIFY(qputenv("HOME", homeDirectory.path().toUtf8())); + + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const QString path = directory.filePath(QStringLiteral("fresh.md")); + + // The snapshot a crash leaves behind, for a new file whose first save + // never happened. + const QString stateDirectory = + QStandardPaths::writableLocation(QStandardPaths::AppDataLocation); + QVERIFY(QDir().mkpath(stateDirectory)); + QFile snapshot(QDir(stateDirectory).filePath(QStringLiteral("recovery-0.json"))); + QVERIFY(snapshot.open(QIODevice::WriteOnly)); + const QJsonObject recovery{ + {QStringLiteral("fileUrl"), QUrl::fromLocalFile(path).toString()}, + {QStringLiteral("pathNeverRead"), true}, + {QStringLiteral("text"), QStringLiteral("words only I have")}}; + snapshot.write(QJsonDocument(recovery).toJson(QJsonDocument::Compact)); + snapshot.close(); + + // A file turns up on the path while Omawrite is not running to see it. + QFile arrived(path); + QVERIFY(arrived.open(QIODevice::WriteOnly | QIODevice::Text)); + arrived.write("arrived while we were down"); + arrived.close(); + + // Reading it back on restore says what is on the path now, which is + // not the same as this document having read it. Without the flag the + // first save takes the guard's silence for permission. + const QString mainQmlPath = QFINDTESTDATA("../src/Main.qml"); + QVERIFY(!mainQmlPath.isEmpty()); + + Backend backend; + QQmlEngine engine; + engine.rootContext()->setContextProperty(QStringLiteral("backend"), &backend); + QQmlComponent component(&engine, QUrl::fromLocalFile(mainQmlPath)); + QVERIFY2(component.isReady(), qPrintable(component.errorString())); + QScopedPointer window(component.create()); + QVERIFY2(window, qPrintable(component.errorString())); + + QCOMPARE(backend.status(), QStringLiteral("Recovered unsaved changes")); + QSignalSpy appearedSpy(&backend, &Backend::externalFileAppeared); + backend.save(); + QCOMPARE(appearedSpy.count(), 1); + QFile untouched(path); + QVERIFY(untouched.open(QIODevice::ReadOnly | QIODevice::Text)); + QCOMPARE(untouched.readAll(), QByteArray("arrived while we were down")); + untouched.close(); + } + void savesAndOpensFromFooterButtons() { const QString mainQmlPath = QFINDTESTDATA("../src/Main.qml"); QVERIFY(!mainQmlPath.isEmpty()); From 6bc009287933d388913c0af8b2a2e8225192e0b2 Mon Sep 17 00:00:00 2001 From: Omabot Date: Fri, 28 Aug 2026 05:39:59 -0700 Subject: [PATCH 8/8] Cover the write half of the never-read snapshot round trip `remembersANeverReadPathAcrossRecovery` hand-writes the recovery JSON, so it exercises `restoreRecovery()` and nothing else. Deleting the `pathNeverRead` key from `writeRecovery()` leaves the whole suite green while the data-loss path it was added for reopens: crash on a never-read path, let something create the file while Omawrite is down, and the first save after the restart replaces it without asking, because a snapshot missing the key restores as false. The new test drives the real writer -- open an absent path, type, wait for the recovery timer -- and reads the file back. Removing the key fails it, and so does writing a constant false; nothing else in the suite moves either way. Co-Authored-By: Claude Opus 5 (1M context) Co-Authored-By: Codex XHigh --- tests/tst_omawrite.cpp | 49 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/tst_omawrite.cpp b/tests/tst_omawrite.cpp index a0eae14..a2e9d01 100644 --- a/tests/tst_omawrite.cpp +++ b/tests/tst_omawrite.cpp @@ -480,6 +480,55 @@ private slots: intact.close(); } + void writesTheNeverReadPathIntoTheSnapshot() { + QTemporaryDir homeDirectory; + QVERIFY(homeDirectory.isValid()); + const QByteArray originalHome = qgetenv("HOME"); + struct HomeRestorer { + QByteArray value; + ~HomeRestorer() { qputenv("HOME", value); } + } restoreHome{originalHome}; + QVERIFY(qputenv("HOME", homeDirectory.path().toUtf8())); + + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const QString path = directory.filePath(QStringLiteral("fresh.md")); + + const QString mainQmlPath = QFINDTESTDATA("../src/Main.qml"); + QVERIFY(!mainQmlPath.isEmpty()); + + Backend backend; + QQmlEngine engine; + engine.rootContext()->setContextProperty(QStringLiteral("backend"), &backend); + QQmlComponent component(&engine, QUrl::fromLocalFile(mainQmlPath)); + QVERIFY2(component.isReady(), qPrintable(component.errorString())); + QScopedPointer window(component.create()); + QVERIFY2(window, qPrintable(component.errorString())); + + QObject *editor = window->findChild(QStringLiteral("sourceEditor")); + QVERIFY(editor); + + backend.open(QUrl::fromLocalFile(path)); + editor->setProperty("text", QStringLiteral("words only I have")); + QVERIFY(backend.modified()); + + // The snapshot the next run reads is the one this run wrote, so the + // flag has to survive the write as well as the read. Hand-writing the + // JSON proves only half of that, and it is the half that cannot lose + // a file. + const QString snapshotPath = + QDir(QStandardPaths::writableLocation(QStandardPaths::AppDataLocation)) + .filePath(QStringLiteral("recovery-0.json")); + QTRY_VERIFY(QFile::exists(snapshotPath)); + + QFile snapshot(snapshotPath); + QVERIFY(snapshot.open(QIODevice::ReadOnly)); + const QJsonObject recovery = QJsonDocument::fromJson(snapshot.readAll()).object(); + snapshot.close(); + QVERIFY(recovery.contains(QStringLiteral("pathNeverRead"))); + QVERIFY(recovery.value(QStringLiteral("pathNeverRead")).toBool()); + } + void remembersANeverReadPathAcrossRecovery() { QTemporaryDir homeDirectory; QVERIFY(homeDirectory.isValid());