diff options
| -rw-r--r-- | README.md | 22 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 34 | ||||
| -rw-r--r-- | src/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | src/keymap.cpp | 2 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 41 | ||||
| -rw-r--r-- | src/mainwindow.h | 6 | ||||
| -rw-r--r-- | src/tagdialog.cpp | 237 | ||||
| -rw-r--r-- | src/tagdialog.h | 102 | ||||
| -rw-r--r-- | tests/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | tests/test_tagdialog.cpp | 243 |
10 files changed, 688 insertions, 1 deletions
@@ -248,6 +248,27 @@ an attachment, so it is visible without opening the thread. It comes from the `attachment` tag notmuch applies while indexing, not from parsing the message, and costs no extra query. +## Tagging + +Archive, delete, spam, flag and toggle-unread write fixed tags. For anything +else, **Ctrl+T** opens a dialog over the selected threads: type tags to add or +remove, separated by commas, or clear a checkbox to drop a tag already present. + +Both fields complete against every tag in your database, which is a guard +against typing `shoppping` beside `shopping`, not a restriction: a tag that does +not exist yet is exactly what the dialog is for, so any valid name is accepted. + +With several threads selected, a tag on only some of them shows a partially +checked box saying how many. **Leaving it alone changes nothing.** Check it to +apply it to all, clear it to remove it from all. + +Tag names are rejected if empty, if they start with `-` (notmuch reads that as +"remove this tag"), or if they contain spaces or unprintable characters. The +dialog says which name was refused and why, and applies nothing until the whole +set is valid. + +Every change goes on the undo stack, so `Ctrl+Z` reverses a mistyped tag. + ## Unsynced changes Tagging changes the notmuch index at once, but the mail store only learns about @@ -311,6 +332,7 @@ Defaults, all rebindable through `[keys]`: | `Ctrl+Space` | `complete_query` | Focus the query bar and offer completions | | `Ctrl+H` | `toggle_html` | Switch the thread between HTML and plain text | | `Ctrl+M` | `load_remote` | Load remote images for the current thread | +| `Ctrl+T` | `edit_tags` | Add or remove any tag on the selected threads | | `Ctrl+Shift+D` | `message_details` | Show the full headers of every message in the thread | | `Ctrl+Z` | `undo` | Undo the last tag change | | `Ctrl+G` | `sync` | Run the configured sync command | 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 6fd40c1..4df48dd 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 @@ -64,7 +64,7 @@ taking that too literally. | 23 | No way to save a search query from the UI | workflow | M | open | | 24 | No right-click actions on the thread list | discoverability | S | open | | 25 | No select-all, and bulk actions are undiscoverable | workflow | S | open | -| 26 | No way to add or remove an arbitrary tag from the UI | workflow | S | open | +| 26 | No way to add or remove an arbitrary tag from the UI | workflow | S | **done** | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1160,6 +1160,38 @@ dialog and two actions, not new plumbing. refreshes the list when a mutation introduces an unknown tag, so that path is in place and should be relied on rather than duplicated. +### Outcome (done) + +Built as the user chose: one **Edit tags** dialog on `Ctrl+T` rather than +separate add and remove actions, since filing something under a new tag while +dropping `inbox` is one thought. + +`TagDialog` is pure UI in `qtmaildir_lib`. It is handed the vocabulary and the +selection's current tags and returns two lists; it contacts no worker and holds +no database handle, which is what lets fifteen tests run without a notmuch +database. Integration is one call to the existing `tagSelected()`, so undo, the +optimistic model update, the one-query multi-row resolution and the completer +refresh all come for free. + +**Tri-state is the part that needed the tests.** With several threads selected a +tag can be on some, and `PartiallyChecked` means "leave alone" rather than +"apply to all". The opposite reading silently tags threads the user never +looked at. `m_fullyTagged` exists for the neighbouring case: a tag already on +every thread and left checked is not a change and must not be sent as one. + +**Validation is a free function** so the rules are testable directly. It rejects +empty, a leading `-` (notmuch's CLI reads that as removal, so such a tag is a +trap), whitespace, and control characters. Nothing is applied until every name +passes, since a half-applied change is worse than none: the user cannot tell +which half landed. + +**One test assumption was wrong and the code was right.** A case asserted that +`QStringLiteral("null\0byte")` truncates at the null and reads as Empty. It +does not; the literal is kept whole, so the null is caught as a control +character. The test was corrected, not the validator. + +Rendered and inspected rather than only asserted. + --- ## Deferred, unsized, or split out diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index de19c8f..5214f42 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -8,6 +8,7 @@ add_library(qtmaildir_lib STATIC notmuchworker.cpp tagchip.cpp tagcolors.cpp + tagdialog.cpp tagstrip.cpp threadlistmodel.cpp mailsync.cpp diff --git a/src/keymap.cpp b/src/keymap.cpp index b18f5d5..0901cfb 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -31,6 +31,7 @@ QStringList KeyMap::knownActions() QStringLiteral("delete"), QStringLiteral("spam"), QStringLiteral("toggle_unread"), + QStringLiteral("edit_tags"), QStringLiteral("flag"), QStringLiteral("focus_query"), QStringLiteral("complete_query"), @@ -65,6 +66,7 @@ QList<QPair<QString, QString>> KeyMap::defaultBindings() { QStringLiteral("Ctrl+Shift+S"), QStringLiteral("spam") }, { QStringLiteral("Ctrl+U"), QStringLiteral("toggle_unread") }, { QStringLiteral("Ctrl+I"), QStringLiteral("flag") }, + { QStringLiteral("Ctrl+T"), QStringLiteral("edit_tags") }, { QStringLiteral("Ctrl+L"), QStringLiteral("focus_query") }, // Ctrl+Space is the completion idiom users already carry over from // shells and editors, and it is a named key rather than a symbol, so diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index d8fba3e..000be85 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -52,6 +52,7 @@ #include "notmuchworker.h" #include "querycompleter.h" #include "tagchip.h" +#include "tagdialog.h" #include "threadlistmodel.h" #include "version.h" @@ -548,6 +549,10 @@ void MainWindow::registerActions() else tagSelected({ QStringLiteral("unread") }, {}, tr("Mark unread")); }); + addAction(QStringLiteral("edit_tags"), tr("Edit &tags..."), + tr("Add or remove any tag on the selected threads"), [this]() { + editTagsOnSelection(); + }); addAction(QStringLiteral("toggle_html"), tr("Toggle &HTML"), tr("Switch the thread between HTML and plain text"), [this]() { m_messageView->toggleHtml(); @@ -634,6 +639,7 @@ void MainWindow::buildMenus() messageMenu->addAction(m_actions.value(QStringLiteral("spam"))); messageMenu->addSeparator(); messageMenu->addAction(m_actions.value(QStringLiteral("toggle_unread"))); + messageMenu->addAction(m_actions.value(QStringLiteral("edit_tags"))); messageMenu->addAction(m_actions.value(QStringLiteral("flag"))); auto *viewMenu = menuBar()->addMenu(tr("&View")); @@ -1152,6 +1158,41 @@ void MainWindow::markCurrentThreadRead() tr("Mark read")); } +void MainWindow::editTagsOnSelection() +{ + const QModelIndexList rows = + m_threadView->selectionModel()->selectedRows(); + if (rows.isEmpty()) { + m_statusLabel->setText(tr("Select a thread first")); + return; + } + + // How many of the selected threads carry each tag, which is what tells a + // tag that is on all of them from one that is on some. + QHash<QString, int> counts; + for (const QModelIndex &index : rows) { + const ThreadSummary thread = m_model->threadAt(index.row()); + for (const QString &tag : thread.tags) + counts[tag] += 1; + } + + // m_knownTags is the same list the query completer uses, so the dialog + // offers every tag in the database without a round trip. + TagDialog dialog(m_knownTags, counts, rows.size(), this); + if (dialog.exec() != QDialog::Accepted) + return; + + const QStringList add = dialog.tagsToAdd(); + const QStringList remove = dialog.tagsToRemove(); + if (add.isEmpty() && remove.isEmpty()) + return; // Applied with nothing changed. + + // Straight through tagSelected(), so this inherits undo, the optimistic + // model update, the one-query multi-row resolution, and the completer + // refresh for a tag that did not exist before. + tagSelected(add, remove, tr("Edit tags")); +} + void MainWindow::tagSelected(const QStringList &add, const QStringList &remove, const QString &description) { diff --git a/src/mainwindow.h b/src/mainwindow.h index b0a4cfd..7897038 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -134,6 +134,12 @@ private: /// Redraws the unsynced-edits indicator from m_pendingEdits. void updatePendingIndicator(); + /// Opens the tag dialog on the current selection and applies its result. + /// + /// The only route to an arbitrary tag: every other tag action writes a + /// hardcoded name. + void editTagsOnSelection(); + /// Set once the user has answered the exit prompt, or once a sync started /// for exit has finished. Stops closeEvent asking a second time, and is /// what lets the deferred close through. diff --git a/src/tagdialog.cpp b/src/tagdialog.cpp new file mode 100644 index 0000000..94d2e05 --- /dev/null +++ b/src/tagdialog.cpp @@ -0,0 +1,237 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * 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 "tagdialog.h" + +#include <QCompleter> +#include <QCoreApplication> +#include <QDialogButtonBox> +#include <QFormLayout> +#include <QHash> +#include <QLabel> +#include <QLineEdit> +#include <QListWidget> +#include <QMessageBox> +#include <QPushButton> +#include <QVBoxLayout> + +TagNameProblem validateTagName(const QString &tag) +{ + const QString trimmed = tag.trimmed(); + + if (trimmed.isEmpty()) + return TagNameProblem::Empty; + + // notmuch's CLI reads "-tag" as an instruction to remove that tag, so a tag + // actually named "-inbox" is a trap the user cannot easily undo later. + if (trimmed.startsWith(QLatin1Char('-'))) + return TagNameProblem::LeadingDash; + + for (const QChar c : trimmed) { + // Checked before the general control-character test, since space is not + // a control character but splits a tag just as effectively. + if (c.isSpace()) + return TagNameProblem::ContainsSpace; + if (c.category() == QChar::Other_Control) + return TagNameProblem::ControlChar; + } + + return TagNameProblem::Ok; +} + +QString tagNameProblemText(TagNameProblem problem, const QString &tag) +{ + switch (problem) { + case TagNameProblem::Ok: + return {}; + case TagNameProblem::Empty: + return QCoreApplication::translate("TagDialog", + "A tag name cannot be empty."); + case TagNameProblem::LeadingDash: + return QCoreApplication::translate( + "TagDialog", + "'%1' cannot start with '-': notmuch reads a leading dash as an " + "instruction to remove a tag.").arg(tag); + case TagNameProblem::ContainsSpace: + return QCoreApplication::translate( + "TagDialog", "'%1' cannot contain spaces.").arg(tag); + case TagNameProblem::ControlChar: + return QCoreApplication::translate( + "TagDialog", "'%1' contains a character that cannot be typed " + "back.").arg(tag); + } + return {}; +} + +namespace { + +/// Splits a line edit's contents into tag names, dropping empties. +/// +/// Comma-separated, so several tags can be applied in one pass. A tag name +/// cannot contain a comma once validateTagName() has run, which is what makes +/// this unambiguous. +QStringList splitTags(const QString &text) +{ + QStringList tags; + const QStringList parts = text.split(QLatin1Char(','), Qt::SkipEmptyParts); + for (const QString &part : parts) { + const QString trimmed = part.trimmed(); + if (!trimmed.isEmpty()) + tags.append(trimmed); + } + return tags; +} + +} // namespace + +TagDialog::TagDialog(const QStringList &knownTags, + const QHash<QString, int> ¤tTags, + int threadCount, + QWidget *parent) + : QDialog(parent), m_threadCount(threadCount) +{ + setWindowTitle(tr("Edit tags")); + + auto *layout = new QVBoxLayout(this); + + auto *heading = new QLabel(tr("%n selected thread(s)", "", threadCount), + this); + layout->addWidget(heading); + + auto *form = new QFormLayout; + + m_addEdit = new QLineEdit(this); + m_addEdit->setPlaceholderText(tr("tag, or several separated by commas")); + m_removeEdit = new QLineEdit(this); + m_removeEdit->setPlaceholderText(tr("tag, or several separated by commas")); + + // Completion is a guard against typos, never a whitelist: a tag absent from + // this list is exactly what the dialog exists to create, so the completer + // suggests and does not constrain. + for (QLineEdit *edit : { m_addEdit, m_removeEdit }) { + auto *completer = new QCompleter(knownTags, edit); + completer->setCaseSensitivity(Qt::CaseInsensitive); + // Hierarchies are the reason this matters: typing "amazon" should find + // "shopping/amazon". + completer->setFilterMode(Qt::MatchContains); + edit->setCompleter(completer); + } + + form->addRow(tr("Add:"), m_addEdit); + form->addRow(tr("Remove:"), m_removeEdit); + layout->addLayout(form); + + // The tags already on the selection, so removing one does not require + // remembering its name. Tri-state, because with several threads selected a + // tag can be on some and not others. + m_currentList = new QListWidget(this); + QStringList sorted = currentTags.keys(); + sorted.sort(); + for (const QString &tag : sorted) { + const int count = currentTags.value(tag); + + auto *item = new QListWidgetItem(tag, m_currentList); + if (count >= threadCount) { + item->setCheckState(Qt::Checked); + m_fullyTagged.append(tag); + } else { + item->setCheckState(Qt::PartiallyChecked); + // Say what "partial" means in numbers, rather than leaving the user + // to infer it from a shaded box. + item->setText(tr("%1 (on %2 of %3)") + .arg(tag).arg(count).arg(threadCount)); + item->setData(Qt::UserRole, tag); + } + if (item->data(Qt::UserRole).isNull()) + item->setData(Qt::UserRole, tag); + } + + if (m_currentList->count() > 0) { + layout->addWidget(new QLabel(tr("Tags on the selection:"), this)); + layout->addWidget(m_currentList); + } else { + m_currentList->hide(); + } + + auto *buttons = new QDialogButtonBox(QDialogButtonBox::Ok + | QDialogButtonBox::Cancel, + this); + buttons->button(QDialogButtonBox::Ok)->setText(tr("Apply")); + connect(buttons, &QDialogButtonBox::accepted, this, &TagDialog::accept); + connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject); + layout->addWidget(buttons); + + m_addEdit->setFocus(); +} + +void TagDialog::accept() +{ + QStringList add = splitTags(m_addEdit->text()); + QStringList remove = splitTags(m_removeEdit->text()); + + // Validate before applying anything: a partial change is worse than none, + // since the user cannot tell which half landed. + for (const QStringList &list : { add, remove }) { + for (const QString &tag : list) { + const TagNameProblem problem = validateTagName(tag); + if (problem != TagNameProblem::Ok) { + QMessageBox::warning(this, tr("Invalid tag"), + tagNameProblemText(problem, tag)); + return; // Stay open, with the text still there to fix. + } + } + } + + // Then the checkbox list. An item whose state the user did not touch must + // change nothing, which is why PartiallyChecked is skipped entirely: it is + // the "leave this alone" state for a tag that is on some threads only. + for (int row = 0; row < m_currentList->count(); ++row) { + QListWidgetItem *item = m_currentList->item(row); + const QString tag = item->data(Qt::UserRole).toString(); + + switch (item->checkState()) { + case Qt::Unchecked: + // Was on at least one thread, since it is in this list at all. + if (!remove.contains(tag)) + remove.append(tag); + break; + case Qt::Checked: + // Only meaningful when it was NOT already on every thread; a tag + // already everywhere and left checked is not a change. + if (!add.contains(tag) && !m_fullyTagged.contains(tag)) + add.append(tag); + break; + case Qt::PartiallyChecked: + break; // Untouched: do nothing, deliberately. + } + } + + m_add = add; + m_remove = remove; + QDialog::accept(); +} + +QStringList TagDialog::tagsToAdd() const +{ + return m_add; +} + +QStringList TagDialog::tagsToRemove() const +{ + return m_remove; +} diff --git a/src/tagdialog.h b/src/tagdialog.h new file mode 100644 index 0000000..40eb089 --- /dev/null +++ b/src/tagdialog.h @@ -0,0 +1,102 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * 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 <QDialog> +#include <QStringList> + +class QLineEdit; +class QListWidget; + +/// Why a tag name was refused, or Ok when it was not. +/// +/// A reason rather than a bool: a rejected tag has to say what was wrong with +/// it, since silently dropping one leaves the user believing they tagged +/// something they did not. +enum class TagNameProblem { + Ok, + Empty, ///< Nothing, or only whitespace. + LeadingDash, ///< notmuch reads a leading '-' as "remove this tag". + ContainsSpace, ///< Splits into two tags, or fails the write outright. + ControlChar, ///< Not typeable, and unreadable once stored. +}; + +/// Whether `tag` is safe to hand to notmuch. +/// +/// A free function so the rules can be tested without a widget. notmuch itself +/// accepts a great deal, so this is deliberately narrow: it rejects only what +/// produces a failed write or a tag the user cannot see they created. +TagNameProblem validateTagName(const QString &tag); + +/// The message for a rejected tag, ready to show. Empty for Ok. +QString tagNameProblemText(TagNameProblem problem, const QString &tag); + +/// Adds and removes tags across the selected threads. +/// +/// One dialog rather than separate add and remove actions: the natural +/// operation is "make these threads look like this", and filing something under +/// a new tag while removing inbox is one thought, not two. +/// +/// Pure UI. It contacts no worker and holds no database handle; it is handed +/// the vocabulary and the current state, and returns two lists. That is what +/// lets it be unit-tested without a notmuch database. +class TagDialog : public QDialog +{ + Q_OBJECT +public: + /// `knownTags` is the completion vocabulary, usually every tag in the + /// database. `currentTags` maps a tag to how many of the selected threads + /// carry it, which is what drives the tri-state checkboxes. + TagDialog(const QStringList &knownTags, + const QHash<QString, int> ¤tTags, + int threadCount, + QWidget *parent = nullptr); + + /// Tags to add. Empty when the user asked for nothing. + QStringList tagsToAdd() const; + + /// Tags to remove. + QStringList tagsToRemove() const; + + /// Reads both line edits and the checkbox list into the two lists, + /// reporting the first invalid name rather than applying a partial change. + /// + /// Public because QDialog::accept() is: a test drives it directly rather + /// than clicking a button, since which button carries the AcceptRole is + /// not what this class is for. + void accept() override; + +private: + + QStringList m_add; + QStringList m_remove; + + /// Tags that were already on EVERY selected thread when the dialog opened. + /// + /// Needed to tell "the user checked this box" apart from "this box was + /// checked all along": the first is an instruction, the second is not a + /// change and must not be sent as one. + QStringList m_fullyTagged; + + int m_threadCount = 0; + + QLineEdit *m_addEdit = nullptr; + QLineEdit *m_removeEdit = nullptr; + QListWidget *m_currentList = nullptr; +}; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c50c5e8..7f7caa5 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -20,3 +20,4 @@ add_qtmaildir_test(threadcidmap) add_qtmaildir_test(mainwindow) add_qtmaildir_test(messageview) add_qtmaildir_test(querycompleter) +add_qtmaildir_test(tagdialog) diff --git a/tests/test_tagdialog.cpp b/tests/test_tagdialog.cpp new file mode 100644 index 0000000..f41850e --- /dev/null +++ b/tests/test_tagdialog.cpp @@ -0,0 +1,243 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * 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 <QtTest> + +#include <QCheckBox> +#include <QLineEdit> +#include <QListWidget> + +#include "tagdialog.h" + +class TestTagDialog : public QObject +{ + Q_OBJECT +private slots: + void validNamesAreAccepted(); + void emptyNameIsRejected(); + void leadingDashIsRejected(); + void whitespaceIsRejected(); + void controlCharactersAreRejected(); + void everyProblemHasAMessage(); + + void typedTagIsAdded(); + void unknownTagIsStillAccepted(); + void multipleTagsSeparateOnComma(); + void uncheckingACurrentTagRemovesIt(); + void aPartialTagLeftAloneChangesNothing(); + void aPartialTagCheckedIsAddedEverywhere(); + void nothingTouchedYieldsNoChange(); +}; + +void TestTagDialog::validNamesAreAccepted() +{ + // Hierarchical tags are the common case here, and the '/' must survive: + // notmuch treats it as an ordinary character in a tag name. + for (const QString &tag : { QStringLiteral("inbox"), + QStringLiteral("shopping/amazon"), + QStringLiteral("mailing-list/SBo"), + QStringLiteral("2026"), + QStringLiteral("with.dots"), + QStringLiteral("under_score"), + // A dash anywhere but the front is fine. + QStringLiteral("half-done") }) { + QCOMPARE(validateTagName(tag), TagNameProblem::Ok); + } +} + +void TestTagDialog::emptyNameIsRejected() +{ + QCOMPARE(validateTagName(QString()), TagNameProblem::Empty); + QCOMPARE(validateTagName(QStringLiteral("")), TagNameProblem::Empty); + // Whitespace only is empty in every sense that matters. + QCOMPARE(validateTagName(QStringLiteral(" ")), TagNameProblem::Empty); + QCOMPARE(validateTagName(QStringLiteral("\t")), TagNameProblem::Empty); +} + +void TestTagDialog::leadingDashIsRejected() +{ + // notmuch's own CLI reads -tag as "remove tag". A tag named "-inbox" would + // therefore be a permanent trap for anyone who later types it at a prompt. + QCOMPARE(validateTagName(QStringLiteral("-inbox")), + TagNameProblem::LeadingDash); + // Also after trimming, or a leading space would smuggle one through. + QCOMPARE(validateTagName(QStringLiteral(" -inbox")), + TagNameProblem::LeadingDash); +} + +void TestTagDialog::whitespaceIsRejected() +{ + // An embedded space is the failure that looks like it worked: the user + // believes they made one tag and notmuch sees something else. + QCOMPARE(validateTagName(QStringLiteral("two words")), + TagNameProblem::ContainsSpace); + QCOMPARE(validateTagName(QStringLiteral("tab\there")), + TagNameProblem::ContainsSpace); + QCOMPARE(validateTagName(QStringLiteral("new\nline")), + TagNameProblem::ContainsSpace); +} + +void TestTagDialog::controlCharactersAreRejected() +{ + // A null is a control character like any other here. QStringLiteral keeps + // the whole literal rather than truncating at the null, so this is + // "null\0byte" in full and the null is what the check catches. + QString withNull = QStringLiteral("null"); + withNull.append(QChar(0x00)); + withNull.append(QStringLiteral("byte")); + QCOMPARE(validateTagName(withNull), TagNameProblem::ControlChar); + + QString withBell = QStringLiteral("bell"); + withBell.append(QChar(0x07)); + QCOMPARE(validateTagName(withBell), TagNameProblem::ControlChar); +} + +void TestTagDialog::everyProblemHasAMessage() +{ + // A rejection the user cannot read is the same as a silent one. + for (TagNameProblem problem : { TagNameProblem::Empty, + TagNameProblem::LeadingDash, + TagNameProblem::ContainsSpace, + TagNameProblem::ControlChar }) { + QVERIFY(!tagNameProblemText(problem, QStringLiteral("x")).isEmpty()); + } + QVERIFY(tagNameProblemText(TagNameProblem::Ok, + QStringLiteral("x")).isEmpty()); +} + +/// Drives the dialog the way a user would, then accepts it. +static void typeAndAccept(TagDialog *dialog, const QString &add, + const QString &remove) +{ + const QList<QLineEdit *> edits = dialog->findChildren<QLineEdit *>(); + QVERIFY(edits.size() >= 2); + edits.at(0)->setText(add); + edits.at(1)->setText(remove); + dialog->accept(); +} + +void TestTagDialog::typedTagIsAdded() +{ + TagDialog dialog({ QStringLiteral("inbox") }, {}, 1); + typeAndAccept(&dialog, QStringLiteral("shopping/amazon"), QString()); + + QCOMPARE(dialog.tagsToAdd(), QStringList{ QStringLiteral("shopping/amazon") }); + QVERIFY(dialog.tagsToRemove().isEmpty()); +} + +void TestTagDialog::unknownTagIsStillAccepted() +{ + // Completion is a guard against typos, NOT a whitelist. Inventing a tag is + // the entire point of the dialog, so a name absent from the vocabulary must + // go through untouched. + TagDialog dialog({ QStringLiteral("inbox") }, {}, 1); + typeAndAccept(&dialog, QStringLiteral("brand/new/tag"), QString()); + + QCOMPARE(dialog.tagsToAdd(), QStringList{ QStringLiteral("brand/new/tag") }); +} + +void TestTagDialog::multipleTagsSeparateOnComma() +{ + TagDialog dialog({}, {}, 1); + typeAndAccept(&dialog, QStringLiteral("one, two,three"), QString()); + + QCOMPARE(dialog.tagsToAdd(), QStringList({ QStringLiteral("one"), + QStringLiteral("two"), + QStringLiteral("three") })); +} + +void TestTagDialog::uncheckingACurrentTagRemovesIt() +{ + // Every selected thread carries "inbox", so its box starts checked. + // Clearing it is how a user removes a tag without typing its name. + QHash<QString, int> current; + current.insert(QStringLiteral("inbox"), 3); + + TagDialog dialog({ QStringLiteral("inbox") }, current, 3); + + auto *list = dialog.findChild<QListWidget *>(); + QVERIFY(list); + QCOMPARE(list->count(), 1); + + QListWidgetItem *item = list->item(0); + QCOMPARE(item->text(), QStringLiteral("inbox")); + QCOMPARE(item->checkState(), Qt::Checked); + + item->setCheckState(Qt::Unchecked); + dialog.accept(); + + QCOMPARE(dialog.tagsToRemove(), QStringList{ QStringLiteral("inbox") }); + QVERIFY(dialog.tagsToAdd().isEmpty()); +} + +void TestTagDialog::aPartialTagLeftAloneChangesNothing() +{ + // THE case worth guarding. Two of three threads are unread, so the box is + // partially checked. Leaving it alone must mean "do not touch", never + // "apply to all": the second reading silently tags a thread the user never + // looked at. + QHash<QString, int> current; + current.insert(QStringLiteral("unread"), 2); + + TagDialog dialog({ QStringLiteral("unread") }, current, 3); + + auto *list = dialog.findChild<QListWidget *>(); + QVERIFY(list); + QListWidgetItem *item = list->item(0); + QCOMPARE(item->checkState(), Qt::PartiallyChecked); + + dialog.accept(); + + QVERIFY2(dialog.tagsToAdd().isEmpty(), + "a partial tag left alone was added to every thread"); + QVERIFY2(dialog.tagsToRemove().isEmpty(), + "a partial tag left alone was removed from every thread"); +} + +void TestTagDialog::aPartialTagCheckedIsAddedEverywhere() +{ + // Deliberately checking a partial box is an instruction: give it to all. + QHash<QString, int> current; + current.insert(QStringLiteral("unread"), 2); + + TagDialog dialog({ QStringLiteral("unread") }, current, 3); + + auto *list = dialog.findChild<QListWidget *>(); + QVERIFY(list); + list->item(0)->setCheckState(Qt::Checked); + dialog.accept(); + + QCOMPARE(dialog.tagsToAdd(), QStringList{ QStringLiteral("unread") }); + QVERIFY(dialog.tagsToRemove().isEmpty()); +} + +void TestTagDialog::nothingTouchedYieldsNoChange() +{ + QHash<QString, int> current; + current.insert(QStringLiteral("inbox"), 2); + current.insert(QStringLiteral("unread"), 1); + + TagDialog dialog({ QStringLiteral("inbox") }, current, 2); + dialog.accept(); + + QVERIFY(dialog.tagsToAdd().isEmpty()); + QVERIFY(dialog.tagsToRemove().isEmpty()); +} + +QTEST_MAIN(TestTagDialog) +#include "test_tagdialog.moc" |