From 95ae5dfe2df7858ad957b353dc0ae1d7af3b4832 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 21 Aug 2026 16:13:28 +0200 Subject: feat(compose): transform the markdown buffer for the toolbar, item 123 MarkdownFormat, task 8 of the compose-and-send plan. Three free functions over (text, selection start, selection end) returning the new text and the selection that follows it, so the grammar is tested without a widget. Three gaps in the plan's draft, each now pinned by a test checked against the mutation that breaks it: - QString::lastIndexOf INCLUDES the position it is given, so quoting with the cursor at the end of a line found that line's newline and quoted the FOLLOWING one. The draft's fixtures never placed a cursor there. - A backwards selection was normalised but never tested, so the swap was unguarded; a right-to-left drag is an ordinary gesture and Qt reports the anchor after the cursor. normalise() now swaps and clamps in one place. - A blank line inside a quoted range produced "> " with trailing whitespace, which editors and mail clients strip anyway. It is written bare. Two further defects came out of review: - quote()'s selectionStart was unasserted for any block not starting at line zero. Hardcoding it to 0 passed all nineteen tests, because the one test naming the property quoted the first line, where right and wrong coincide. A wrong selection there means a second press quotes a line the user never selected, and a following Bold bolds the wrong text. - A selection splitting a surrogate pair split the character across the inserted tokens, leaving invalid UTF-16. Not reachable from the toolbar, where arrow keys and mouse hit-testing both move in whole clusters, but reachable by any code computing a position arithmetically. normalise() nudges off a low surrogate; a collapsed cursor moves back on both ends, since widening would turn "insert an empty pair here" into "wrap the emoji". The buttons stack rather than toggle: a second Bold press gives ****this****, and a second Quote press nests. That is what the spec specifies, and the preserved selection exists so a second press can apply a SECOND token. A toggle was built during this task at the user's request and reverted on finding it contradicts the spec at two sites; it is recorded as backlog item 135, where the unanswered question is what replaces bold-then-italic. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LoaLBowZ6w1JNx6SEhDP1L --- .../plans/2026-08-03-post-0.1.0-usability.md | 73 +++++ src/CMakeLists.txt | 1 + src/formattoolbar.cpp | 182 +++++++++++ src/formattoolbar.h | 76 +++++ tests/CMakeLists.txt | 1 + tests/test_formattoolbar.cpp | 346 +++++++++++++++++++++ 6 files changed, 679 insertions(+) create mode 100644 src/formattoolbar.cpp create mode 100644 src/formattoolbar.h create mode 100644 tests/test_formattoolbar.cpp 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 01be5d0..e9f99a4 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 @@ -204,6 +204,7 @@ taking that too literally. | 132 | Every action must have a shortcut, and that no longer serves | policy | S | done, 2026-08-20. `everyActionHasAShortcut` is deleted and nothing replaces it: `everyActionIsReachableFromAMenu()` is the required rule and a shortcut is now a chosen subset. Nothing else needed changing, since `showShortcutReference()` already printed `(unbound)` for an empty sequence. Verified by unbinding `tag_rules` and running the suite green, which would have failed before | | 133 | The composer shows no markdown syntax highlighting | v2 | S | open, 2026-08-20, from the item 123 brainstorm. **Blocked on 123.** A `QSyntaxHighlighter` over the composer's editor, so `**bold**` reads as bold while the buffer stays plain markdown. Standard Qt, no dependency. Deliberately after 123's formatting toolbar: agreeing with the grammar about nesting and about code spans suppressing what is inside them is the expensive part, and the toolbar is what makes the feature usable | | 134 | The busy indicator is built inline and is about to be built twice | maintenance | S | done, 2026-08-20, af902e0. `BusyIndicator` (`src/busyindicator.h`) carries both modes: `MainWindow` uses the indeterminate one, and item 123's send popup takes the determinate half for its undo countdown, switching the same widget over when the command starts. Only the BAR was extracted, not the status label this row paired with it. `m_statusLabel` has 34 uses across `MainWindow` for transient messages, selection counts and sync phases, so it belongs to the window rather than to the indicator, and the send popup owns its own phase text | +| 135 | The formatting toolbar's buttons stack rather than toggle | v2 | S | open, 2026-08-21, asked for by the user during item 123 task 8 and reverted the same session. **A spec change, not a defect**: it conflicts with spec:236 ("deliberately no live toggle") and spec:187-190. Both sites need amending FIRST, and the amendment must resolve what replaces bold-then-italic, which is the gesture spec:187's preserved selection exists to serve and which a toggle makes unreachable. That question is the work; the state machine is understood and written up in the section | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1098,6 +1099,78 @@ Then Delete a message. Verified by hand on 2026-08-20; this is how it was found. **Size: S.** +## 135. The formatting toolbar's buttons stack rather than toggle + +**Observed (user, 2026-08-21):** pressing Bold a second time on already-bold +text adds another pair of asterisks rather than removing the first, so +`**this**` becomes `****this****`. Quote nests the same way: a second press on +`> one` gives `> > one`. The user asked for both to toggle. + +**A toggle was built and reverted the same session**, and the reason matters +more than the code: it was not unwanted, it **conflicts with the spec**, which +was not checked before the work started. + +- `2026-08-20-compose-and-send-design.md:236` states there is "deliberately no + live toggle that inserts and removes the quote while editing". +- `:187-190` is the complete statement of the wrap behaviour and describes only + wrapping, with no toggle anywhere. + +**Cause.** This is a **spec change, not a defect**, and both sites need +amending before any code is written again. + +Underneath sits a real design question the spec answers one way and a toggle +answers the other, which is why the two cannot simply coexist. `:187` preserves +the selection after a wrap **so that a second press applies a SECOND token** to +the same words: bold, then italic, without touching the mouse. A toggle makes +that gesture unreachable, because the second press now removes the first token +instead. **What replaces bold-then-italic is unanswered**, and answering it is +the substance of this item, not the state machine below. Possible directions, +none chosen: a modifier on the second press, a separate un-format action, or +accepting that the chord is lost and reaching nested emphasis by typing. + +**Approach.** When it is picked up, the transformation half is already +understood, so the notes below exist to stop it being rediscovered. A toggling +`wrap()` must distinguish three states, and a single "it unwraps" test passes +against most of them being broken: + +- **INSIDE** the tokens: `**this**` with `this` selected (2..6). The tokens sit + just outside the selection; the same characters stay selected afterwards. +- **AROUND** them: `**this**` selected whole (0..8). The selection shrinks to + the text that was between them. +- **PARTIALLY overlapping** one: `*this**` (6..13). Neither of the above. It + does not describe a wrapped span, and stripping would have to guess which + half of a token to keep, so wrapping is the predictable answer. + +**INSIDE must be checked before AROUND.** On `***this***` both tests match, and +only INSIDE removes the level the user actually asked for. + +**A naive adjacency test is wrong, and looks right.** Checking only whether the +characters either side of the selection equal the token means pressing *Italic* +on `**this**` finds a `*` on each side, strips one asterisk per side, and +**un-bolds text the user asked to italicise**. A strip must require the adjacent +RUN of token characters to be the token exactly, or the token plus one other +complete emphasis token: `***` is bold+italic and divisible either way, while a +run of two is one indivisible token whose half is not a token at all. This was +found by writing the italic-on-bold test, not by reading the code. + +The quote side is simpler but has one trap: a bare `>` is what the quote path +writes for a blank line, so an unquote that only recognises `"> "` leaves a +stray marker on every blank line in a round trip. Whether a mixed block (some +lines quoted, some not) quotes or unquotes is a decision; quoting it, so one +press makes the block uniform and the next unquotes it, avoids the button doing +two opposite things to two halves of one selection. + +**Constraints.** The spec amendment comes first and must resolve the +bold-then-italic question, or the same conflict recurs. `MarkdownFormat` is +painter-free and widget-free, so the whole state machine is unit-testable +without the composer; keep it that way. The toolbar shortcuts belong to the +composer window and do not touch `KeyMap`, so nothing here interacts with item +132. Note that toggling changes what the preserved selection is FOR, so +`wrappingTwiceNestsTheTokensAroundTheSameWords` and +`quotingAnAlreadyQuotedLineNestsIt` in `tests/test_formattoolbar.cpp` both +assert the current spec behaviour and would be replaced rather than extended. + + ## Deferred, unsized, or split out Items noted while triaging but not part of the original list. Same numbering diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a462ba3..501c276 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -16,6 +16,7 @@ add_library(qtmaildir_lib STATIC draftstore.cpp messagesender.cpp composecontext.cpp + formattoolbar.cpp tagchip.cpp tagcolors.cpp savequerydialog.cpp diff --git a/src/formattoolbar.cpp b/src/formattoolbar.cpp new file mode 100644 index 0000000..565d4af --- /dev/null +++ b/src/formattoolbar.cpp @@ -0,0 +1,182 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "formattoolbar.h" + +#include + + +namespace { + +/// Normalises the selection a widget reports into an ordered, in-range pair +/// that does not split a character. +/// +/// Three hazards, handled once rather than per function. A backwards drag +/// reports the anchor AFTER the cursor; a selection can outlive the edit that +/// shortened the buffer under it; and a boundary can land in the middle of a +/// surrogate pair, where inserting a token splits one character into two +/// halves and the result is not valid UTF-16 at all. +/// +/// The surrogate case is not reachable with an arrow key or the mouse, which +/// move in whole clusters, but QTextCursor::setPosition accepts such a +/// position, so any caller computing one arithmetically can produce it: a +/// draft restore, a find/replace, a template insertion. A boundary sitting on +/// a LOW surrogate is inside a pair, and moving it back by one puts it before +/// the whole character. +/// +/// A COLLAPSED cursor moves back, not outward: nudging the two ends in +/// opposite directions would turn an empty selection into a two-unit one and +/// wrap a character the user never selected. A real selection widens, so that +/// touching any part of a character covers the whole of it. +void normalise(const QString &text, int &from, int &to) +{ + from = qBound(0, from, int(text.size())); + to = qBound(0, to, int(text.size())); + if (from > to) + qSwap(from, to); + + const auto insidePair = [&text](int at) { + return at < text.size() && text.at(at).isLowSurrogate(); + }; + + if (from == to) { + if (insidePair(from)) { + --from; + to = from; + } + return; + } + + if (insidePair(from)) + --from; + if (insidePair(to)) + ++to; +} + +} // namespace + +MarkdownFormat::Edit MarkdownFormat::wrap(const QString &text, int start, + int end, const QString &token) +{ + Edit edit; + int from = start; + int to = end; + normalise(text, from, to); + + edit.text = text; + // The closing token first: inserting at `from` would shift `to`. + edit.text.insert(to, token); + edit.text.insert(from, token); + + if (from == to) { + // No selection: the cursor goes BETWEEN the two tokens so typing + // continues inside them. Landing after the closing token instead is + // the mistake a user notices on the first keystroke. + edit.selectionStart = from + token.size(); + edit.selectionEnd = edit.selectionStart; + } else { + // The selection is preserved so a second press applies a second token + // to the same words without reselecting: bold then italic. + edit.selectionStart = from + token.size(); + edit.selectionEnd = to + token.size(); + } + + return edit; +} + +MarkdownFormat::Edit MarkdownFormat::link(const QString &text, int start, int end) +{ + Edit edit; + int from = start; + int to = end; + normalise(text, from, to); + + const QString label = text.mid(from, to - from); + + edit.text = text; + edit.text.replace(from, to - from, QStringLiteral("[%1]()").arg(label)); + + if (label.isEmpty()) { + // Nothing selected: the label is what gets typed first, so the cursor + // goes inside the brackets, one past the '['. + edit.selectionStart = from + 1; + } else { + // The label is written; the URL is what remains, so the cursor goes + // inside the parentheses: past '[', the label, ']' and '('. + edit.selectionStart = from + label.size() + 3; + } + edit.selectionEnd = edit.selectionStart; + + return edit; +} + +MarkdownFormat::Edit MarkdownFormat::quote(const QString &text, int start, int end) +{ + Edit edit; + int from = start; + int to = end; + normalise(text, from, to); + + // Line-based, not a wrap. The selection is widened to whole lines first: + // quoting half a line produces markdown that means something else. + // + // The backwards search starts at `from - 1`, not at `from`. QString's + // lastIndexOf INCLUDES the position it is given, so a cursor sitting at + // the end of a line, immediately before its newline, would find that + // newline and quote the FOLLOWING line instead of the one the cursor is + // on. The guard against a negative position matters too, since -1 means + // "search from the end" and would find the last newline in the buffer. + const int firstLineStart = + from > 0 ? text.lastIndexOf(QLatin1Char('\n'), from - 1) + 1 : 0; + + // No newline after the last line, so the end of the text is the end of + // the block. Without this the whole tail would be dropped. + int lastLineEnd = text.indexOf(QLatin1Char('\n'), to); + if (lastLineEnd < 0) + lastLineEnd = text.size(); + + const QString before = text.left(firstLineStart); + const QString middle = text.mid(firstLineStart, lastLineEnd - firstLineStart); + const QString after = text.mid(lastLineEnd); + + const QStringList lines = middle.split(QLatin1Char('\n')); + + // Nesting rather than toggling, per the spec: a second press deepens the + // quote. There is deliberately no live toggle here, because tracking "my + // text" and "the quote" as separate pieces to make one reversible is + // machinery for a case the user answers by closing the composer. + QStringList result; + result.reserve(lines.size()); + for (const QString &line : lines) { + // A blank line keeps the marker, since that is what continues a quote + // block in markdown, but WITHOUT the trailing space: several editors + // and mail clients strip trailing whitespace, and stripping it from + // "> " leaves ">" anyway, so writing it bare is the same result + // reached deliberately. + result.append(line.isEmpty() ? QStringLiteral(">") + : QStringLiteral("> ") + line); + } + + const QString replacement = result.join(QLatin1Char('\n')); + edit.text = before + replacement + after; + // The quoted block stays selected, so a second press nests it. + edit.selectionStart = firstLineStart; + edit.selectionEnd = firstLineStart + replacement.size(); + + return edit; +} diff --git a/src/formattoolbar.h b/src/formattoolbar.h new file mode 100644 index 0000000..d820a88 --- /dev/null +++ b/src/formattoolbar.h @@ -0,0 +1,76 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include + +/// The markdown transformations behind the composer's formatting toolbar. +/// +/// Free functions over text and a selection, with no widget anywhere, so the +/// grammar is tested without a painter. Each one is a transformation over the +/// SOURCE: nothing about the buffer changes, it stays markdown the user can +/// also type by hand. +/// +/// Every function takes the selection as the widget reports it, which means +/// the anchor may sit AFTER the cursor. Each one normalises with qMin/qMax +/// rather than requiring the caller to, since a backwards drag is an ordinary +/// gesture and a caller that forgets would corrupt the buffer silently. +/// Out-of-range positions are clamped to the text, so a stale selection +/// cannot index past the end, and a boundary landing INSIDE a surrogate pair +/// is nudged off it, so a position computed arithmetically cannot split a +/// character in half. +/// +/// Neither wrap() nor quote() TOGGLES. A second press stacks another level: +/// `**this**` becomes `***this***` and `> one` becomes `> > one`. That is the +/// design, not an omission. The selection is preserved precisely so a second +/// press can apply a SECOND token to the same words, bold then italic without +/// reselecting, and a toggle would make that gesture unreachable. A toggle is +/// wanted eventually and is a spec change rather than a fix; see item 135 in +/// the backlog for the states it has to distinguish. +namespace MarkdownFormat { + +/// The result of a transformation: the new text and where the selection +/// should end up. +struct Edit +{ + QString text; + int selectionStart = 0; + int selectionEnd = 0; +}; + +/// Wraps the selection in \p token, or inserts an empty pair with the cursor +/// BETWEEN the tokens when there is no selection. +/// +/// The cursor landing between the tokens is the property a user notices +/// immediately when it is wrong, and it is invisible to a test that only +/// compares the resulting text. +Edit wrap(const QString &text, int start, int end, const QString &token); + +/// `[text](url)`. With a selection the selected text becomes the label and +/// the cursor lands inside the empty parentheses, which is where the user has +/// to type next. With none the cursor lands inside the brackets, since the +/// label is then what gets typed first. +Edit link(const QString &text, int start, int end); + +/// `> ` on every line the selection touches, including a line the selection +/// only starts or ends on. Line-based rather than a wrap, so it cannot be +/// expressed with wrap(). +Edit quote(const QString &text, int start, int end); + +} // namespace MarkdownFormat diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 48b30fc..b28bf6f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -74,6 +74,7 @@ add_qtmaildir_test(maildirname) add_qtmaildir_test(draftstore) add_qtmaildir_test(messagesender) add_qtmaildir_test(composecontext) +add_qtmaildir_test(formattoolbar) add_qtmaildir_test(translations) # Asserts on the tracked .ts rather than the generated .qm: an untranslated # string is dropped by lrelease, so it is invisible in the .qm and shows up diff --git a/tests/test_formattoolbar.cpp b/tests/test_formattoolbar.cpp new file mode 100644 index 0000000..36918f9 --- /dev/null +++ b/tests/test_formattoolbar.cpp @@ -0,0 +1,346 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include + +#include "formattoolbar.h" + +class TestFormatToolbar : public QObject +{ + Q_OBJECT + +private slots: + void wrappingASelectionKeepsItSelected(); + void wrappingWithNoSelectionPutsTheCursorBetweenTheTokens(); + void wrappingAppliesTheTokenOnBothSides(); + void aBackwardsSelectionWrapsTheSameWordsAsAForwardOne(); + + void wrappingTwiceNestsTheTokensAroundTheSameWords(); + + void aLinkWithASelectionUsesItAsTheLabel(); + void aLinkWithNoSelectionLeavesTheCursorInTheLabel(); + void aBackwardsSelectionLinksTheSameWordsAsAForwardOne(); + void quotingPrefixesEveryLineTheSelectionTouches(); + void quotingAPartialLineStillQuotesTheWholeLine(); + void quotingASingleLineWithNoSelectionQuotesThatLine(); + void quotingWithTheCursorAtTheEndOfALineQuotesThatLineNotTheNext(); + void quotingSelectsTheQuotedLines(); + void quotingSelectsOnlyTheLineItQuoted(); + void quotingTheLastLineKeepsTheRestOfTheText(); + void quotingAnAlreadyQuotedLineNestsIt(); + void quotingAnEmptyLineLeavesTheMarkerWithoutTrailingSpace(); + void aSelectionPastTheEndIsClamped(); + void aSelectionSplittingASurrogatePairKeepsTheCharacterWhole(); +}; + +void TestFormatToolbar::wrappingASelectionKeepsItSelected() +{ + // The selection is preserved so a second button press applies a second + // token to the same words: bold then italic, without reselecting. + const MarkdownFormat::Edit edit = MarkdownFormat::wrap( + QStringLiteral("make this bold"), 5, 9, QStringLiteral("**")); + + QCOMPARE(edit.text, QStringLiteral("make **this** bold")); + QCOMPARE(edit.text.mid(edit.selectionStart, + edit.selectionEnd - edit.selectionStart), + QStringLiteral("this")); +} + +void TestFormatToolbar::wrappingWithNoSelectionPutsTheCursorBetweenTheTokens() +{ + // The property a user notices immediately when it is wrong: press Bold, + // start typing, and the words must appear INSIDE the asterisks. A text + // comparison alone passes whether the cursor is inside or after. + const MarkdownFormat::Edit edit = MarkdownFormat::wrap( + QStringLiteral("ab"), 2, 2, QStringLiteral("**")); + + QCOMPARE(edit.text, QStringLiteral("ab****")); + QCOMPARE(edit.selectionStart, edit.selectionEnd); + QCOMPARE(edit.selectionStart, 4); + + // Stated as the behaviour rather than the index: typing "x" here must + // produce "ab**x**". + QString typed = edit.text; + typed.insert(edit.selectionStart, QStringLiteral("x")); + QCOMPARE(typed, QStringLiteral("ab**x**")); +} + +void TestFormatToolbar::wrappingAppliesTheTokenOnBothSides() +{ + QCOMPARE(MarkdownFormat::wrap(QStringLiteral("x"), 0, 1, + QStringLiteral("~~")).text, + QStringLiteral("~~x~~")); + QCOMPARE(MarkdownFormat::wrap(QStringLiteral("x"), 0, 1, + QStringLiteral("`")).text, + QStringLiteral("`x`")); +} + +void TestFormatToolbar::aBackwardsSelectionWrapsTheSameWordsAsAForwardOne() +{ + // A drag from right to left reports the anchor after the cursor. Qt hands + // that over as-is, so a transformation that trusts the order inserts the + // closing token before the opening one and corrupts the buffer. + const MarkdownFormat::Edit edit = MarkdownFormat::wrap( + QStringLiteral("make this bold"), 9, 5, QStringLiteral("**")); + + QCOMPARE(edit.text, QStringLiteral("make **this** bold")); + QCOMPARE(edit.text.mid(edit.selectionStart, + edit.selectionEnd - edit.selectionStart), + QStringLiteral("this")); +} + +void TestFormatToolbar::wrappingTwiceNestsTheTokensAroundTheSameWords() +{ + // The reason the selection is preserved at all: bold, then italic, + // without touching the mouse. Asserting on the second result is what + // makes the preserved selection load-bearing rather than decorative, + // since a wrong selection here produces valid-looking but wrong markdown + // ("make ***this** bold*" or similar). + // + // Stacking rather than toggling is the spec's behaviour, not an + // omission: a second Bold press gives "****this****". A toggle is wanted + // eventually and would make THIS gesture unreachable, which is the + // unanswered design question recorded as backlog item 135. + const MarkdownFormat::Edit first = MarkdownFormat::wrap( + QStringLiteral("make this bold"), 5, 9, QStringLiteral("**")); + const MarkdownFormat::Edit second = MarkdownFormat::wrap( + first.text, first.selectionStart, first.selectionEnd, + QStringLiteral("*")); + + QCOMPARE(second.text, QStringLiteral("make ***this*** bold")); + QCOMPARE(second.text.mid(second.selectionStart, + second.selectionEnd - second.selectionStart), + QStringLiteral("this")); +} + +void TestFormatToolbar::aLinkWithASelectionUsesItAsTheLabel() +{ + const MarkdownFormat::Edit edit = MarkdownFormat::link( + QStringLiteral("see the docs"), 8, 12); + + QCOMPARE(edit.text, QStringLiteral("see the [docs]()")); + + // The cursor goes inside the parentheses: the label is written and the + // URL is what the user still has to type. + QCOMPARE(edit.selectionStart, edit.selectionEnd); + QString typed = edit.text; + typed.insert(edit.selectionStart, QStringLiteral("https://example.org")); + QCOMPARE(typed, QStringLiteral("see the [docs](https://example.org)")); +} + +void TestFormatToolbar::aLinkWithNoSelectionLeavesTheCursorInTheLabel() +{ + // With nothing selected there is no label yet, so the label is what the + // user types first. + const MarkdownFormat::Edit edit = MarkdownFormat::link(QString(), 0, 0); + + QCOMPARE(edit.text, QStringLiteral("[]()")); + QCOMPARE(edit.selectionStart, edit.selectionEnd); + QString typed = edit.text; + typed.insert(edit.selectionStart, QStringLiteral("label")); + QCOMPARE(typed, QStringLiteral("[label]()")); +} + +void TestFormatToolbar::aBackwardsSelectionLinksTheSameWordsAsAForwardOne() +{ + const MarkdownFormat::Edit edit = MarkdownFormat::link( + QStringLiteral("see the docs"), 12, 8); + + QCOMPARE(edit.text, QStringLiteral("see the [docs]()")); + QString typed = edit.text; + typed.insert(edit.selectionStart, QStringLiteral("https://example.org")); + QCOMPARE(typed, QStringLiteral("see the [docs](https://example.org)")); +} + +void TestFormatToolbar::quotingPrefixesEveryLineTheSelectionTouches() +{ + const MarkdownFormat::Edit edit = MarkdownFormat::quote( + QStringLiteral("one\ntwo\nthree"), 0, 7); + + QCOMPARE(edit.text, QStringLiteral("> one\n> two\nthree")); +} + +void TestFormatToolbar::quotingAPartialLineStillQuotesTheWholeLine() +{ + // A selection from the middle of one line into the middle of the next + // must quote both whole lines. Quoting half a line produces markdown that + // means something else entirely. + const MarkdownFormat::Edit edit = MarkdownFormat::quote( + QStringLiteral("one\ntwo\nthree"), 1, 5); + + QCOMPARE(edit.text, QStringLiteral("> one\n> two\nthree")); +} + +void TestFormatToolbar::quotingASingleLineWithNoSelectionQuotesThatLine() +{ + const MarkdownFormat::Edit edit = MarkdownFormat::quote( + QStringLiteral("one\ntwo"), 5, 5); + + QCOMPARE(edit.text, QStringLiteral("one\n> two")); +} + +void TestFormatToolbar::quotingWithTheCursorAtTheEndOfALineQuotesThatLineNotTheNext() +{ + // Position 3 is the end of "one", immediately BEFORE the newline, so the + // cursor is on the first line. Searching backwards from the cursor itself + // rather than from one before it finds that newline and quotes the SECOND + // line, which is the line the user is not on. The off-by-one is invisible + // in every other case because no newline sits at the search position. + const MarkdownFormat::Edit edit = MarkdownFormat::quote( + QStringLiteral("one\ntwo"), 3, 3); + + QCOMPARE(edit.text, QStringLiteral("> one\ntwo")); +} + +void TestFormatToolbar::quotingSelectsTheQuotedLines() +{ + // The quoted block stays selected, so pressing Quote again nests it and + // a following transformation applies to the same lines. + const MarkdownFormat::Edit edit = MarkdownFormat::quote( + QStringLiteral("one\ntwo\nthree"), 1, 5); + + QCOMPARE(edit.text.mid(edit.selectionStart, + edit.selectionEnd - edit.selectionStart), + QStringLiteral("> one\n> two")); +} + +void TestFormatToolbar::quotingTheLastLineKeepsTheRestOfTheText() +{ + // No trailing newline after the last line, so the end-of-text search + // returns -1 and an unguarded implementation truncates everything from + // the selection onwards. + const MarkdownFormat::Edit edit = MarkdownFormat::quote( + QStringLiteral("one\ntwo\nthree"), 9, 9); + + QCOMPARE(edit.text, QStringLiteral("one\ntwo\n> three")); +} + +void TestFormatToolbar::quotingSelectsOnlyTheLineItQuoted() +{ + // The line quoted here is the SECOND one, so a selection that wrongly + // starts at 0 is distinguishable from a correct one. The existing + // quotingSelectsTheQuotedLines fixture starts on the first line, where a + // hardcoded 0 and the right answer coincide: that coincidence let a + // mutation replacing firstLineStart with 0 pass the whole suite. + // + // The damage is not cosmetic. With the wrong selection a second Quote + // press quotes a line the user never selected, and a following Bold + // bolds the wrong text. + const MarkdownFormat::Edit edit = MarkdownFormat::quote( + QStringLiteral("one\ntwo\nthree"), 5, 5); + + QCOMPARE(edit.text, QStringLiteral("one\n> two\nthree")); + QCOMPARE(edit.text.mid(edit.selectionStart, + edit.selectionEnd - edit.selectionStart), + QStringLiteral("> two")); +} + +void TestFormatToolbar::quotingAnAlreadyQuotedLineNestsIt() +{ + // Nests rather than toggling, per the spec, which states there is + // deliberately no live toggle that inserts and removes the quote while + // editing. A second press deepens the quote. Backlog item 135 holds the + // toggle design if that is ever revisited. + const MarkdownFormat::Edit edit = MarkdownFormat::quote( + QStringLiteral("> one"), 0, 5); + + QCOMPARE(edit.text, QStringLiteral("> > one")); +} + +void TestFormatToolbar::quotingAnEmptyLineLeavesTheMarkerWithoutTrailingSpace() +{ + // A blank line inside a quoted block is what continues the block in + // markdown, so it gets the marker. "> " with nothing after it is trailing + // whitespace that several editors and mail clients strip, which would + // break the block; the marker is written bare. + const MarkdownFormat::Edit edit = MarkdownFormat::quote( + QStringLiteral("one\n\ntwo"), 0, 8); + + QCOMPARE(edit.text, QStringLiteral("> one\n>\n> two")); +} + +void TestFormatToolbar::aSelectionPastTheEndIsClamped() +{ + // A stale selection outliving an edit to the buffer would otherwise index + // past the end. QString tolerates that in some calls and not in others, + // so it is clamped once at the entry rather than relied on per call. + QCOMPARE(MarkdownFormat::wrap(QStringLiteral("ab"), 0, 99, + QStringLiteral("**")).text, + QStringLiteral("**ab**")); + QCOMPARE(MarkdownFormat::link(QStringLiteral("ab"), -5, 99).text, + QStringLiteral("[ab]()")); + QCOMPARE(MarkdownFormat::quote(QStringLiteral("ab"), -5, 99).text, + QStringLiteral("> ab")); +} + +void TestFormatToolbar::aSelectionSplittingASurrogatePairKeepsTheCharacterWhole() +{ + // An emoji is two UTF-16 code units, so a boundary at 4 lands BETWEEN + // them. Inserting there splits the character: the result is invalid + // UTF-16 and the emoji is destroyed, not merely moved. + // + // Not reachable by arrow key or mouse, which both move in whole clusters, + // but QTextCursor::setPosition accepts it, so anything computing a + // position arithmetically gets there: a draft restore, a find/replace, a + // template insertion. + const QString emoji = QString::fromUcs4(U"\U0001F600"); + const QString text = QStringLiteral("hi ") + emoji + QStringLiteral(" there"); + QCOMPARE(text.size(), 11); + QVERIFY(text.at(3).isHighSurrogate()); + QVERIFY(text.at(4).isLowSurrogate()); + + // Boundary inside the pair on the closing side. + const MarkdownFormat::Edit a = + MarkdownFormat::wrap(text, 3, 4, QStringLiteral("**")); + QVERIFY2(a.text.isValidUtf16(), "wrap split the surrogate pair"); + QVERIFY2(a.text.contains(emoji), "wrap destroyed the character"); + + // Boundary inside the pair on the opening side. + const MarkdownFormat::Edit b = + MarkdownFormat::wrap(text, 4, 5, QStringLiteral("**")); + QVERIFY2(b.text.isValidUtf16(), "wrap split the surrogate pair"); + QVERIFY2(b.text.contains(emoji), "wrap destroyed the character"); + + const MarkdownFormat::Edit c = MarkdownFormat::link(text, 3, 4); + QVERIFY2(c.text.isValidUtf16(), "link split the surrogate pair"); + QVERIFY2(c.text.contains(emoji), "link destroyed the character"); + + // A COLLAPSED cursor inside the pair must stay collapsed. Nudging its two + // ends in opposite directions would keep the character whole while + // turning "insert an empty pair here" into "wrap the emoji", which is a + // character the user never selected. + const MarkdownFormat::Edit e = + MarkdownFormat::wrap(text, 4, 4, QStringLiteral("**")); + QVERIFY2(e.text.isValidUtf16(), "wrap split the surrogate pair"); + QCOMPARE(e.text, QStringLiteral("hi ****") + emoji + QStringLiteral(" there")); + QCOMPARE(e.selectionStart, e.selectionEnd); + QString typedInto = e.text; + typedInto.insert(e.selectionStart, QStringLiteral("x")); + QCOMPARE(typedInto, + QStringLiteral("hi **x**") + emoji + QStringLiteral(" there")); + + // quote() snaps to line boundaries, so it is immune by construction. + // Asserted rather than assumed, so a later change to how it widens the + // selection cannot quietly lose that. + const MarkdownFormat::Edit d = MarkdownFormat::quote(text, 3, 4); + QVERIFY2(d.text.isValidUtf16(), "quote split the surrogate pair"); + QCOMPARE(d.text, QStringLiteral("> ") + text); +} + +QTEST_APPLESS_MAIN(TestFormatToolbar) +#include "test_formattoolbar.moc" -- cgit v1.2.3