From 01ea9e9d7df04dc771430e4c378202b8ef37b8db Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Tue, 25 Aug 2026 09:54:10 +0200 Subject: feat(compose): report autosave state in a status bar Autosave worked and said nothing on success. The only feedback was m_banner, which is the failure channel and whose persistence is load-bearing for the quit path, so success got its own channel rather than sharing one. The fix is a funnel, not a label. m_dirty had seven writers, four of which clear it and only two of those are a save: the constructor clears it because seeding is not an edit, and the send handler clears it because the message is gone. A cue hung off saveDraftNow() would have been silently wrong in both. setDirty() is the only writer now, and it refreshes the status cue and setWindowModified() together so neither display can drift from the flag. The age line needs a tick of its own, since it moves with no edit to drive it. Five seconds against a label that reads in tens of them. Two defects found by probing rather than by reading. The %n plural rendered as "2 minute(s) ago" for every English user, because Qt picks a plural form only when a translation supplies the forms and there is no English .ts; it uses %1 and "min" now, which Italian substitutes identically. And the status mark was inside the translatable string, where a translator could drop it; it is concatenated outside tr(). Presentation reworked after the user looked at it. The first version reused item 151's yellow ribbon treatment, which reads as a misplaced widget on a bare status label rather than as a warning, and put both labels in the permanent widget area, which is the right-hand tray. They are ordinary status text on the left now. onlyTheSetterWritesTheDirtyFlag() asserts the funnel structurally, by reading composewindow.cpp: the first test for the send path called markClean() directly and a mutation restoring a direct assignment left the whole suite green. Four mutations now fail. The suite still cannot see the presentation, which is why that half needed a hand test. lrelease reports 487 finished, 0 unfinished. Closes item 160, and unblocks 161. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8 --- CHANGELOG.md | 9 + .../2026-08-03-post-0.1.0-usability-closed.md | 96 ++++++++++ .../plans/2026-08-03-post-0.1.0-usability.md | 53 +----- src/composewindow.cpp | 119 ++++++++++++- src/composewindow.h | 38 ++++ tests/CMakeLists.txt | 6 + tests/test_composewindow.cpp | 196 +++++++++++++++++++++ translations/qtmaildir_it_IT.ts | 20 ++- 8 files changed, 481 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 966297f..61a93c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,15 @@ point at which they are stable. ## [Unreleased] +### Added + +- **The composer says when it saved a draft.** A status bar reports the age of + the last autosave, and an `unsaved content` cue sits beside it whenever + there is text newer than that save. The window title carries the platform's + own modified marker for the same state, so it is visible while the composer + sits behind another window. Autosaving already worked; it was silent on + success, and the only feedback was the banner that reports a FAILED save. + ## [0.27.0] - 2026-08-25 qtmaildir can write mail. A composer, markdown bodies sent as plain text or diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md index 7a52984..41a7a1b 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md @@ -7613,3 +7613,99 @@ second test that would have restated it. Mutation-checked: reverting Suite 37 of 38; the failure is `undoMovesTheMessageBack`, item 136, pre-existing and on an unrelated path. + + + +## 160. The composer never says a draft was autosaved + +**Observed (user, 2026-08-25):** "we should add a status bar to the compose +window, to report every time a draft is autosaved. With a timer like 'last +autosave 20s ago' progressing into 'Draft autosaved' and next to it a visually +highlighted notification 'unsaved content' (blinking, color yellow, something +eye catching) whenever there's new content since the last autosave." + +**Cause (verified in code):** autosave is built and works. `m_autosaveTimer` +is a single-shot timer restarted on every edit (`composewindow.cpp:960-963`), +so it fires once the user pauses rather than once per character, and +`saveDraftNow()` writes the Maildir file. It reports **nothing on success**. + +The only feedback that exists is `m_banner`, and it is deliberately a FAILURE +channel: the comment at `composewindow.cpp:1026` records the reasoning, "a +PERSISTENT banner, not a modal and not a status-bar line that fades", because +a failed save must survive until it is dealt with. Success is the opposite +case and wants the fading line that comment rejected for failure. + +**Both states already exist as data**, which is what makes this small: + +- `m_dirty` is true exactly when there is content newer than the last save. +- `m_savedFingerprint` and the `draftSaved` signal mark a successful write. + +Nothing displays either. There is no `QStatusBar` on the composer at all. + +**Built 2026-08-25.** A `QStatusBar` on the composer, ticking every five +seconds against a label that reads in tens of them. + +**The fix is a funnel, not a label, and that is the whole item.** `m_dirty` +had SEVEN writers. Four of them CLEAR it and only two of those are a save: the +constructor clears it because seeding is not an edit, and the send handler +clears it because the message is gone. A cue hung off `saveDraftNow()` would +have been silently wrong in both, which is the shape of every defect items 105 +to 109 recorded. `setDirty()` is the only writer now; it refreshes the status +cue and calls `setWindowModified()`, so neither display can drift from the +flag. `markClean()` is the send path's entry to it. + +**Two cues, deliberately.** The status label is what the user reads while +typing; the title marker is what they see when the composer is behind another +window. Qt substitutes the `[*]` placeholder with the platform's own +convention, so the title half is native rather than invented. + +**The presentation was wrong first, and only looking found it.** It shipped +reusing item 151's yellow ground, border and text, on the reasoning that a +warning should look like the message pane's warnings. It should not: those are +bars spanning the pane and have something to be a ground OF, while the same +treatment on a bare status label reads as a misplaced widget, which is exactly +what the user reported. The cue is ordinary status text with a `โ—‹` mark now. +The first version also put both labels in the PERMANENT widget area, which is +the right-hand tray; `addWidget` is the left, which is where they belong. + +**Two defects found by probing, neither visible by reading.** + +- **The `%n` plural rendered as `2 minute(s) ago` for every English user.** + Qt picks a plural form only when a TRANSLATION supplies the forms, and there + is no English `.ts`, so an untranslated `%n` string falls back to its source + text with the `(s)` intact. Replaced with `%1` and "min", which Italian + substitutes identically. Same family as the `tr()` traps in `CLAUDE.md`: the + source reads correctly and the runtime does not. +- **The `โ—‹` was inside the translatable string** at first, so a translator + could drop or mangle it. Concatenated outside `tr()` now. + +**Testing, and one probe that measured nothing.** + +`aSentMessageLeavesNoUnsavedCue` first called `markClean()` directly. A +mutation putting `m_dirty = false` back into the send handler left all fifteen +tests GREEN, measured: the test proved what the setter does and nothing about +whether the send path calls it, which is `CLAUDE.md`'s "a probe pointed at the +wrong object". The property is structural and no runtime probe can see it, so +`onlyTheSetterWritesTheDirtyFlag()` reads `composewindow.cpp` and fails naming +the offending line. It carries a guard asserting `m_dirty` still exists, so a +rename makes it fail rather than quietly verify nothing. + +Four mutations now fail: dropping `setWindowModified`, bypassing the setter in +the send path, not recording the save time, and never starting the age tick. + +**The suite cannot see the presentation.** The tests assert visibility and +text, not styling, so the restyle left them green. That is correct and it is +also the limit: the yellow-chip version passed everything. It was fixed +because the user looked at it. + +**Constraints.** + +- **Do not reuse `m_banner`.** Its persistence is load-bearing for the quit + path's honesty, and a success message that shares it would either fade the + failure away or make success sticky. +- The "blinking" the note asks for should be treated as "eye-catching", not + literally: a blinking widget is an accessibility problem and Qt has no + blink facility to reach for. A yellow ground matching item 151's warning + severity is the established vocabulary here. +- `saveDraftNow()` returns false on failure and the banner takes over, so the + status line must not claim a save the write did not make. diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index 755caa5..ab11c4f 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -232,8 +232,8 @@ taking that too literally. | 159 | The Drafts view lists threads, so a draft is unreachable by double-click | defect | S | **done** 2026-08-25, unreleased. Reverses item 138's own decision, confirmed with the user. `generatorIsFlat()` in `config.cpp` is now the single closed set of flat generators, replacing three hardcoded comparisons against `"sent"`: the built-in filter, the reader that reapplies the mode, and the writer that skips storing what the generator implies. Those three had to agree and nothing made them; a `drafts` entry saved and reloaded would otherwise have come back THREADED while the button was flat. `builtinFilter()` sets `flat` once from the helper rather than in a branch, so the set cannot drift from the labels | -| 160 | The composer never says a draft was autosaved | feedback | S | open, 2026-08-25, from the notes. Autosave works (`m_autosaveTimer`, `saveDraftNow()`) and is SILENT on success: the only feedback is `m_banner`, which appears on FAILURE. The user asks for a status bar reporting "last autosave 20s ago" progressing to "Draft autosaved", plus an eye-catching "unsaved content" cue whenever `m_dirty` is true. Both states already exist as data; nothing displays them | -| 161 | The composer has no menu bar | discoverability | S | open, 2026-08-25, from the notes. Save draft on `Ctrl+S` is asked for and does not exist at all: `saveDraftNow()` is reachable only from the timer, Send and `closeEvent`. The composer's actions are ad-hoc `QAction`s parented to the window (Send, Close, and the formatting toolbar's), none registered in `KeyMap`, so item 132's menu-reachability rule does not currently reach them. Depends on 160 for what "saved" then reports | +| 160 | The composer never says a draft was autosaved | feedback | S | **done** 2026-08-25, unreleased. A status bar on the composer: the age line left, the `โ—‹ unsaved content` cue beside it. **The fix is a funnel, not a label.** `m_dirty` had SEVEN writers and four of them clear it, only two of which are a save, so a cue hung off the save path silently missed the constructor and the send; `setDirty()` is the one writer now and refreshes both cues plus `setWindowModified()`. Presentation was **reworked after the user looked at it**: it first reused item 151's yellow ribbon treatment, which reads as a misplaced widget on a bare status label, and the cue sat in the permanent (right-hand) tray. Two defects found by probing rather than by reading, see the section | +| 161 | The composer has no menu bar | discoverability | S | open, 2026-08-25, from the notes. Save draft on `Ctrl+S` is asked for and does not exist at all: `saveDraftNow()` is reachable only from the timer, Send and `closeEvent`. The composer's actions are ad-hoc `QAction`s parented to the window (Send, Close, and the formatting toolbar's), none registered in `KeyMap`, so item 132's menu-reachability rule does not currently reach them. **Unblocked** 2026-08-25: 160 shipped the status bar, so a manual save now has somewhere to report. Still needs the user to say WHICH main-window actions belong on a composer menu, since most message actions are meaningless over a message being written | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1337,50 +1337,6 @@ The 70-second duration recorded above fits a `QTRY_*` waiting for a file that is never going to appear, which is consistent with a wrong destination rather than a slow one. -## 160. The composer never says a draft was autosaved - -**Observed (user, 2026-08-25):** "we should add a status bar to the compose -window, to report every time a draft is autosaved. With a timer like 'last -autosave 20s ago' progressing into 'Draft autosaved' and next to it a visually -highlighted notification 'unsaved content' (blinking, color yellow, something -eye catching) whenever there's new content since the last autosave." - -**Cause (verified in code):** autosave is built and works. `m_autosaveTimer` -is a single-shot timer restarted on every edit (`composewindow.cpp:960-963`), -so it fires once the user pauses rather than once per character, and -`saveDraftNow()` writes the Maildir file. It reports **nothing on success**. - -The only feedback that exists is `m_banner`, and it is deliberately a FAILURE -channel: the comment at `composewindow.cpp:1026` records the reasoning, "a -PERSISTENT banner, not a modal and not a status-bar line that fades", because -a failed save must survive until it is dealt with. Success is the opposite -case and wants the fading line that comment rejected for failure. - -**Both states already exist as data**, which is what makes this small: - -- `m_dirty` is true exactly when there is content newer than the last save. -- `m_savedFingerprint` and the `draftSaved` signal mark a successful write. - -Nothing displays either. There is no `QStatusBar` on the composer at all. - -**Approach.** A `QStatusBar` on the composer, with the age line as a permanent -widget and the unsaved cue beside it. The age needs a second timer of its own, -since "20s ago" changes with no edit to drive it; a one-second tick is wasteful -for a label that reads in tens of seconds, so tick slower and accept the -granularity. - -**Constraints.** - -- **Do not reuse `m_banner`.** Its persistence is load-bearing for the quit - path's honesty, and a success message that shares it would either fade the - failure away or make success sticky. -- The "blinking" the note asks for should be treated as "eye-catching", not - literally: a blinking widget is an accessibility problem and Qt has no - blink facility to reach for. A yellow ground matching item 151's warning - severity is the established vocabulary here. -- `saveDraftNow()` returns false on failure and the banner takes over, so the - status line must not claim a save the write did not make. - ## 161. The composer has no menu bar **Observed (user, 2026-08-25):** "the compose window should have a menu bar on @@ -1417,6 +1373,7 @@ new action and the existing ones gathered under it rather than duplicated. - Save draft must go through `saveDraftNow()`, which already handles the failure banner and emits `draftSaved` for the indexing item 158 added. A second write path would reintroduce the ghost-file problem that fixed. -- **Depends on item 160** for what a manual save then reports: with no status - line, a successful `Ctrl+S` would be as silent as an autosave is now. +- **Item 160 unblocked this** on 2026-08-25: the status bar exists, so a + manual save reports through `refreshDraftStatus()` like an autosave. Route + `Ctrl+S` through `saveDraftNow()` and the reporting is already done. diff --git a/src/composewindow.cpp b/src/composewindow.cpp index c35bb5d..2b0b9d8 100644 --- a/src/composewindow.cpp +++ b/src/composewindow.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include #include #include @@ -109,7 +110,9 @@ ComposeWindow::ComposeWindow(const ComposeContext &context, // parent still makes Qt treat it as a window because of Qt::Window, which // QMainWindow carries. setAttribute(Qt::WA_DeleteOnClose); - setWindowTitle(tr("Compose")); + // The [*] is Qt's placeholder for the modified marker, substituted with + // the platform's own convention by setWindowModified(). + setWindowTitle(tr("Compose[*]")); // A sensible default. NOT restored and NOT saved; see the header. resize(760, 640); @@ -150,7 +153,7 @@ ComposeWindow::ComposeWindow(const ComposeContext &context, // either, must show what the message is addressed to. revealCcBccIfUsed(); - m_dirty = false; + setDirty(false); m_autosaveTimer->stop(); // The body, whenever there is already a recipient: a Reply or a Forward @@ -406,6 +409,8 @@ void ComposeWindow::buildUi() for (QLineEdit *field : { m_to, m_cc, m_bcc, m_subject }) connect(field, &QLineEdit::textChanged, this, &ComposeWindow::markDirty); connect(m_sendHtml, &QCheckBox::toggled, this, &ComposeWindow::markDirty); + + buildDraftStatusBar(); connect(m_from, &QComboBox::currentIndexChanged, this, [this]() { markDirty(); // The account SEEDS the signature, so a change to it re-seeds. It @@ -953,9 +958,104 @@ void ComposeWindow::applyFormat(const QString &token) cursor.selectionEnd(), token)); } +void ComposeWindow::buildDraftStatusBar() +{ + // The success channel item 160 added. Deliberately NOT the banner: that + // one reports FAILURE and its persistence is load-bearing, since the quit + // path's honesty depends on a failed save still being visible. + QStatusBar *bar = statusBar(); + + m_draftAge = new QLabel(bar); + m_draftAge->setObjectName(QStringLiteral("draftAge")); + m_draftAge->setTextFormat(Qt::PlainText); + + // A ring, not a chip. Item 151's yellow ground belongs to the message + // pane's ribbons, which are bars spanning the pane and have something to + // be a ground OF; the same treatment on a bare status label reads as a + // misplaced widget rather than as a warning, which is what the user saw. + // Ordinary status text with a mark carries the same meaning quietly. + // The mark is NOT part of the translatable string: a translator cannot + // drop or mangle what they are never handed. + m_unsavedCue = new QLabel( + QStringLiteral("%1 %2").arg(QChar(0x25CB), tr("unsaved content")), bar); + m_unsavedCue->setObjectName(QStringLiteral("unsavedCue")); + m_unsavedCue->setTextFormat(Qt::PlainText); + m_unsavedCue->hide(); + + // addWidget, not addPermanentWidget: the permanent tray is the RIGHT hand + // end, and both of these belong on the left, the age first with the cue + // beside it. + bar->addWidget(m_draftAge); + bar->addWidget(m_unsavedCue); + + // The age moves with no edit to drive it, so it needs a tick of its own. + // Five seconds against a label that reads in tens of them: a per-second + // tick would wake the window sixty times a minute to redraw the same + // string. + m_draftAgeTick = new QTimer(this); + m_draftAgeTick->setObjectName(QStringLiteral("draftAgeTick")); + m_draftAgeTick->setInterval(5000); + connect(m_draftAgeTick, &QTimer::timeout, this, + &ComposeWindow::refreshDraftStatus); + m_draftAgeTick->start(); +} + +void ComposeWindow::setDirty(bool dirty) +{ + m_dirty = dirty; + + // Both cues, from the one flag. The status label is what the user reads + // while typing; the title marker is what they see when the composer sits + // behind another window. Qt substitutes the [*] placeholder in the title + // with the platform's own convention, so this is the native gesture + // rather than an invented one. + if (m_unsavedCue) + m_unsavedCue->setVisible(dirty); + setWindowModified(dirty); +} + +void ComposeWindow::markClean() +{ + // No draft is written: the message has been sent, so there is nothing + // left to save. Only the state and its two displays move. + setDirty(false); +} + +void ComposeWindow::refreshDraftStatus() +{ + if (!m_draftAge) + return; + if (!m_lastSavedAt.isValid()) { + m_draftAge->clear(); + return; + } + reportDraftAgeFor(m_lastSavedAt.secsTo(QDateTime::currentDateTime())); +} + +void ComposeWindow::reportDraftAgeFor(qint64 seconds) +{ + if (!m_draftAge) + return; + + // Coarse on purpose. The label reads in tens of seconds and the tick is + // slower than a second, so a precise count would advertise an accuracy + // the refresh does not have. + if (seconds < 10) + m_draftAge->setText(tr("Draft autosaved")); + else if (seconds < 60) + m_draftAge->setText(tr("Last autosave %1s ago").arg(seconds)); + else + // Minutes as a number rather than as a %n plural. There is no English + // .ts, so an untranslated %n string falls back to its SOURCE text and + // renders literally as "2 minute(s) ago" for every English user; + // measured. Italian keeps its own plural forms through this same + // string, since %1 is substituted either way. + m_draftAge->setText(tr("Last autosave %1 min ago").arg(seconds / 60)); +} + void ComposeWindow::markDirty() { - m_dirty = true; + setDirty(true); // Debounced: the timer restarts on every keystroke, so a write happens // once the user has paused, not once per character. Every autosave // produces a Maildir write that mbsync uploads, which is what the debounce @@ -996,7 +1096,7 @@ bool ComposeWindow::saveDraftNow() // blocking build entirely for the no-change case, which is the common one. const QString fingerprint = fingerprintOf(message); if (!m_savedFingerprint.isEmpty() && fingerprint == m_savedFingerprint) { - m_dirty = false; + setDirty(false); return true; } @@ -1036,10 +1136,17 @@ bool ComposeWindow::saveDraftNow() m_draftPath = written.path; m_savedFingerprint = fingerprint; - m_dirty = false; + setDirty(false); m_saveFailed = false; m_banner->hide(); + // The success half of item 160: an autosave used to be entirely silent, + // so the only sign a draft had been written was the file appearing in the + // Drafts view. The banner is deliberately NOT reused; it is the failure + // channel and its persistence is load-bearing for the quit path. + m_lastSavedAt = QDateTime::currentDateTime(); + refreshDraftStatus(); + // The write is done and the previous revision already unlinked; hand both // paths up so the owner indexes the new one and drops the old (item 158). emit draftSaved(written.path, previousPath); @@ -1233,7 +1340,7 @@ void ComposeWindow::send() // twice. m_finished stops closeEvent() saving a draft for a // message that is gone, and stops it refusing the close. m_finished = true; - m_dirty = false; + markClean(); close(); }, Qt::SingleShotConnection); diff --git a/src/composewindow.h b/src/composewindow.h index caff011..dc7f3c0 100644 --- a/src/composewindow.h +++ b/src/composewindow.h @@ -18,6 +18,7 @@ #pragma once +#include #include #include @@ -96,6 +97,15 @@ public: /// and quitting therefore loses that text. bool lastSaveFailed() const { return m_saveFailed; } + /// Clear the unsaved-edits state without writing a draft. The send path + /// needs this: the message is gone, so there is nothing left to save and + /// nothing to warn about on the way out. + void markClean(); + + /// Render the age line as though the last save were \p seconds ago. + /// Exists so a test can drive the clock instead of waiting on it. + void reportDraftAgeFor(qint64 seconds); + /// Where the signature files live. Defaults to /// /qtmaildir/signatures; a test points it at its own directory. /// @@ -217,6 +227,17 @@ private: void showSendFailure(const QString &stderrText); void applyEdit(const MarkdownFormat::Edit &edit); void markDirty(); + + /// Builds the status bar carrying the unsaved cue and the age line. + void buildDraftStatusBar(); + + /// The one writer of m_dirty. Refreshes the status cue and the title + /// marker so neither can drift from the flag. + void setDirty(bool dirty); + + /// Repaint the age line from m_lastSavedAt. Called by the tick and after + /// a save. + void refreshDraftStatus(); void autosave(); void send(); void applyFormat(const QString &token); @@ -272,6 +293,18 @@ private: /// how send_html seeds from context and is then left alone. bool m_signatureChosen = false; QLabel *m_banner = nullptr; + + /// The status bar's two halves. The cue answers "is there anything + /// unwritten"; the age answers "when did the last write happen". Both + /// are refreshed from setDirty()/refreshDraftStatus() and never assigned + /// directly, so they cannot disagree with m_dirty. + QLabel *m_unsavedCue = nullptr; + QLabel *m_draftAge = nullptr; + QTimer *m_draftAgeTick = nullptr; + + /// When the last successful save happened, invalid until one has. Drives + /// the age line, which changes with no edit to prompt it. + QDateTime m_lastSavedAt; QListWidget *m_attachmentList = nullptr; QWidget *m_sendLogPane = nullptr; QPlainTextEdit *m_sendLog = nullptr; @@ -295,6 +328,11 @@ private: /// on the bytes can never fire. It would read as working while writing a /// file, and an mbsync upload, on every debounce. QString m_savedFingerprint; + + /// Never assigned directly outside setDirty(). Seven sites used to write + /// it and four of them clear it, only two of which are a save, so a cue + /// hung off the save path alone missed the constructor and the send. The + /// setter is what keeps the flag and both displays in step. bool m_dirty = false; bool m_saveFailed = false; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 5938aeb..830e2b2 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -85,6 +85,12 @@ add_qtmaildir_test(translations) target_compile_definitions(test_translations PRIVATE TRANSLATIONS_DIR="${CMAKE_SOURCE_DIR}/translations") +# One test reads composewindow.cpp itself, to assert that m_dirty has a single +# writer. The property is structural, so no runtime probe can see it: a +# mutation restoring a direct assignment in the send path left the suite green. +target_compile_definitions(test_composewindow PRIVATE + SOURCE_DIR="${CMAKE_SOURCE_DIR}") + # The notmuch hooks (assets/hooks/), which are Python rather than C++ and are # therefore registered directly rather than through add_qtmaildir_test(). # diff --git a/tests/test_composewindow.cpp b/tests/test_composewindow.cpp index 472c103..68ec4d4 100644 --- a/tests/test_composewindow.cpp +++ b/tests/test_composewindow.cpp @@ -23,7 +23,10 @@ #include #include #include +#include +#include #include +#include #include #include @@ -49,6 +52,11 @@ private slots: void changingTheAccountStopsFollowingOnceTheSwitchIsUsed(); void aResumedDraftDoesNotReseedOnAnAccountChange(); void savingADraftEmitsItsPathAndTheReplacedOne(); + void anUnsavedEditIsAnnouncedInBothPlaces(); + void aSavedDraftReportsItAndClearsTheCue(); + void aSentMessageLeavesNoUnsavedCue(); + void onlyTheSetterWritesTheDirtyFlag(); + void theAgeLineFollowsTheClock(); private: /// A config pointing at a signatures directory holding \p files, with one @@ -62,6 +70,9 @@ private: /// non-void function. A void helper keeps the check and sidesteps that. void writeFile(const QString &path, const QString &content); + /// A config whose one account has a drafts folder, so a save can write. + Config configWithDrafts(); + QTemporaryDir *m_dir = nullptr; QString m_signatureDir; }; @@ -447,5 +458,190 @@ void TestComposeWindow::savingADraftEmitsItsPathAndTheReplacedOne() QVERIFY2(second != first, "a rewrite reused the old filename"); } +/// A helper for the status-bar tests: a config whose account has a drafts +/// folder, which makeConfig() deliberately does not set. +Config TestComposeWindow::configWithDrafts() +{ + const QString confPath = m_dir->path() + QStringLiteral("/qtmaildir.conf"); + QString conf; + { + QTextStream out(&conf); + out << "[account.work]\n" + << "name = Someone\n" + << "address = someone@example.org\n" + << "maildir = work\n" + << "drafts = Drafts\n" + << "send_command = /bin/cat\n"; + } + writeFile(confPath, conf); + + Config config; + config.load(confPath); + return config; +} + +/// Both cues answer the same question and must agree. The status label is +/// what the user reads while typing; the title marker is what they see when +/// the composer is behind another window. +void TestComposeWindow::anUnsavedEditIsAnnouncedInBothPlaces() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + + // Seeding is not an edit: the constructor clears the flag after filling + // the fields, so a composer nobody has typed into is clean. + auto *unsaved = window.findChild(QStringLiteral("unsavedCue")); + QVERIFY(unsaved); + QVERIFY2(unsaved->isHidden(), "a freshly opened composer is not dirty"); + QVERIFY2(!window.isWindowModified(), + "a freshly opened composer must not claim unsaved edits"); + + auto *body = window.findChild(QStringLiteral("body")); + QVERIFY(body); + body->setPlainText(QStringLiteral("Something typed.")); + + QVERIFY2(!unsaved->isHidden(), "the status cue must appear on an edit"); + QVERIFY2(window.isWindowModified(), + "the title marker must appear on an edit"); +} + +/// The gap item 160 exists to close: a successful save said nothing at all. +void TestComposeWindow::aSavedDraftReportsItAndClearsTheCue() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + auto *body = window.findChild(QStringLiteral("body")); + QVERIFY(body); + auto *unsaved = window.findChild(QStringLiteral("unsavedCue")); + auto *age = window.findChild(QStringLiteral("draftAge")); + QVERIFY(unsaved); + QVERIFY(age); + + QVERIFY2(age->text().isEmpty(), + "nothing has been saved yet, so there is no age to report"); + + body->setPlainText(QStringLiteral("First revision.")); + QVERIFY(window.saveDraftNow()); + + QVERIFY2(unsaved->isHidden(), "a save must clear the unsaved cue"); + QVERIFY2(!window.isWindowModified(), + "a save must clear the title marker"); + QVERIFY2(!age->text().isEmpty(), "a save must be reported"); +} + +/// The send path clears the flag WITHOUT saving a draft, and it is one of the +/// four sites that write it. A cue hung off the save alone would leave a sent +/// message claiming unsaved edits on the way out. +void TestComposeWindow::aSentMessageLeavesNoUnsavedCue() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + auto *body = window.findChild(QStringLiteral("body")); + QVERIFY(body); + body->setPlainText(QStringLiteral("Outgoing.")); + QVERIFY(window.isWindowModified()); + + // What the send handler does on success, without running a real send. + window.markClean(); + + auto *unsaved = window.findChild(QStringLiteral("unsavedCue")); + QVERIFY(unsaved); + QVERIFY2(unsaved->isHidden(), "a sent message has no unsaved edits"); + QVERIFY2(!window.isWindowModified(), + "a sent message must not claim unsaved edits"); +} + +/// The test above drives markClean() directly, which proves what the SETTER +/// does and nothing about whether the send path calls it: a mutation putting +/// `m_dirty = false` back into the send handler left the whole suite green, +/// measured. That is CLAUDE.md's "a probe pointed at the wrong object". +/// +/// The property that actually matters is structural, so it is asserted +/// structurally: m_dirty has ONE writer. Four of the seven sites that used to +/// assign it clear it, and only two of those are a save, so a cue hung off +/// the save path alone silently missed the constructor and the send. +void TestComposeWindow::onlyTheSetterWritesTheDirtyFlag() +{ + QFile source(QStringLiteral(SOURCE_DIR "/src/composewindow.cpp")); + QVERIFY2(source.open(QIODevice::ReadOnly | QIODevice::Text), + qPrintable(source.errorString())); + const QStringList lines = + QString::fromUtf8(source.readAll()).split(QLatin1Char('\n')); + source.close(); + + // A guard against the probe itself rotting: if the member is ever + // renamed, this test must fail rather than quietly verify nothing. + QVERIFY2(lines.join(QLatin1Char('\n')).contains(QStringLiteral("m_dirty")), + "m_dirty is gone; this test needs updating, not deleting"); + + QStringList offenders; + for (int i = 0; i < lines.size(); ++i) { + const QString line = lines.at(i); + // An assignment, not a read: `m_dirty =` but not `m_dirty ==`. + static const QRegularExpression assignment( + QStringLiteral("\\bm_dirty\\s*=[^=]")); + if (!assignment.match(line).hasMatch()) + continue; + // The one legitimate writer. + if (line.contains(QStringLiteral("m_dirty = dirty"))) + continue; + offenders.append(QStringLiteral("%1: %2").arg(i + 1).arg(line.trimmed())); + } + + QVERIFY2(offenders.isEmpty(), + qPrintable(QStringLiteral( + "m_dirty must only be written by setDirty(), or the status " + "cue and the title marker drift from it. Offending lines:\n%1") + .arg(offenders.join(QLatin1Char('\n'))))); +} + +/// The age changes with no edit to drive it, so it needs a tick of its own. +/// Asserting on the TEXT changing rather than on a wording, which is +/// translated and would pin the test to one locale. +void TestComposeWindow::theAgeLineFollowsTheClock() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + auto *body = window.findChild(QStringLiteral("body")); + QVERIFY(body); + body->setPlainText(QStringLiteral("First revision.")); + QVERIFY(window.saveDraftNow()); + + auto *age = window.findChild(QStringLiteral("draftAge")); + QVERIFY(age); + const QString justSaved = age->text(); + QVERIFY(!justSaved.isEmpty()); + + // Driven rather than waited for: a real wait would put seconds into the + // suite for a label that reads in tens of them. + auto *tick = window.findChild(QStringLiteral("draftAgeTick")); + QVERIFY2(tick, "the age needs a tick of its own; an edit cannot drive it"); + QVERIFY(tick->isActive()); + + window.reportDraftAgeFor(90); + QVERIFY2(age->text() != justSaved, + "the age line must move as the clock does"); +} + QTEST_MAIN(TestComposeWindow) #include "test_composewindow.moc" diff --git a/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts index a9c0f88..5773461 100644 --- a/translations/qtmaildir_it_IT.ts +++ b/translations/qtmaildir_it_IT.ts @@ -4,8 +4,8 @@ ComposeWindow - Compose - Componi + Compose[*] + Componi[*] The forwarded attachments could not be extracted. @@ -122,6 +122,22 @@ '%1' is %2. Many mail servers refuse messages above about %3. Attach it anyway? '%1' occupa %2. Molti server di posta rifiutano messaggi oltre i %3 circa. Allegarlo comunque? + + unsaved content + contenuto non salvato + + + Draft autosaved + Bozza salvata + + + Last autosave %1s ago + Ultimo salvataggio %1s fa + + + Last autosave %1 min ago + Ultimo salvataggio %1 min fa + The draft could not be saved: %1 Non รจ stato possibile salvare la bozza: %1 -- cgit v1.2.3