diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-03 15:42:40 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-03 15:42:40 +0200 |
| commit | eb35acda4207392c10b4b1192b3b057bcad99a47 (patch) | |
| tree | c962e91fc502ec57d92df386e773033fe42fd17b | |
| parent | afd20c9527fa77ba60901707c7bc73b2af926a67 (diff) | |
| parent | f62ced3c2c85675e746bff7ef8aca5c75c9737e0 (diff) | |
| download | qtmaildir-eb35acda4207392c10b4b1192b3b057bcad99a47.tar.gz qtmaildir-eb35acda4207392c10b4b1192b3b057bcad99a47.zip | |
Merge branch 'feature/qaction-menus'
Menus, a toolbar and a generated shortcut reference, built on converting
the action registry from a hash of callbacks to QActions. Along the way:
three default key bindings that had never fired, a shortcut dialog taller
than the screen, thread list columns that could not be resized, no visible
feedback that a tag action had landed, and a tags column so wide it was
unreadable.
Backlog items 3, 8, 9, 13 and 14 done; 11 partly.
| -rw-r--r-- | CHANGELOG.md | 61 | ||||
| -rw-r--r-- | CMakeLists.txt | 1 | ||||
| -rw-r--r-- | README.md | 90 | ||||
| -rw-r--r-- | assets/qtmaildir.desktop | 13 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 113 | ||||
| -rw-r--r-- | src/CMakeLists.txt | 16 | ||||
| -rw-r--r-- | src/config.cpp | 17 | ||||
| -rw-r--r-- | src/config.h | 10 | ||||
| -rw-r--r-- | src/keymap.cpp | 128 | ||||
| -rw-r--r-- | src/keymap.h | 27 | ||||
| -rw-r--r-- | src/main.cpp | 8 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 359 | ||||
| -rw-r--r-- | src/mainwindow.h | 33 | ||||
| -rw-r--r-- | src/messageview.cpp | 18 | ||||
| -rw-r--r-- | src/messageview.h | 9 | ||||
| -rw-r--r-- | src/resources.qrc | 6 | ||||
| -rw-r--r-- | src/tagchip.cpp | 126 | ||||
| -rw-r--r-- | src/tagchip.h | 62 | ||||
| -rw-r--r-- | src/tagcolors.cpp | 171 | ||||
| -rw-r--r-- | src/tagcolors.h | 92 | ||||
| -rw-r--r-- | src/tagstrip.cpp | 136 | ||||
| -rw-r--r-- | src/tagstrip.h | 63 | ||||
| -rw-r--r-- | src/threadlistmodel.cpp | 69 | ||||
| -rw-r--r-- | src/threadlistmodel.h | 30 | ||||
| -rw-r--r-- | src/types.h | 7 | ||||
| -rw-r--r-- | tests/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | tests/test_keymap.cpp | 151 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 93 | ||||
| -rw-r--r-- | tests/test_tagcolors.cpp | 251 | ||||
| -rw-r--r-- | tests/test_threadlistmodel.cpp | 182 |
30 files changed, 2174 insertions, 169 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 90853ce..c78cb94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,66 @@ point at which they are stable. ## [Unreleased] -Nothing yet. +### Added + +- Tags render as coloured chips instead of text in a column. The account tag + sits in front of the subject in the thread list, and the functional tags fill + a single row under the message pane, with anything that does not fit + collapsing into a `+N` chip whose tooltip names the rest. +- `[tagcolors]` config group. Colours resolve by exact tag first, then by + top-level prefix, so one `shopping` entry covers `shopping/amazon` and + `shopping/nike` while `shopping/amazon` can still override its own. Built-in + defaults cover the usual state tags; anything unconfigured gets a stable + colour derived from its name. +- `color` and `label` keys in an account stanza, setting the account chip's + fill and its text. `label` shortens a long key for display only and renames + nothing in notmuch; unset falls back to the key. +- The application icon is now used: window icon, a `.desktop` entry, and + install rules placing both into `hicolor` and `share/applications`. +- Toolbar and menu actions carry icons from the system theme, falling back to + text where a theme lacks one. +- Menu bar covering every action: File, Edit, Message, View and Help. +- Toolbar with the frequent subset, Sync, Archive, Delete and Undo. +- **Help > Keyboard shortcuts**, listing the current bindings. Generated from + the actions themselves, so it shows configured overrides rather than a + hand-written copy of the defaults. +- **Help > About**. +- Default bindings for `spam` and `load_remote`, which previously had none + and were unreachable until bound by hand. + +### Fixed + +- Acting on a thread now visibly changes its row. A thread tagged `deleted` + or `spam` is filled dark red or orange, in white struck-through text, across + every column. The tag change was already applied, but `Tags` sat after the + stretching `Subject` column and was pushed off-screen, so Delete looked like + it had done nothing. +- Thread list columns are Date, From and Subject, all resizable. The tags + column is gone: spelling out a dozen tags per row consumed most of the list's + width. Widening past the viewport scrolls horizontally rather than squeezing + the other columns. +- Hierarchical tags in `[tagcolors]` were silently ignored. QSettings treats + `/` in a key as a group separator, so `shopping/amazon` becomes a nested key + that `childKeys()` never returns, and every tag containing a `/` fell through + to its prefix. +- Three default bindings never fired. Typing a capital sends `Shift`+the key, + but `N`, `F` and `G` were stored as the unshifted key, which no keystroke + produces, leaving `toggle_unread`, `flag` and `sync` dead. A bare capital in + `[keys]` is now read as `Shift`+that letter. As a side effect `y` and `Y` + are two distinct keys rather than a collision that silently dropped one. +- Modifier shortcuts such as `Ctrl+Q` now work while the query bar has focus. + The old event filter suppressed every binding there, not only the plain + letters that would have interfered with typing. + +### Changed + +- Default bindings moved to modifier shortcuts (`Ctrl+E` archive, `Ctrl+D` + delete, and so on). Existing `[keys]` entries are unaffected, and single + letters are still safe to bind. See "Upgrading from 0.1.0" in the README. +- Actions are `QAction`s dispatched by shortcut rather than a hash of + callbacks behind an event filter, which is what lets them appear in menus. + The hand-maintained list of registered action names is now derived from the + actions, so it can no longer drift from them. ## [0.1.0] - 2026-08-03 diff --git a/CMakeLists.txt b/CMakeLists.txt index ed9db9c..7b73a7f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4,6 +4,7 @@ project(qtmaildir VERSION 0.1.0 LANGUAGES CXX) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_AUTOMOC ON) +set(CMAKE_AUTORCC ON) find_package(Qt6 6.5 REQUIRED COMPONENTS Widgets WebEngineWidgets Test) @@ -91,6 +91,8 @@ name = Your Name address = you@example.org maildir = work-mail ; relative to notmuch's database.path drafts = Drafts ; recorded for v2; unused today +label = W ; optional chip text; defaults to the key +color = #2f6fa8 ; optional chip colour; generated when unset [account.personal] name = Your Name @@ -98,57 +100,97 @@ address = you@example.net maildir = personal drafts = Drafts +[tagcolors] +; Optional. Colours resolve by exact tag first, then by top-level prefix, so +; one entry covers a whole hierarchy. +shopping = #3366cc ; also colours shopping/amazon, shopping/nike, ... +shopping/amazon = #ff9900 ; ... unless the exact tag overrides it +work = #cc4444 + [queries] Inbox = tag:inbox Unread = tag:unread Flagged = tag:flagged [keys] +Ctrl+E = archive +Ctrl+D = delete j = next_thread k = prev_thread -Return = open_thread -a = archive -d = delete -N = toggle_unread -F = flag -/ = focus_query -h = toggle_html -u = undo -G = sync -Ctrl+Q = quit ``` Saved-query buttons appear in alphabetical order rather than file order: QSettings returns keys sorted, and preserving file order would mean hand-rolling an INI parser. +## Tags + +Tags render as coloured chips, and fall into two kinds. + +**Account tags** (`account-<key>`, matching an `[account.<key>]` stanza) say +which mailbox a thread arrived in. They appear as a chip in front of the +subject in the thread list, coloured by that account's `color` key and labelled +by its `label` key. `label` changes the chip text only; the notmuch tag is +never renamed, so queries and external tagging are unaffected. + +**Functional tags** say what state a thread is in. They fill one row under the +message pane, sorted, with whatever does not fit collapsing into a `+N` chip +whose tooltip lists the rest. Colours come from `[tagcolors]`, falling back to +built-in defaults for the usual state tags (`flagged`, `unread`, `deleted`, +`spam`, `attachment`, `replied`, and others), and finally to a colour derived +from the tag name so no chip is ever unstyled. + +Lookup is exact tag first, then top-level prefix. One `shopping` entry +therefore covers `shopping/amazon` and `shopping/nike`, while a +`shopping/amazon` entry still overrides its own. + +Note that a `/` in an INI key is a group separator to QSettings, so +`shopping/amazon = #ff9900` is stored as a nested key and written to the file +as `shopping\amazon`. It is read back correctly; the escaping is QSettings' +own. + ## Keybindings Defaults, all rebindable through `[keys]`: | Key | Action | Does | |---|---|---| -| `j` | `next_thread` | Select the next thread | -| `k` | `prev_thread` | Select the previous thread | +| `Ctrl+J` | `next_thread` | Select the next thread | +| `Ctrl+K` | `prev_thread` | Select the previous thread | | `Return` | `open_thread` | Focus the thread list | -| `a` | `archive` | Remove `inbox` from every selected thread | -| `d` | `delete` | Add `deleted` | -| `N` | `toggle_unread` | Toggle `unread` | -| `F` | `flag` | Add `flagged` | -| `/` | `focus_query` | Focus and select the query bar | -| `h` | `toggle_html` | Switch the thread between HTML and plain text | -| `u` | `undo` | Undo the last tag change | -| `G` | `sync` | Run the configured sync command | +| `Ctrl+E` | `archive` | Remove `inbox` from every selected thread | +| `Ctrl+D` | `delete` | Add `deleted` | +| `Ctrl+Shift+S` | `spam` | Add `spam`, remove `inbox` | +| `Ctrl+U` | `toggle_unread` | Toggle `unread` | +| `Ctrl+I` | `flag` | Add `flagged` | +| `Ctrl+L` | `focus_query` | Focus and select the query bar | +| `Ctrl+H` | `toggle_html` | Switch the thread between HTML and plain text | +| `Ctrl+M` | `load_remote` | Load remote images for the current thread | +| `Ctrl+Z` | `undo` | Undo the last tag change | +| `Ctrl+G` | `sync` | Run the configured sync command | | `Ctrl+Q` | `quit` | Quit | -Two further actions exist but have **no default binding**, so they are -unreachable until you bind them: `spam` (adds `spam`, removes `inbox`) and -`load_remote` (the keyboard equivalent of the "Load remote content" -button). +Every action now carries a default binding, and every one appears in a menu. +**Help > Keyboard shortcuts** lists the current bindings, generated from the +actions themselves, so it shows your overrides rather than these defaults. An unknown action name in `[keys]` produces a warning at startup rather than binding silently, so a typo is visible. +### Upgrading from 0.1.0 + +0.1.0 used single letters (`j`, `k`, `a`, `d`, `N`, `F`, `h`, `u`, `G`, `/`). +Those still work if you keep them in `[keys]`, and single letters remain safe +to bind: Qt suppresses a plain-letter shortcut while the query bar has focus, +so typing a query is unaffected. + +Three of the old defaults never actually fired. Typing a capital sends +`Shift`+the key, but `N`, `F` and `G` were stored as the unshifted key, a +combination no keystroke produces, so `toggle_unread`, `flag` and `sync` were +dead. A bare capital in `[keys]` is now read as `Shift`+that letter, which is +what you press, so those bindings work whether you keep the old names or move +to the new defaults. Note this makes `y` and `Y` two different keys. + Tag actions apply to **every selected thread**, not only the focused one. ## Security posture of the message view diff --git a/assets/qtmaildir.desktop b/assets/qtmaildir.desktop new file mode 100644 index 0000000..98e435b --- /dev/null +++ b/assets/qtmaildir.desktop @@ -0,0 +1,13 @@ +[Desktop Entry] +Type=Application +Version=1.0 +Name=qtmaildir +GenericName=Mail Reader +Comment=Read and organize a local notmuch-indexed Maildir +Exec=qtmaildir +Icon=qtmaildir +Terminal=false +Categories=Network;Email;Qt; +Keywords=mail;email;notmuch;maildir; +StartupNotify=true +StartupWMClass=qtmaildir 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 8847b9b..915c8cb 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 @@ -36,15 +36,17 @@ taking that too literally. |---|------|---------|------|--------| | 1 | Splitter/column widths do not survive restart | persistence | S | open | | 2 | No way to see full message details (From/To/Cc/Subject) | information | M | open | -| 3 | Too few clickable affordances, shortcuts are the only route | discoverability | M | open | +| 3 | Too few clickable affordances, shortcuts are the only route | discoverability | M | **done** | | 4 | Message-pane font size does not survive restart | persistence | S | open | | 5 | Thread list is cramped, poor readability | presentation | S | open | | 6 | Opened message stays unread | behavior | S | open | | 7 | HTML view should be default for HTML messages | behavior | XS | **verify first, may already be done** | -| 8 | No buttons or menu entries for archive, undo, etc | discoverability | M | open | -| 9 | No in-app view of configured shortcuts | discoverability | S | open | +| 8 | No buttons or menu entries for archive, undo, etc | discoverability | M | **done** | +| 9 | No in-app view of configured shortcuts | discoverability | S | **done** | | 10 | Reaching an account's inbox takes two steps | workflow | S | open | -| 11 | Icon, `.desktop` file, SlackBuild | packaging | M | open | +| 11 | Icon, `.desktop` file, SlackBuild | packaging | M | **partly done**: icon and `.desktop` landed, SlackBuild open | +| 13 | No visual feedback that an action stuck | feedback | S | **done** | +| 14 | Tag column unreadable, tags need another home | presentation | M | **done** | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -167,6 +169,35 @@ smuggle in a "Are you sure?" for Delete. **Verification:** the existing keymap test must still pass unchanged, proving user bindings survive the conversion. That is the load-bearing check here. +### Outcome (done) + +Built as described: menu bar, toolbar, and a generated shortcut reference. +Four things the plan did not anticipate, all verified by probe rather than +assumed: + +- **The event filter was removable, but not for the stated reason.** The plan + worried that `QAction` shortcuts might lose to `QAbstractItemView`'s + type-to-search. They do not: shortcut dispatch runs before the focused + widget sees the key. The filter is gone, and the thread view no longer + needs its own. +- **Qt already solves the query-bar case.** A plain-letter shortcut is + suppressed while an editable widget has focus, so the `hasFocus()` guard + was unnecessary. Removing it also fixed `Ctrl+Q`, which the old filter + swallowed while typing a query. +- **Three default bindings had never worked.** `N`, `F` and `G` stored the + unshifted key, which no keystroke emits, so `toggle_unread`, `flag` and + `sync` were dead in 0.1.0. Fixed in `KeyMap::normalizeSequence()` and + committed separately from the menu work. +- **The drift test did become unnecessary**, as the plan hoped. + `registeredActionNames()` is now derived from the `QAction`s, and + `defaultBindings()` is the single source for the defaults. The two tests + that pinned the hand-maintained lists together were replaced by ones that + check a configured binding actually reaches its action. + +Defaults moved to modifier shortcuts, since a single letter cannot be a menu +accelerator without claiming that letter window-wide. Existing `[keys]` +entries are unaffected. + ## 4. Message-pane font size does not survive restart **Observed:** described as "very annoying", more so than item 1. @@ -331,6 +362,80 @@ Packaging, independent of everything above, and can proceed in parallel. --- +## 13. No visual feedback that an action stuck + +**Observed:** selecting a thread and hitting Delete changed nothing on screen. +No way to tell whether the thread was really going to be deleted on the next +sync, which is bad UX for every tag action, not only delete. + +**Cause:** not a missing update. `ThreadListModel::applyTagChange()` already +added the tag and emitted `dataChanged` across the whole row, so the Tags +column did change. But `SubjectColumn` was set to `QHeaderView::Stretch` while +`TagsColumn` came after it, so Subject absorbed all free width and pushed Tags +out of view. The feedback existed in the one column that could not be seen. + +**Approach:** two changes, since the cause was two things. + +- Column order is now Tags, Date, From, Subject. Subject stretches and is + last, so nothing sits to its right to be pushed out. The other three size + to their contents. +- A thread tagged `deleted` or `spam` styles its entire row: muted dark red + (`#8b2c2c`) or orange (`#a85c18`) fill, white text, struck through. Applied + through `Qt::BackgroundRole`, `Qt::ForegroundRole` and `Qt::FontRole` for + every column, so no cue depends on a single column staying visible. + +Strike-through rides along with the fill deliberately: it survives a theme +that overrides background colours, a colourblind reader, and a screenshot. +Bold for unread still composes with it. + +**Decisions:** no status-bar or toast changes, the existing `tagSelected()` +message stays as it is. Archive removes `inbox` and adds nothing, so an +archived thread gets no row styling; whether it should disappear from an inbox +query is deliberately left open rather than guessed at. + +**Verification:** four model tests covering the colours, the strike-through, +that styling spans every column, and that undo restores a plain row. Rendered +and inspected: normal, unread, deleted, spam, and deleted-plus-unread rows. + +--- + +## 14. Tag column unreadable + +**Observed:** with tags spelled out per row the column ran to 500 pixels of +mostly repeated text ("account-privateemail-danilo.macri attachment flagged +inbox passed replied"), dominated by the account prefix, and consumed most of +the list's width. + +**Cause:** presentation, not data. 96 tags in this database, many hierarchical +(`shopping/amazon`, `mailing-list/SBo`), rendered as a joined string. + +**Approach:** the column is gone. Tags now render as coloured chips in two +places, split by taxonomy: + +- The **account tag** says which mailbox a thread came from. It draws as a chip + in front of the subject, coloured and labelled from its own `[account.<key>]` + stanza via new `color` and `label` keys. `label` is display-only; the notmuch + tag is never renamed. +- **Functional tags** say what state a thread is in. They fill a single row + under the message pane, with overflow collapsing into a `+N` chip whose + tooltip lists the hidden ones. A single row keeps the message area from + shifting between threads with different tag counts. + +Colours resolve exact tag first, then top-level prefix, so one `shopping` entry +covers the hierarchy without listing all 96. Unconfigured tags fall back to a +hash of the name, stable so a chip never changes colour as the list scrolls. + +**Defect found while building:** QSettings treats `/` in a key as a group +separator, so `shopping/amazon` becomes a nested key that `childKeys()` never +returns. Reading `[tagcolors]` with `childKeys()` silently dropped every +hierarchical tag, and each fell through to its prefix colour. Fixed by reading +`allKeys()`, with a regression test. The same gotcha is already documented in +`CLAUDE.md` for `[account.work]` section names. + +**Deferred:** clicking a chip to search that tag. Display only for now. + +--- + ## 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 7e4cea8..26cb37c 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -6,6 +6,9 @@ add_library(qtmaildir_lib STATIC htmlbuilder.cpp cidschemehandler.cpp notmuchworker.cpp + tagchip.cpp + tagcolors.cpp + tagstrip.cpp threadlistmodel.cpp mailsync.cpp threadcidmap.cpp @@ -20,7 +23,18 @@ target_include_directories(qtmaildir_lib target_link_libraries(qtmaildir_lib PUBLIC Qt6::Widgets Qt6::WebEngineWidgets PkgConfig::GMIME ${NOTMUCH_LIBRARY}) -add_executable(qtmaildir main.cpp) +# resources.qrc belongs to the executable, not to the static library. A qrc +# compiled into a .a registers itself from a global initialiser, and the linker +# drops that object because nothing references it, so the resource silently +# fails to exist at runtime. +add_executable(qtmaildir main.cpp resources.qrc) target_link_libraries(qtmaildir PRIVATE qtmaildir_lib) install(TARGETS qtmaildir RUNTIME DESTINATION bin) + +# The icon goes into the hicolor theme under its scalable directory, which is +# where a desktop environment looks for the Icon= name in the .desktop entry. +install(FILES ${CMAKE_SOURCE_DIR}/assets/icons/qtmaildir.svg + DESTINATION share/icons/hicolor/scalable/apps) +install(FILES ${CMAKE_SOURCE_DIR}/assets/qtmaildir.desktop + DESTINATION share/applications) diff --git a/src/config.cpp b/src/config.cpp index de6a783..f21bba9 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -89,6 +89,23 @@ void Config::load(const QString &path) account.address = settings.value(QStringLiteral("address")).toString(); account.maildir = settings.value(QStringLiteral("maildir")).toString(); account.drafts = settings.value(QStringLiteral("drafts")).toString(); + + // Both optional, and both describe this account's chip in the thread + // list. An account tag is a different taxonomy from a functional one, + // saying which mailbox a thread arrived in rather than what state it + // is in, so these live here rather than in [tagcolors]. + account.label = settings.value(QStringLiteral("label")).toString(); + + const QString colour = settings.value(QStringLiteral("color")).toString(); + if (!colour.isEmpty()) { + account.color = QColor(colour); + if (!account.color.isValid()) { + addProblem( + QStringLiteral("Account '%1' has an unparseable color '%2'; " + "using a generated one.") + .arg(account.key, colour)); + } + } settings.endGroup(); if (!account.isValid()) { diff --git a/src/config.h b/src/config.h index 270b780..943cbd8 100644 --- a/src/config.h +++ b/src/config.h @@ -18,6 +18,7 @@ #pragma once +#include <QColor> #include <QList> #include <QString> #include <QStringList> @@ -33,6 +34,15 @@ struct Account QString maildir; ///< Relative to notmuch's database.path. QString drafts; ///< Unused in v1; send is v2. + /// Chip colour in the thread list. Invalid when unset, in which case one + /// is generated from the account tag's name. + QColor color; + + /// Text shown on the chip. Empty falls back to the key, which can be long: + /// "privateemail-danilo.macri" is a lot of row for one bit of information. + /// This renames nothing in notmuch, only what the chip displays. + QString label; + bool isValid() const { return !key.isEmpty() && !maildir.isEmpty(); } /// Restricts a notmuch query to this account's subtree. diff --git a/src/keymap.cpp b/src/keymap.cpp index 39991dc..42ccd40 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -41,25 +41,111 @@ QStringList KeyMap::knownActions() }; } -void KeyMap::loadDefaults() +QList<QPair<QString, QString>> KeyMap::defaultBindings() { - const QHash<QString, QString> defaults = { - { QStringLiteral("j"), QStringLiteral("next_thread") }, - { QStringLiteral("k"), QStringLiteral("prev_thread") }, - { QStringLiteral("Return"), QStringLiteral("open_thread") }, - { QStringLiteral("a"), QStringLiteral("archive") }, - { QStringLiteral("d"), QStringLiteral("delete") }, - { QStringLiteral("N"), QStringLiteral("toggle_unread") }, - { QStringLiteral("F"), QStringLiteral("flag") }, - { QStringLiteral("/"), QStringLiteral("focus_query") }, - { QStringLiteral("h"), QStringLiteral("toggle_html") }, - { QStringLiteral("u"), QStringLiteral("undo") }, - { QStringLiteral("G"), QStringLiteral("sync") }, - { QStringLiteral("Ctrl+Q"), QStringLiteral("quit") }, + // Modifier shortcuts throughout, rather than the bare letters of 0.1.0. + // Two reasons. A bare capital never worked: "N" parses to plain Key_N + // while typing a capital emits Shift+N, so toggle_unread, flag and sync + // were dead keys. And a single letter cannot be a QAction shortcut in a + // menu without stealing that letter from every text field in the window. + // + // Ordered as the menus present them; a QList keeps that order, which a + // QHash would not. + return { + { QStringLiteral("Ctrl+J"), QStringLiteral("next_thread") }, + { QStringLiteral("Ctrl+K"), QStringLiteral("prev_thread") }, + { QStringLiteral("Return"), QStringLiteral("open_thread") }, + { QStringLiteral("Ctrl+E"), QStringLiteral("archive") }, + { QStringLiteral("Ctrl+D"), QStringLiteral("delete") }, + { QStringLiteral("Ctrl+Shift+S"), QStringLiteral("spam") }, + { QStringLiteral("Ctrl+U"), QStringLiteral("toggle_unread") }, + { QStringLiteral("Ctrl+I"), QStringLiteral("flag") }, + { QStringLiteral("Ctrl+L"), QStringLiteral("focus_query") }, + { QStringLiteral("Ctrl+H"), QStringLiteral("toggle_html") }, + { QStringLiteral("Ctrl+M"), QStringLiteral("load_remote") }, + { QStringLiteral("Ctrl+Z"), QStringLiteral("undo") }, + { QStringLiteral("Ctrl+G"), QStringLiteral("sync") }, + { QStringLiteral("Ctrl+Q"), QStringLiteral("quit") }, }; +} + +QStringList KeyMap::defaultActions() +{ + QStringList actions; + const auto bindings = defaultBindings(); + actions.reserve(bindings.size()); + for (const auto &binding : bindings) + actions.append(binding.second); + return actions; +} + +QKeySequence KeyMap::normalizeSequence(const QString &text) +{ + const QKeySequence sequence = QKeySequence::fromString(text); + + // fromString() does not return an empty sequence for unparseable input; + // it returns a non-empty one whose toString() is empty (verified on + // Qt 6.11). Both checks are needed to detect garbage. + if (sequence.isEmpty() || sequence.toString().isEmpty()) + return {}; + + // A bare uppercase letter, no modifiers: the user wrote "N" meaning the + // key they press to type a capital N, which is Shift+N. fromString() + // folded the case away, so put the Shift back. + if (text.size() == 1 && text.at(0).isUpper() && text.at(0).isLetter()) + return QKeySequence(sequence[0].key() | Qt::SHIFT); + + return sequence; +} + +void KeyMap::loadDefaults() +{ + for (const auto &binding : defaultBindings()) + m_bindings.insert(normalizeSequence(binding.first), binding.second); +} + +QKeySequence KeyMap::sequenceFor(const QString &action) const +{ + // Several sequences can reach one action: the built-in default, which + // loadOverrides() does not remove, plus whatever the user added. Their + // binding is the one to show and to put on the QAction, or configuring + // "Ctrl+Alt+A = archive" would leave the menu still advertising Ctrl+E. + // + // QHash iteration order is unspecified, so ties are broken on the text + // rather than left to chance. + const QKeySequence builtIn = defaultSequenceFor(action); + QKeySequence best; + bool bestIsBuiltIn = false; + + for (auto it = m_bindings.cbegin(); it != m_bindings.cend(); ++it) { + if (it.value() != action) + continue; + + const bool isBuiltIn = !builtIn.isEmpty() && it.key() == builtIn; + if (best.isEmpty()) { + best = it.key(); + bestIsBuiltIn = isBuiltIn; + continue; + } + // A user binding always beats the default. + if (bestIsBuiltIn && !isBuiltIn) { + best = it.key(); + bestIsBuiltIn = false; + } else if (bestIsBuiltIn == isBuiltIn + && it.key().toString() < best.toString()) { + best = it.key(); + } + } + return best; +} - for (auto it = defaults.cbegin(); it != defaults.cend(); ++it) - m_bindings.insert(QKeySequence::fromString(it.key()), it.value()); +QKeySequence KeyMap::defaultSequenceFor(const QString &action) +{ + for (const auto &binding : defaultBindings()) { + if (binding.second == action) + return normalizeSequence(binding.first); + } + return {}; } void KeyMap::loadOverrides(QSettings &settings) @@ -79,12 +165,10 @@ void KeyMap::loadOverrides(QSettings &settings) for (const QString &key : keys) { const QString action = settings.value(key).toString(); - const QKeySequence sequence = QKeySequence::fromString(key); - // QKeySequence::fromString() does not return an empty sequence for - // unparseable input; it returns a non-empty sequence whose - // toString() is empty (verified on Qt 6.11). Use that to detect - // garbage input instead. - if (sequence.isEmpty() || sequence.toString().isEmpty()) { + // Shares the defaults' normalization, so a hand-written "N" binds the + // key the user actually presses rather than one nothing emits. + const QKeySequence sequence = normalizeSequence(key); + if (sequence.isEmpty()) { m_warnings.append( QStringLiteral("Unparseable key sequence '%1' in [keys]").arg(key)); continue; diff --git a/src/keymap.h b/src/keymap.h index 564eb10..1c7df5f 100644 --- a/src/keymap.h +++ b/src/keymap.h @@ -20,6 +20,8 @@ #include <QHash> #include <QKeySequence> +#include <QList> +#include <QPair> #include <QStringList> class QSettings; @@ -33,6 +35,11 @@ public: /// anything not in this set, so a typo in the config cannot bind silently. static QStringList knownActions(); + /// The built-in bindings, in menu order: {sequence, action}. The single + /// source of truth for the defaults, so the menus, the shortcut reference + /// and loadDefaults() cannot disagree about them. + static QList<QPair<QString, QString>> defaultBindings(); + void loadDefaults(); /// Reads the [keys] group. Invalid sequences and unknown action names are @@ -42,6 +49,26 @@ public: /// Empty string when nothing is bound. QString actionFor(const QKeySequence &sequence) const; + /// The sequence currently bound to an action, empty if none. The reverse + /// of actionFor(): menus need a shortcut for an action they already know. + /// When several sequences are bound to one action, returns the shortest + /// text, so the menu shows a stable choice rather than a hash-order one. + QKeySequence sequenceFor(const QString &action) const; + + /// The built-in sequence for an action, ignoring any user override. + static QKeySequence defaultSequenceFor(const QString &action); + + /// Every action name carrying a built-in binding. + static QStringList defaultActions(); + + /// Normalizes a configured key string into the sequence a real keypress + /// produces. QKeySequence::fromString() discards the case of a bare + /// letter, so "N" parses to plain Key_N, which no keystroke ever emits: + /// typing a capital sends Shift+N. A bare uppercase letter is therefore + /// rewritten to Shift+<letter>. Returns an empty sequence for input + /// fromString() cannot parse. + static QKeySequence normalizeSequence(const QString &text); + QStringList warnings() const { return m_warnings; } private: diff --git a/src/main.cpp b/src/main.cpp index 4d3ed0a..231594f 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -17,6 +17,7 @@ */ #include <QApplication> +#include <QIcon> #include <QMessageBox> #include <QWebEngineUrlScheme> @@ -76,6 +77,13 @@ int main(int argc, char *argv[]) app.setOrganizationName(QStringLiteral("qtmaildir")); app.setApplicationVersion(QStringLiteral(QTMAILDIR_VERSION)); + // Compiled in rather than read from disk, so the icon is there whether or + // not the app was installed. setDesktopFileName() is what lets a Wayland + // compositor match the window to its .desktop entry, which is where the + // taskbar icon really comes from there. + app.setWindowIcon(QIcon(QStringLiteral(":/icons/qtmaildir.svg"))); + app.setDesktopFileName(QStringLiteral("qtmaildir")); + // Fail loudly on an ABI mismatch rather than crashing later. if (LIBNOTMUCH_MAJOR_VERSION < 5) { QMessageBox::critical(nullptr, QObject::tr("qtmaildir"), diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 2a484fb..48b475c 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -18,13 +18,16 @@ #include "mainwindow.h" +#include <QAction> #include <QComboBox> -#include <QEvent> +#include <QDialog> +#include <QDialogButtonBox> #include <QHBoxLayout> #include <QHeaderView> -#include <QKeyEvent> #include <QLabel> #include <QLineEdit> +#include <QMenu> +#include <QMenuBar> #include <QMessageBox> #include <QPlainTextEdit> #include <QPushButton> @@ -32,35 +35,24 @@ #include <QSplitter> #include <QStatusBar> #include <QTableView> +#include <QToolBar> #include <QVBoxLayout> #include "mailsync.h" #include "messageview.h" #include "mimeparser.h" #include "notmuchworker.h" +#include "tagchip.h" #include "threadlistmodel.h" #include "version.h" -QStringList MainWindow::registeredActionNames() +QStringList MainWindow::registeredActionNames() const { - // Keep in sync with registerActions(). Held against KeyMap::knownActions() - // by a test rather than by hope. - return { - QStringLiteral("next_thread"), - QStringLiteral("prev_thread"), - QStringLiteral("open_thread"), - QStringLiteral("archive"), - QStringLiteral("delete"), - QStringLiteral("spam"), - QStringLiteral("toggle_unread"), - QStringLiteral("flag"), - QStringLiteral("focus_query"), - QStringLiteral("toggle_html"), - QStringLiteral("load_remote"), - QStringLiteral("undo"), - QStringLiteral("sync"), - QStringLiteral("quit"), - }; + // Derived from the actions themselves, so it cannot drift from what + // registerActions() really installed. + QStringList names = m_actions.keys(); + names.sort(); + return names; } QString MainWindow::cidPrefixForIndex(int index) @@ -82,23 +74,28 @@ MainWindow::MainWindow(const Config &config, QWidget *parent) { QSettings settings(Config::defaultPath(), QSettings::IniFormat); m_keyMap.loadOverrides(settings); + m_tagColors.load(settings); + } + + // An account's chip colour comes from its own stanza, since an account tag + // is a different taxonomy from a functional one. + for (const Account &account : m_config.accounts()) { + m_tagColors.setAccountColour(account.key, account.color); + m_tagColors.setAccountLabel(account.key, account.label); } buildUi(); registerActions(); + buildMenus(); wireWorker(); showWarnings(); - installEventFilter(this); - - // The thread view needs its own filter, not just the window's. A filter on - // the window only sees key presses the focused child did not consume, and - // QAbstractItemView consumes plain letters for its type-to-search feature: - // with the list focused, 'h' jumped to the next thread whose subject began - // with "h" instead of toggling HTML, and every other single-letter binding - // (j, k, a, d, N, F, u, G) was swallowed the same way. Filtering the view - // itself puts the keymap ahead of that search. - m_threadView->installEventFilter(this); + // No event filter: QAction shortcuts are dispatched before the focused + // widget sees the key, so they beat QAbstractItemView's type-to-search + // without one. Qt also suppresses a plain-letter shortcut while an + // editable widget has focus, so typing in the query bar stays typing; + // modifier shortcuts such as Ctrl+Q still work there, which the old + // filter blocked. if (!m_config.savedQueries().isEmpty()) { m_queryEdit->setText(m_config.savedQueries().first().query); @@ -175,20 +172,41 @@ void MainWindow::buildUi() // Thread list and message pane. m_model = new ThreadListModel(this); + m_model->setTagColors(&m_tagColors); m_threadView = new QTableView(central); m_threadView->setModel(m_model); m_threadView->setSelectionBehavior(QAbstractItemView::SelectRows); m_threadView->setSelectionMode(QAbstractItemView::ExtendedSelection); m_threadView->verticalHeader()->hide(); m_threadView->horizontalHeader()->setStretchLastSection(false); - m_threadView->horizontalHeader()->setSectionResizeMode( - ThreadListModel::SubjectColumn, QHeaderView::Stretch); + // Every column Interactive, Subject included: Stretch and ResizeToContents + // both compute a width and discard the user's drag. Nothing absorbs spare + // width as a result, so the columns end where they end. + for (int column = 0; column < ThreadListModel::ColumnCount; ++column) { + m_threadView->horizontalHeader()->setSectionResizeMode( + column, QHeaderView::Interactive); + } + + // The subject cell carries the account chip in front of its text. + m_threadView->setItemDelegateForColumn(ThreadListModel::SubjectColumn, + new SubjectDelegate(this)); + // Widening a column past the viewport scrolls rather than squeezing the + // others. Per-pixel so the scroll does not jump a whole column at a time. + m_threadView->setHorizontalScrollBarPolicy(Qt::ScrollBarAsNeeded); + m_threadView->setHorizontalScrollMode(QAbstractItemView::ScrollPerPixel); + + // Starting widths only; a drag overrides them, and they are what the + // saved-widths item will persist. + m_threadView->setColumnWidth(ThreadListModel::DateColumn, 130); + m_threadView->setColumnWidth(ThreadListModel::AuthorsColumn, 180); + m_threadView->setColumnWidth(ThreadListModel::SubjectColumn, 520); connect(m_threadView->selectionModel(), &QItemSelectionModel::currentRowChanged, this, &MainWindow::onThreadSelected); m_messageView = new MessageView(central); + m_messageView->setTagColors(&m_tagColors); connect(m_messageView, &MessageView::statusMessage, this, [this](const QString &text) { m_statusLabel->setText(text); }); @@ -206,40 +224,77 @@ void MainWindow::buildUi() setWindowTitle(QStringLiteral("qtmaildir %1").arg(QTMAILDIR_VERSION)); } +QAction *MainWindow::addAction(const QString &name, const QString &text, + const QString &description, + const std::function<void()> &handler) +{ + auto *action = new QAction(text, this); + action->setObjectName(name); + action->setStatusTip(description); + m_actionDescriptions.insert(name, description); + + // The binding comes from KeyMap, so a [keys] override reaches the menus + // and the shortcut reference as well as the keyboard. + const QKeySequence sequence = m_keyMap.sequenceFor(name); + if (!sequence.isEmpty()) + action->setShortcut(sequence); + + // Shortcuts must work while focus is in the thread list or the message + // view, not only on the window itself. + action->setShortcutContext(Qt::WindowShortcut); + + connect(action, &QAction::triggered, this, handler); + + // Added to the window so the shortcut is live even before the action is + // put in a menu; the ones that never reach a menu depend on this. + QMainWindow::addAction(action); + m_actions.insert(name, action); + return action; +} + void MainWindow::registerActions() { - m_actions[QStringLiteral("focus_query")] = [this]() { + addAction(QStringLiteral("focus_query"), tr("&Find"), + tr("Focus and select the query bar"), [this]() { m_queryEdit->setFocus(); m_queryEdit->selectAll(); - }; - m_actions[QStringLiteral("next_thread")] = [this]() { + }); + addAction(QStringLiteral("next_thread"), tr("&Next thread"), + tr("Select the next thread"), [this]() { const QModelIndex current = m_threadView->currentIndex(); const int row = current.isValid() ? current.row() + 1 : 0; if (row < m_model->rowCount()) m_threadView->selectRow(row); - }; - m_actions[QStringLiteral("prev_thread")] = [this]() { + }); + addAction(QStringLiteral("prev_thread"), tr("&Previous thread"), + tr("Select the previous thread"), [this]() { const QModelIndex current = m_threadView->currentIndex(); if (current.isValid() && current.row() > 0) m_threadView->selectRow(current.row() - 1); - }; - m_actions[QStringLiteral("open_thread")] = [this]() { + }); + addAction(QStringLiteral("open_thread"), tr("&Open thread"), + tr("Focus the thread list"), [this]() { m_threadView->setFocus(); - }; - m_actions[QStringLiteral("archive")] = [this]() { + }); + addAction(QStringLiteral("archive"), tr("&Archive"), + tr("Remove inbox from every selected thread"), [this]() { tagSelected({}, { QStringLiteral("inbox") }, tr("Archive")); - }; - m_actions[QStringLiteral("delete")] = [this]() { + }); + addAction(QStringLiteral("delete"), tr("&Delete"), + tr("Add the deleted tag"), [this]() { tagSelected({ QStringLiteral("deleted") }, {}, tr("Delete")); - }; - m_actions[QStringLiteral("spam")] = [this]() { + }); + addAction(QStringLiteral("spam"), tr("Mark &spam"), + tr("Add spam and remove inbox"), [this]() { tagSelected({ QStringLiteral("spam") }, { QStringLiteral("inbox") }, tr("Mark spam")); - }; - m_actions[QStringLiteral("flag")] = [this]() { + }); + addAction(QStringLiteral("flag"), tr("&Flag"), + tr("Add the flagged tag"), [this]() { tagSelected({ QStringLiteral("flagged") }, {}, tr("Flag")); - }; - m_actions[QStringLiteral("toggle_unread")] = [this]() { + }); + addAction(QStringLiteral("toggle_unread"), tr("Toggle &unread"), + tr("Toggle the unread tag"), [this]() { // The direction comes from the current row, but the change applies to // the whole selection, so a mixed selection lands in one consistent // state rather than each row flipping its own way. @@ -251,28 +306,176 @@ void MainWindow::registerActions() tagSelected({}, { QStringLiteral("unread") }, tr("Mark read")); else tagSelected({ QStringLiteral("unread") }, {}, tr("Mark unread")); - }; - m_actions[QStringLiteral("toggle_html")] = [this]() { + }); + addAction(QStringLiteral("toggle_html"), tr("Toggle &HTML"), + tr("Switch the thread between HTML and plain text"), [this]() { m_messageView->toggleHtml(); - }; - m_actions[QStringLiteral("load_remote")] = [this]() { + }); + addAction(QStringLiteral("load_remote"), tr("Load &remote content"), + tr("Load remote images for the current thread"), [this]() { m_messageView->loadRemoteContent(); - }; - m_actions[QStringLiteral("undo")] = [this]() { + }); + addAction(QStringLiteral("undo"), tr("&Undo"), + tr("Undo the last tag change"), [this]() { if (m_undoStack.canUndo()) m_undoStack.undo(); else m_statusLabel->setText(tr("Nothing to undo")); - }; - m_actions[QStringLiteral("sync")] = [this]() { + }); + addAction(QStringLiteral("sync"), tr("&Sync"), + tr("Run the configured sync command"), [this]() { if (m_sync->isAvailable()) m_sync->start(); + }); + addAction(QStringLiteral("quit"), tr("&Quit"), + tr("Quit qtmaildir"), [this]() { close(); }); + + // A binding the user wrote for an action that does not exist would be + // silently dead. KeyMap warns about unknown names, but only a check here + // catches the reverse: a known action nothing implements. + Q_ASSERT(m_actions.size() == KeyMap::knownActions().size()); +} + +void MainWindow::buildMenus() +{ + auto *fileMenu = menuBar()->addMenu(tr("&File")); + fileMenu->addAction(m_actions.value(QStringLiteral("sync"))); + fileMenu->addSeparator(); + fileMenu->addAction(m_actions.value(QStringLiteral("quit"))); + + auto *editMenu = menuBar()->addMenu(tr("&Edit")); + editMenu->addAction(m_actions.value(QStringLiteral("undo"))); + editMenu->addSeparator(); + editMenu->addAction(m_actions.value(QStringLiteral("focus_query"))); + + auto *messageMenu = menuBar()->addMenu(tr("&Message")); + messageMenu->addAction(m_actions.value(QStringLiteral("archive"))); + messageMenu->addAction(m_actions.value(QStringLiteral("delete"))); + messageMenu->addAction(m_actions.value(QStringLiteral("spam"))); + messageMenu->addSeparator(); + messageMenu->addAction(m_actions.value(QStringLiteral("toggle_unread"))); + messageMenu->addAction(m_actions.value(QStringLiteral("flag"))); + + auto *viewMenu = menuBar()->addMenu(tr("&View")); + viewMenu->addAction(m_actions.value(QStringLiteral("prev_thread"))); + viewMenu->addAction(m_actions.value(QStringLiteral("next_thread"))); + viewMenu->addSeparator(); + viewMenu->addAction(m_actions.value(QStringLiteral("toggle_html"))); + viewMenu->addAction(m_actions.value(QStringLiteral("load_remote"))); + + auto *helpMenu = menuBar()->addMenu(tr("&Help")); + auto *shortcuts = helpMenu->addAction(tr("&Keyboard shortcuts")); + connect(shortcuts, &QAction::triggered, + this, &MainWindow::showShortcutReference); + auto *about = helpMenu->addAction(tr("&About")); + connect(about, &QAction::triggered, this, &MainWindow::showAbout); + + // Standard names from the icon theme, so the buttons match the rest of the + // desktop rather than shipping bespoke art. A theme that lacks one leaves + // that action with text alone, which still works. + const QHash<QString, QString> themeIcons = { + { QStringLiteral("sync"), QStringLiteral("mail-receive") }, + { QStringLiteral("archive"), QStringLiteral("mail-mark-read") }, + { QStringLiteral("delete"), QStringLiteral("edit-delete") }, + { QStringLiteral("undo"), QStringLiteral("edit-undo") }, + { QStringLiteral("spam"), QStringLiteral("mail-mark-junk") }, + { QStringLiteral("flag"), QStringLiteral("mail-mark-important") }, + { QStringLiteral("quit"), QStringLiteral("application-exit") }, + { QStringLiteral("focus_query"), QStringLiteral("edit-find") }, }; - m_actions[QStringLiteral("quit")] = [this]() { close(); }; + for (auto it = themeIcons.cbegin(); it != themeIcons.cend(); ++it) { + QAction *action = m_actions.value(it.key()); + if (!action) + continue; + const QIcon icon = QIcon::fromTheme(it.value()); + if (!icon.isNull()) + action->setIcon(icon); + } + + // The frequent subset only. A toolbar holding every action is as + // unreadable as no toolbar. + auto *toolBar = addToolBar(tr("Main")); + toolBar->setObjectName(QStringLiteral("main_toolbar")); + toolBar->setToolButtonStyle(Qt::ToolButtonTextBesideIcon); + toolBar->addAction(m_actions.value(QStringLiteral("sync"))); + toolBar->addSeparator(); + toolBar->addAction(m_actions.value(QStringLiteral("archive"))); + toolBar->addAction(m_actions.value(QStringLiteral("delete"))); + toolBar->addSeparator(); + toolBar->addAction(m_actions.value(QStringLiteral("undo"))); +} + +void MainWindow::showShortcutReference() +{ + // Generated from the actions, so it cannot disagree with what the keys + // really do. A hand-written list would drift the first time a binding + // changed. + QStringList rows; + for (const QString &name : registeredActionNames()) { + const QAction *action = m_actions.value(name); + if (!action) + continue; + const QString sequence = action->shortcut().toString(QKeySequence::NativeText); + rows.append(QStringLiteral("<tr><td><tt>%1</tt> </td>" + "<td>%2 </td>" + "<td><tt>%3</tt></td></tr>") + .arg(sequence.isEmpty() ? tr("(unbound)") : sequence.toHtmlEscaped(), + m_actionDescriptions.value(name).toHtmlEscaped(), + name.toHtmlEscaped())); + } - // The two lists are maintained by hand and a test pins them together; this - // catches the same drift in a debug run. - Q_ASSERT(m_actions.size() == registeredActionNames().size()); + // Two columns rather than one. Fourteen actions in a single table made a + // dialog taller than the screen, which cut off its own title bar. + const int half = (rows.size() + 1) / 2; + const QString header = + tr("<tr><th align='left'>Key</th><th align='left'>Does</th>" + "<th align='left'>Action name</th></tr>"); + const QString left = header + rows.mid(0, half).join(QString()); + const QString right = header + rows.mid(half).join(QString()); + + // A QDialog rather than QMessageBox: the message box wraps its text at a + // narrow default width, which turned every description into a column of + // single words and made the dialog taller than the screen. + QDialog dialog(this); + dialog.setWindowTitle(tr("Keyboard shortcuts")); + + auto *label = new QLabel(&dialog); + label->setTextFormat(Qt::RichText); + label->setText(tr("<table cellspacing='0'><tr>" + "<td valign='top'><table cellpadding='3'>%1</table></td>" + "<td width='32'></td>" + "<td valign='top'><table cellpadding='3'>%2</table></td>" + "</tr></table>") + .arg(left, right)); + + auto *note = new QLabel( + tr("Rebind any of these in the <tt>[keys]</tt> section of " + "<tt>qtmaildir.conf</tt>, using the action name."), + &dialog); + note->setTextFormat(Qt::RichText); + note->setWordWrap(true); + + auto *buttons = new QDialogButtonBox(QDialogButtonBox::Ok, &dialog); + connect(buttons, &QDialogButtonBox::accepted, &dialog, &QDialog::accept); + + auto *layout = new QVBoxLayout(&dialog); + layout->addWidget(label); + layout->addWidget(note); + layout->addStretch(); + layout->addWidget(buttons); + + dialog.exec(); +} + +void MainWindow::showAbout() +{ + QMessageBox::about( + this, tr("About qtmaildir"), + tr("<h3>qtmaildir %1</h3>" + "<p>A Qt6 mail client for notmuch-indexed Maildirs.</p>" + "<p>Reads and organizes local mail. Fetching and sending are " + "handled by external scripts.</p>") + .arg(QStringLiteral(QTMAILDIR_VERSION))); } void MainWindow::wireWorker() @@ -376,7 +579,9 @@ void MainWindow::onThreadSelected(const QModelIndex ¤t, if (!current.isValid()) return; - m_currentThreadId = m_model->threadAt(current.row()).threadId; + const ThreadSummary thread = m_model->threadAt(current.row()); + m_currentThreadId = thread.threadId; + m_messageView->setTags(thread.tags); QMetaObject::invokeMethod(m_worker, "loadThread", Qt::QueuedConnection, Q_ARG(QString, m_currentThreadId), Q_ARG(QString, m_lastQuery), @@ -498,6 +703,14 @@ void MainWindow::sendThreadTagChange(const QStringList &threadIds, for (const QString &threadId : threadIds) m_model->applyTagChange(threadId, add, remove); + // The strip shows the open thread's tags, so it has to follow a change to + // that thread rather than waiting for the next selection. + if (threadIds.contains(m_currentThreadId)) { + const QModelIndex current = m_threadView->currentIndex(); + if (current.isValid()) + m_messageView->setTags(m_model->threadAt(current.row()).tags); + } + m_pendingThreadIds = threadIds; m_pendingChange = TagChange{ {}, add, remove, description }; @@ -511,23 +724,3 @@ void MainWindow::sendThreadTagChange(const QStringList &threadIds, Q_ARG(QString, description)); } -bool MainWindow::eventFilter(QObject *watched, QEvent *event) -{ - if (event->type() != QEvent::KeyPress) - return QMainWindow::eventFilter(watched, event); - - // The query bar must receive ordinary typing, so single-key bindings are - // suppressed while it has focus. - if (m_queryEdit->hasFocus()) - return QMainWindow::eventFilter(watched, event); - - auto *keyEvent = static_cast<QKeyEvent *>(event); - const QKeySequence sequence(keyEvent->keyCombination()); - - const QString action = m_keyMap.actionFor(sequence); - if (action.isEmpty() || !m_actions.contains(action)) - return QMainWindow::eventFilter(watched, event); - - m_actions.value(action)(); - return true; -} diff --git a/src/mainwindow.h b/src/mainwindow.h index 30679bb..4445894 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -28,8 +28,10 @@ #include "config.h" #include "keymap.h" +#include "tagcolors.h" #include "types.h" +class QAction; class QLineEdit; class QTableView; class QLabel; @@ -49,10 +51,10 @@ public: explicit MainWindow(const Config &config, QWidget *parent = nullptr); ~MainWindow() override; - /// Every action name registerActions() installs. Exposed so a test can hold - /// it against KeyMap::knownActions(): the two lists are maintained by hand, - /// and a drift either way silently breaks a user's key binding. - static QStringList registeredActionNames(); + /// Every action name registerActions() installs. Derived from the actions + /// themselves rather than hand-maintained, so it cannot drift from what is + /// really registered. + QStringList registeredActionNames() const; /// The cid: namespace prefix for the nth message of a thread. /// @@ -61,9 +63,6 @@ public: /// cid: references from resolving to another's. static QString cidPrefixForIndex(int index); -protected: - bool eventFilter(QObject *watched, QEvent *event) override; - private slots: void runCurrentQuery(); void onThreadsReady(const QVector<ThreadSummary> &threads, quint64 generation); @@ -76,8 +75,17 @@ private slots: private: void buildUi(); void registerActions(); + void buildMenus(); void wireWorker(); void showWarnings(); + void showShortcutReference(); + void showAbout(); + + /// Creates a QAction, binds it to the sequence KeyMap holds for `name`, + /// and registers it. `name` is the action name used in [keys]. + QAction *addAction(const QString &name, const QString &text, + const QString &description, + const std::function<void()> &handler); void tagSelected(const QStringList &add, const QStringList &remove, const QString &description); @@ -96,6 +104,7 @@ private: Config m_config; KeyMap m_keyMap; + TagColors m_tagColors; QThread m_workerThread; NotmuchWorker *m_worker = nullptr; @@ -112,7 +121,15 @@ private: QLabel *m_statusLabel = nullptr; QPlainTextEdit *m_syncLog = nullptr; - QHash<QString, std::function<void()>> m_actions; + /// Action name (as used in [keys]) to the QAction implementing it. Owned + /// by the window through the QObject parent, not by this hash. + QHash<QString, QAction *> m_actions; + + /// One-line description per action, for the shortcut reference. Kept + /// beside the actions so the dialog is generated, never hand-written in + /// parallel with them. + QHash<QString, QString> m_actionDescriptions; + quint64 m_generation = 0; QString m_lastQuery; QString m_currentThreadId; diff --git a/src/messageview.cpp b/src/messageview.cpp index 3bb08a0..aebb81b 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -34,6 +34,7 @@ #include "cidschemehandler.h" #include "htmlbuilder.h" #include "requestinterceptor.h" +#include "tagstrip.h" #include "threadcidmap.h" namespace { @@ -118,17 +119,33 @@ MessageView::MessageView(QWidget *parent) m_attachmentBar = new QWidget(this); new QHBoxLayout(m_attachmentBar); + // Tags live under the message rather than in the thread list, where + // spelling them out cost most of the list's width. + m_tagStrip = new TagStrip(this); + m_tagStrip->hide(); + auto *layout = new QVBoxLayout(this); layout->addWidget(m_headerLabel); layout->addLayout(blockedRow); layout->addWidget(m_view, 1); layout->addWidget(m_attachmentBar); + layout->addWidget(m_tagStrip); clear(); } MessageView::~MessageView() = default; +void MessageView::setTagColors(const TagColors *colours) +{ + m_tagStrip->setTagColors(colours); +} + +void MessageView::setTags(const QStringList &tags) +{ + m_tagStrip->setTags(tags); +} + /// The single place that loads a document into the view. /// /// RequestInterceptor trusts exactly one qtmaildir: URL and denies every other @@ -145,6 +162,7 @@ void MessageView::setDocument(const QString &html) void MessageView::clear() { m_items.clear(); + m_tagStrip->setTags({}); // No thread is displayed, so nothing may be served or allowed. Without // this, the previous thread's parts would stay reachable. diff --git a/src/messageview.h b/src/messageview.h index f3bd96f..9570db5 100644 --- a/src/messageview.h +++ b/src/messageview.h @@ -30,6 +30,8 @@ class QPushButton; class QWebEngineView; class QWebEngineProfile; class CidSchemeHandler; +class TagColors; +class TagStrip; class RequestInterceptor; /// The message pane: thread header, body, attachment bar. @@ -57,6 +59,12 @@ public: void showError(const QString &text, const QString &filePath); void clear(); + /// Supplies the tag strip's colours. Not owned; must outlive the view. + void setTagColors(const TagColors *colours); + + /// Tags of the thread on display, shown as chips along the bottom. + void setTags(const QStringList &tags); + public slots: void toggleHtml(); void loadRemoteContent(); @@ -81,4 +89,5 @@ private: QLabel *m_blockedLabel = nullptr; QPushButton *m_loadRemoteButton = nullptr; QWidget *m_attachmentBar = nullptr; + TagStrip *m_tagStrip = nullptr; }; diff --git a/src/resources.qrc b/src/resources.qrc new file mode 100644 index 0000000..7bdb592 --- /dev/null +++ b/src/resources.qrc @@ -0,0 +1,6 @@ +<!DOCTYPE RCC> +<RCC version="1.0"> + <qresource prefix="/"> + <file alias="icons/qtmaildir.svg">../assets/icons/qtmaildir.svg</file> + </qresource> +</RCC> diff --git a/src/tagchip.cpp b/src/tagchip.cpp new file mode 100644 index 0000000..2e21419 --- /dev/null +++ b/src/tagchip.cpp @@ -0,0 +1,126 @@ +/* + * 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 "tagchip.h" + +#include <QApplication> +#include <QFontMetrics> +#include <QPainter> + +#include "tagcolors.h" +#include "threadlistmodel.h" + +namespace TagChip { + +QSize sizeFor(const QFontMetrics &metrics, const QString &text) +{ + return QSize(metrics.horizontalAdvance(text) + kPaddingX * 2, + metrics.height() + kPaddingY * 2); +} + +void paint(QPainter *painter, const QRect &rect, const QString &text, + const QColor &background) +{ + painter->save(); + painter->setRenderHint(QPainter::Antialiasing, true); + painter->setPen(Qt::NoPen); + painter->setBrush(background); + painter->drawRoundedRect(rect, kRadius, kRadius); + + painter->setPen(TagColors::textColourOn(background)); + painter->drawText(rect, Qt::AlignCenter, text); + painter->restore(); +} + +} // namespace TagChip + +void SubjectDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, + const QModelIndex &index) const +{ + const QString account = + index.data(ThreadListModel::AccountLabelRole).toString(); + if (account.isEmpty()) { + QStyledItemDelegate::paint(painter, option, index); + return; + } + + // Draw the row's own background and selection first, then the chip and the + // subject on top, so a selected or struck-through row still looks right. + QStyleOptionViewItem chrome = option; + initStyleOption(&chrome, index); + chrome.text.clear(); + const QWidget *widget = option.widget; + QStyle *style = widget ? widget->style() : QApplication::style(); + style->drawControl(QStyle::CE_ItemViewItem, &chrome, painter, widget); + + const QFontMetrics metrics(option.font); + const QSize chipSize = TagChip::sizeFor(metrics, account); + const QRect chipRect(option.rect.left() + TagChip::kSpacing, + option.rect.top() + + (option.rect.height() - chipSize.height()) / 2, + chipSize.width(), chipSize.height()); + + const QColor colour = + index.data(ThreadListModel::AccountColourRole).value<QColor>(); + TagChip::paint(painter, chipRect, account, + colour.isValid() ? colour : QColor(0x55, 0x55, 0x5f)); + + // The subject follows the chip, elided so a long one cannot overflow. + QRect textRect = option.rect; + textRect.setLeft(chipRect.right() + TagChip::kSpacing * 2); + if (textRect.width() <= 0) + return; + + painter->save(); + // The model supplies the row's colours; honouring them keeps a deleted + // thread white-on-red here as everywhere else. + const QVariant foreground = index.data(Qt::ForegroundRole); + if (foreground.isValid()) + painter->setPen(foreground.value<QBrush>().color()); + else if (option.state & QStyle::State_Selected) + painter->setPen(option.palette.highlightedText().color()); + else + painter->setPen(option.palette.text().color()); + + // The model's font carries bold for unread and strike-out for deleted. + // initStyleOption() already resolved it into chrome.font; using it rather + // than option.font is what keeps those cues on a delegate-drawn subject. + const QVariant fontData = index.data(Qt::FontRole); + const QFont rowFont = fontData.isValid() ? fontData.value<QFont>() + : chrome.font; + painter->setFont(rowFont); + const QFontMetrics rowMetrics(rowFont); + painter->drawText(textRect, Qt::AlignVCenter | Qt::AlignLeft, + rowMetrics.elidedText(index.data(Qt::DisplayRole).toString(), + Qt::ElideRight, textRect.width())); + painter->restore(); +} + +QSize SubjectDelegate::sizeHint(const QStyleOptionViewItem &option, + const QModelIndex &index) const +{ + QSize size = QStyledItemDelegate::sizeHint(option, index); + const QString account = + index.data(ThreadListModel::AccountLabelRole).toString(); + if (!account.isEmpty()) { + const QFontMetrics metrics(option.font); + size.setWidth(size.width() + TagChip::sizeFor(metrics, account).width() + + TagChip::kSpacing * 3); + } + return size; +} diff --git a/src/tagchip.h b/src/tagchip.h new file mode 100644 index 0000000..9bd4e78 --- /dev/null +++ b/src/tagchip.h @@ -0,0 +1,62 @@ +/* + * 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 <QColor> +#include <QRect> +#include <QSize> +#include <QString> +#include <QStyledItemDelegate> + +class QPainter; +class QFontMetrics; + +/// Draws one rounded, filled tag chip. Shared so the account chip in the +/// thread list and the strip under the message pane cannot drift apart. +namespace TagChip { + +/// Padding inside a chip and the gap between two of them. +constexpr int kPaddingX = 6; +constexpr int kPaddingY = 1; +constexpr int kSpacing = 4; +constexpr int kRadius = 3; + +QSize sizeFor(const QFontMetrics &metrics, const QString &text); + +/// Paints the chip into `rect`, using `text` and `background`. The text colour +/// is derived from the fill so it stays legible. +void paint(QPainter *painter, const QRect &rect, const QString &text, + const QColor &background); + +} // namespace TagChip + +/// Item delegate for the subject column: draws the account chip in front of +/// the subject text, so which mailbox a thread came from reads at a glance +/// without a tags column spelling it out. +class SubjectDelegate : public QStyledItemDelegate +{ + Q_OBJECT +public: + using QStyledItemDelegate::QStyledItemDelegate; + + void paint(QPainter *painter, const QStyleOptionViewItem &option, + const QModelIndex &index) const override; + QSize sizeHint(const QStyleOptionViewItem &option, + const QModelIndex &index) const override; +}; diff --git a/src/tagcolors.cpp b/src/tagcolors.cpp new file mode 100644 index 0000000..88ca2ab --- /dev/null +++ b/src/tagcolors.cpp @@ -0,0 +1,171 @@ +/* + * 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 "tagcolors.h" + +#include <QCryptographicHash> +#include <QSettings> + +namespace { + +/// Colours for the tags every notmuch setup has. Chosen to stay legible on a +/// dark theme, which is where the message pane already sits. +QHash<QString, QColor> builtInColours() +{ + return { + { QStringLiteral("flagged"), QColor(0xd4, 0x9c, 0x1a) }, + { QStringLiteral("unread"), QColor(0x2f, 0x6f, 0xa8) }, + { QStringLiteral("deleted"), QColor(0x8b, 0x2c, 0x2c) }, + { QStringLiteral("spam"), QColor(0xa8, 0x5c, 0x18) }, + { QStringLiteral("attachment"), QColor(0x5a, 0x5a, 0x64) }, + { QStringLiteral("replied"), QColor(0x3d, 0x7a, 0x4a) }, + { QStringLiteral("passed"), QColor(0x3d, 0x7a, 0x62) }, + { QStringLiteral("draft"), QColor(0x77, 0x66, 0x33) }, + { QStringLiteral("encrypted"), QColor(0x6a, 0x4a, 0x8a) }, + { QStringLiteral("signed"), QColor(0x53, 0x4a, 0x8a) }, + { QStringLiteral("inbox"), QColor(0x44, 0x4a, 0x52) }, + { QStringLiteral("mailing-list"), QColor(0x36, 0x6a, 0x6a) }, + }; +} + +} // namespace + +bool TagColors::isAccountTag(const QString &tag) +{ + // The prefix alone, with nothing after it, names no account. + return tag.startsWith(accountTagPrefix()) + && tag.size() > accountTagPrefix().size(); +} + +QString TagColors::accountKeyForTag(const QString &tag) +{ + if (!isAccountTag(tag)) + return {}; + return tag.mid(accountTagPrefix().size()); +} + +QString TagColors::tagForAccountKey(const QString &key) +{ + return accountTagPrefix() + key; +} + +QColor TagColors::textColourOn(const QColor &background) +{ + // Perceived luminance: the eye weights green far above blue, so a plain + // average would call a saturated blue "light" and print black on it. + const double luminance = (0.299 * background.red() + + 0.587 * background.green() + + 0.114 * background.blue()) / 255.0; + return luminance > 0.55 ? QColor(Qt::black) : QColor(Qt::white); +} + +QString TagColors::topLevelPrefix(const QString &tag) +{ + const int slash = tag.indexOf(QLatin1Char('/')); + return slash < 0 ? tag : tag.left(slash); +} + +void TagColors::load(QSettings &settings) +{ + settings.beginGroup(QStringLiteral("tagcolors")); + // allKeys(), not childKeys(): QSettings treats '/' in a key as a group + // separator, so a hierarchical tag like shopping/amazon becomes a nested + // key that childKeys() does not return. allKeys() reports both, and the + // nested one comes back in the "shopping/amazon" form the tag already has. + // (In the INI file itself it is written as shopping\amazon.) + const QStringList keys = settings.allKeys(); + for (const QString &key : keys) { + const QString value = settings.value(key).toString(); + const QColor colour(value); + if (!colour.isValid()) { + m_warnings.append( + QStringLiteral("Unparseable colour '%1' for tag '%2' in " + "[tagcolors]").arg(value, key)); + continue; + } + m_colours.insert(key, colour); + } + settings.endGroup(); +} + +void TagColors::setAccountColour(const QString &accountKey, const QColor &colour) +{ + if (accountKey.isEmpty() || !colour.isValid()) + return; + m_accountColours.insert(accountKey, colour); +} + +void TagColors::setAccountLabel(const QString &accountKey, const QString &label) +{ + if (accountKey.isEmpty() || label.isEmpty()) + return; + m_accountLabels.insert(accountKey, label); +} + +QString TagColors::labelForAccountTag(const QString &tag) const +{ + const QString key = accountKeyForTag(tag); + if (key.isEmpty()) + return {}; + return m_accountLabels.value(key, key); +} + +bool TagColors::hasColour(const QString &tag) const +{ + if (isAccountTag(tag)) + return m_accountColours.contains(accountKeyForTag(tag)); + + const QHash<QString, QColor> builtIn = builtInColours(); + return m_colours.contains(tag) || builtIn.contains(tag) + || m_colours.contains(topLevelPrefix(tag)) + || builtIn.contains(topLevelPrefix(tag)); +} + +QColor TagColors::colourFor(const QString &tag) const +{ + // An account's colour lives in its own stanza, not in [tagcolors]. + if (isAccountTag(tag)) { + const QColor colour = m_accountColours.value(accountKeyForTag(tag)); + if (colour.isValid()) + return colour; + } + + const QHash<QString, QColor> builtIn = builtInColours(); + + // Most specific first: an exact entry must beat the prefix it falls under, + // or a single child tag could never be singled out. + if (m_colours.contains(tag)) + return m_colours.value(tag); + if (builtIn.contains(tag)) + return builtIn.value(tag); + + const QString prefix = topLevelPrefix(tag); + if (m_colours.contains(prefix)) + return m_colours.value(prefix); + if (builtIn.contains(prefix)) + return builtIn.value(prefix); + + // Nothing configured: derive a colour from the name so the chip is still + // readable and distinguishable. Hashing keeps it stable across calls, and + // the fixed saturation and lightness keep it in the same family as the + // built-ins rather than producing neon. + const QByteArray digest = + QCryptographicHash::hash(tag.toUtf8(), QCryptographicHash::Md5); + const int hue = static_cast<quint8>(digest.at(0)) * 360 / 256; + return QColor::fromHsl(hue, 90, 80); +} diff --git a/src/tagcolors.h b/src/tagcolors.h new file mode 100644 index 0000000..f9e1a95 --- /dev/null +++ b/src/tagcolors.h @@ -0,0 +1,92 @@ +/* + * 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 <QColor> +#include <QHash> +#include <QString> +#include <QStringList> + +class QSettings; + +/// Colours for tag chips. +/// +/// Tags fall into two taxonomies. A functional tag says what state a thread is +/// in (flagged, replied, shopping/amazon) and is coloured from built-in +/// defaults or the [tagcolors] config group. An account tag says which mailbox +/// it arrived in, is named account-<key> after the [account.<key>] stanza, and +/// takes its colour from that stanza instead. +/// +/// Lookup is by exact tag first, then by top-level prefix, so one entry can +/// colour a whole hierarchy: "shopping" covers shopping/amazon and +/// shopping/nike, while "shopping/amazon" still overrides its own. +class TagColors +{ +public: + /// The prefix marking a tag as naming an account rather than a state. + static QString accountTagPrefix() { return QStringLiteral("account-"); } + + static bool isAccountTag(const QString &tag); + + /// The [account.<key>] suffix behind an account tag, empty if not one. + static QString accountKeyForTag(const QString &tag); + + /// The tag notmuch carries for an account key. The mapping is derived, + /// never configured, so the two cannot drift. + static QString tagForAccountKey(const QString &key); + + /// Black or white, whichever stays legible on the given fill. + static QColor textColourOn(const QColor &background); + + /// Reads the [tagcolors] group. An unparseable colour is collected into + /// warnings() and the previous value kept, so one typo cannot leave a tag + /// unstyled. + void load(QSettings &settings); + + /// Registers an account's colour, taken from its own stanza. + void setAccountColour(const QString &accountKey, const QColor &colour); + + /// Registers the text shown on an account's chip. Empty is ignored: a + /// blank label would render an unreadable chip. The notmuch tag itself is + /// never renamed, only what the chip displays. + void setAccountLabel(const QString &accountKey, const QString &label); + + /// Chip text for an account tag, falling back to the account key. Empty + /// when the tag does not name an account. + QString labelForAccountTag(const QString &tag) const; + + /// True when this tag resolves to a colour that was chosen for it, as + /// opposed to the fallback every unknown tag receives. + bool hasColour(const QString &tag) const; + + /// Always valid: an unconfigured tag falls back to a colour derived from + /// its name, stable across calls so a chip never changes as you scroll. + QColor colourFor(const QString &tag) const; + + QStringList warnings() const { return m_warnings; } + +private: + /// The part before the first '/', which is the whole tag when it has none. + static QString topLevelPrefix(const QString &tag); + + QHash<QString, QColor> m_colours; ///< Exact tags and prefixes. + QHash<QString, QColor> m_accountColours; ///< Keyed by account key. + QHash<QString, QString> m_accountLabels; ///< Keyed by account key. + QStringList m_warnings; +}; diff --git a/src/tagstrip.cpp b/src/tagstrip.cpp new file mode 100644 index 0000000..bad116a --- /dev/null +++ b/src/tagstrip.cpp @@ -0,0 +1,136 @@ +/* + * 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 "tagstrip.h" + +#include <QFontMetrics> +#include <QPainter> + +#include "tagchip.h" +#include "tagcolors.h" + +namespace { + +/// Text of the chip standing in for tags that did not fit. +QString overflowText(int count) +{ + return QStringLiteral("+%1").arg(count); +} + +} // namespace + +TagStrip::TagStrip(QWidget *parent) + : QWidget(parent) +{ + setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Fixed); +} + +void TagStrip::setTagColors(const TagColors *colours) +{ + m_tagColors = colours; + update(); +} + +void TagStrip::setTags(const QStringList &tags) +{ + m_tags.clear(); + for (const QString &tag : tags) { + // The account tag is shown as a chip in the thread list instead: it + // says which mailbox the thread came from, not what state it is in. + if (!TagColors::isAccountTag(tag)) + m_tags.append(tag); + } + m_tags.sort(); + + relayout(); + setVisible(!m_tags.isEmpty()); + update(); +} + +void TagStrip::relayout() +{ + m_visible.clear(); + m_hidden.clear(); + if (m_tags.isEmpty()) + return; + + const QFontMetrics metrics(font()); + // Reserve room for the overflow chip up front. Sizing it for the worst + // case avoids the loop having to back out a chip it already placed. + const int overflowWidth = + TagChip::sizeFor(metrics, overflowText(m_tags.size())).width() + + TagChip::kSpacing; + + int used = 0; + for (int i = 0; i < m_tags.size(); ++i) { + const int chipWidth = + TagChip::sizeFor(metrics, m_tags.at(i)).width() + TagChip::kSpacing; + const bool isLast = (i == m_tags.size() - 1); + // Every chip but the last must also leave room for the overflow chip, + // since anything after it will be hidden. + const int needed = used + chipWidth + (isLast ? 0 : overflowWidth); + if (needed > width() && !m_visible.isEmpty()) { + m_hidden = m_tags.mid(i); + break; + } + m_visible.append(m_tags.at(i)); + used += chipWidth; + } + + setToolTip(m_hidden.isEmpty() ? QString() + : m_hidden.join(QStringLiteral(", "))); +} + +QSize TagStrip::sizeHint() const +{ + const QFontMetrics metrics(font()); + return QSize(0, metrics.height() + TagChip::kPaddingY * 2 + + TagChip::kSpacing * 2); +} + +void TagStrip::resizeEvent(QResizeEvent *event) +{ + QWidget::resizeEvent(event); + relayout(); +} + +void TagStrip::paintEvent(QPaintEvent *) +{ + if (m_visible.isEmpty()) + return; + + QPainter painter(this); + const QFontMetrics metrics(font()); + const int top = (height() - (metrics.height() + TagChip::kPaddingY * 2)) / 2; + + int x = 0; + for (const QString &tag : m_visible) { + const QSize size = TagChip::sizeFor(metrics, tag); + const QColor colour = m_tagColors ? m_tagColors->colourFor(tag) + : TagColors().colourFor(tag); + TagChip::paint(&painter, QRect(QPoint(x, top), size), tag, colour); + x += size.width() + TagChip::kSpacing; + } + + if (!m_hidden.isEmpty()) { + const QString text = overflowText(m_hidden.size()); + const QSize size = TagChip::sizeFor(metrics, text); + TagChip::paint(&painter, QRect(QPoint(x, top), size), text, + QColor(0x44, 0x44, 0x4c)); + } +} diff --git a/src/tagstrip.h b/src/tagstrip.h new file mode 100644 index 0000000..4102bed --- /dev/null +++ b/src/tagstrip.h @@ -0,0 +1,63 @@ +/* + * 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 <QStringList> +#include <QWidget> + +class TagColors; + +/// One row of tag chips under the message pane. +/// +/// A single row by design: the message area must not shift as you move between +/// threads with different numbers of tags. Whatever does not fit collapses +/// into a trailing "+N" chip whose tooltip names the hidden tags. +class TagStrip : public QWidget +{ + Q_OBJECT +public: + explicit TagStrip(QWidget *parent = nullptr); + + /// Not owned; must outlive the strip. + void setTagColors(const TagColors *colours); + + /// Account tags are filtered out: they belong to the thread list chip, + /// being a different taxonomy from the functional tags shown here. + void setTags(const QStringList &tags); + + QSize sizeHint() const override; + + /// The tags actually drawn, in order. Exposed for testing the overflow + /// split without rendering. + QStringList visibleTags() const { return m_visible; } + QStringList hiddenTags() const { return m_hidden; } + +protected: + void paintEvent(QPaintEvent *event) override; + void resizeEvent(QResizeEvent *event) override; + +private: + /// Recomputes the visible/hidden split for the current width. + void relayout(); + + QStringList m_tags; ///< Functional tags only, account ones removed. + QStringList m_visible; + QStringList m_hidden; + const TagColors *m_tagColors = nullptr; +}; diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp index c129be1..2f2882e 100644 --- a/src/threadlistmodel.cpp +++ b/src/threadlistmodel.cpp @@ -18,8 +18,24 @@ #include "threadlistmodel.h" +#include <QBrush> #include <QFont> +QColor ThreadListModel::deletedColour() +{ + // Desaturated crimson: legible under white text on a dark theme, and calm + // enough that deleting fifty threads does not repaint the list as a + // warning banner. + return QColor(0x8b, 0x2c, 0x2c); +} + +QColor ThreadListModel::spamColour() +{ + // Distinct hue rather than a lighter red, so spam and deleted are told + // apart by colour and not by shade. + return QColor(0xa8, 0x5c, 0x18); +} + ThreadListModel::ThreadListModel(QObject *parent) : QAbstractTableModel(parent) { @@ -49,6 +65,26 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const if (role == ThreadIdRole) return thread.threadId; + if (role == TagsRole) + return thread.tags; + + if (role == AccountLabelRole || role == AccountColourRole) { + // At most one account tag per thread in practice, but a thread whose + // messages landed in two mailboxes carries both; the first is shown. + for (const QString &tag : thread.tags) { + if (!TagColors::isAccountTag(tag)) + continue; + if (role == AccountLabelRole) { + // The configured label when there is one, otherwise the key. + return m_tagColors ? m_tagColors->labelForAccountTag(tag) + : TagColors::accountKeyForTag(tag); + } + return m_tagColors ? m_tagColors->colourFor(tag) + : TagColors().colourFor(tag); + } + return {}; + } + if (role == Qt::DisplayRole) { switch (index.column()) { case DateColumn: @@ -60,17 +96,39 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const ? QStringLiteral("%1 (%2)").arg(thread.subject) .arg(thread.totalCount) : thread.subject; - case TagsColumn: - return thread.tags.join(QLatin1Char(' ')); default: return {}; } } - if (role == Qt::FontRole && thread.isUnread()) { + // A thread tagged deleted or spam is on its way out, and the user needs to + // see that the moment they act. Every one of these roles applies to the + // whole row: a cue on a single column disappears as soon as that column + // scrolls out of view, which is exactly how the tag change used to go + // unnoticed. + if (thread.isDoomed()) { + if (role == Qt::BackgroundRole) + return QBrush(thread.isDeleted() ? deletedColour() : spamColour()); + if (role == Qt::ForegroundRole) + return QBrush(QColor(Qt::white)); + } + + if (role == Qt::FontRole) { QFont font; - font.setBold(true); - return font; + bool styled = false; + if (thread.isUnread()) { + font.setBold(true); + styled = true; + } + // Struck through as well as filled, so the state survives a + // screenshot, a colourblind reader, and a theme that overrides the + // background. + if (thread.isDoomed()) { + font.setStrikeOut(true); + styled = true; + } + if (styled) + return font; } return {}; @@ -86,7 +144,6 @@ QVariant ThreadListModel::headerData(int section, Qt::Orientation orientation, case DateColumn: return QStringLiteral("Date"); case AuthorsColumn: return QStringLiteral("From"); case SubjectColumn: return QStringLiteral("Subject"); - case TagsColumn: return QStringLiteral("Tags"); default: return {}; } } diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h index eb1a7ff..7ed8fef 100644 --- a/src/threadlistmodel.h +++ b/src/threadlistmodel.h @@ -19,8 +19,10 @@ #pragma once #include <QAbstractTableModel> +#include <QColor> #include <QVector> +#include "tagcolors.h" #include "types.h" /// Table model over query results, filled in batches so a large query paints @@ -29,11 +31,14 @@ class ThreadListModel : public QAbstractTableModel { Q_OBJECT public: + /// No tags column: spelling out a dozen tags per row cost most of the + /// list's width and was unreadable. Functional tags moved to a chip strip + /// under the message pane, and the account tag renders as a chip in front + /// of the subject. enum Column { DateColumn = 0, AuthorsColumn, SubjectColumn, - TagsColumn, ColumnCount, }; @@ -42,10 +47,32 @@ public: /// worker speaks thread ids, so the mapping belongs on the model /// rather than in every caller. ThreadIdRole = Qt::UserRole + 1, + + /// The account tag on this thread without its "account-" prefix, for + /// the chip drawn in front of the subject. Empty when the thread + /// carries none. + AccountLabelRole, + + /// Fill colour for that chip. + AccountColourRole, + + /// Every tag on the thread, for the strip under the message pane. + TagsRole, }; + /// Row fill for a thread tagged `deleted`, and for one tagged `spam`. + /// Muted rather than saturated: a bulk delete paints every selected row, + /// and a wall of pure red is harder to read than the list it replaces. + /// Exposed so a test names the same colour the model uses. + static QColor deletedColour(); + static QColor spamColour(); + explicit ThreadListModel(QObject *parent = nullptr); + /// Supplies the account chip colours. Not owned; must outlive the model. + /// Without one, chips fall back to a colour generated from the tag name. + void setTagColors(const TagColors *colours) { m_tagColors = colours; } + int rowCount(const QModelIndex &parent = {}) const override; int columnCount(const QModelIndex &parent = {}) const override; QVariant data(const QModelIndex &index, int role) const override; @@ -65,4 +92,5 @@ public: private: QVector<ThreadSummary> m_threads; + const TagColors *m_tagColors = nullptr; }; diff --git a/src/types.h b/src/types.h index 2de6129..e25c3a9 100644 --- a/src/types.h +++ b/src/types.h @@ -35,6 +35,13 @@ struct ThreadSummary bool isUnread() const { return tags.contains(QStringLiteral("unread")); } bool isFlagged() const { return tags.contains(QStringLiteral("flagged")); } + bool isDeleted() const { return tags.contains(QStringLiteral("deleted")); } + bool isSpam() const { return tags.contains(QStringLiteral("spam")); } + + /// True while the thread is tagged for removal. notmuch deletes nothing + /// itself: the tag marks the thread for whatever the user's sync script + /// does next, so the row has to show it is on its way out. + bool isDoomed() const { return isDeleted() || isSpam(); } }; struct MessageRef diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1833f29..e761cb6 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -13,6 +13,7 @@ target_compile_definitions(test_mimeparser PRIVATE add_qtmaildir_test(interceptor) add_qtmaildir_test(htmlbuilder) add_qtmaildir_test(notmuchworker) +add_qtmaildir_test(tagcolors) add_qtmaildir_test(threadlistmodel) add_qtmaildir_test(mailsync) add_qtmaildir_test(threadcidmap) diff --git a/tests/test_keymap.cpp b/tests/test_keymap.cpp index 7fc31ef..c81eeb0 100644 --- a/tests/test_keymap.cpp +++ b/tests/test_keymap.cpp @@ -32,18 +32,143 @@ private slots: void unknownActionIsReported(); void invalidSequenceIsReported(); void collidingOverridesAreReported(); + void bareCapitalMatchesShiftedPress(); + void userBindingWinsOverDefaultInMenus(); + void defaultsDoNotCollide(); + void everyDefaultIsAKnownAction(); }; void TestKeyMap::defaultsAreLoaded() { KeyMap map; map.loadDefaults(); - QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("j"))), + QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("Ctrl+J"))), QStringLiteral("next_thread")); - QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("a"))), + QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("Ctrl+E"))), QStringLiteral("archive")); } +void TestKeyMap::bareCapitalMatchesShiftedPress() +{ + // Typing a capital produces Shift+<key>, but QKeySequence::fromString() + // discards the case of a bare letter: "N" and "n" both parse to plain + // Key_N, which no keypress can ever produce. A user who writes "N = flag" + // would get a binding that silently never fires. Normalizing a bare + // capital to Shift+<key> is what they meant. + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("keys")); + s.setValue(QStringLiteral("N"), QStringLiteral("flag")); + s.endGroup(); + } + + KeyMap map; + map.loadDefaults(); + QSettings s(path, QSettings::IniFormat); + map.loadOverrides(s); + + // The sequence a real Shift+N keypress produces. + QKeyEvent press(QEvent::KeyPress, Qt::Key_N, Qt::ShiftModifier); + QCOMPARE(map.actionFor(QKeySequence(press.keyCombination())), + QStringLiteral("flag")); + + // A lowercase binding stays unshifted, so the two remain distinguishable. + QVERIFY(map.warnings().isEmpty()); + + // "y" and "Y" are two different keys, not a collision: the second would + // have silently displaced the first before normalization. + QTemporaryDir caseDir; + const QString casePath = caseDir.filePath(QStringLiteral("case.conf")); + { + QSettings s(casePath, QSettings::IniFormat); + s.beginGroup(QStringLiteral("keys")); + s.setValue(QStringLiteral("y"), QStringLiteral("archive")); + s.setValue(QStringLiteral("Y"), QStringLiteral("delete")); + s.endGroup(); + } + KeyMap caseMap; + QSettings caseSettings(casePath, QSettings::IniFormat); + caseMap.loadOverrides(caseSettings); + + QKeyEvent lower(QEvent::KeyPress, Qt::Key_Y, Qt::NoModifier); + QKeyEvent upper(QEvent::KeyPress, Qt::Key_Y, Qt::ShiftModifier); + QCOMPARE(caseMap.actionFor(QKeySequence(lower.keyCombination())), + QStringLiteral("archive")); + QCOMPARE(caseMap.actionFor(QKeySequence(upper.keyCombination())), + QStringLiteral("delete")); + QVERIFY(caseMap.warnings().isEmpty()); +} + +void TestKeyMap::userBindingWinsOverDefaultInMenus() +{ + // loadOverrides() adds a binding without removing the default, so two + // sequences reach 'archive'. sequenceFor() is what the menus and the + // shortcut reference display: it must show the user's, not the built-in + // one they were trying to replace. + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("keys")); + s.setValue(QStringLiteral("Ctrl+Alt+A"), QStringLiteral("archive")); + s.endGroup(); + } + + KeyMap map; + map.loadDefaults(); + QSettings s(path, QSettings::IniFormat); + map.loadOverrides(s); + + QCOMPARE(map.sequenceFor(QStringLiteral("archive")), + QKeySequence(QStringLiteral("Ctrl+Alt+A"))); + + // The default still fires; it is only no longer the advertised one. + QCOMPARE(map.actionFor(KeyMap::defaultSequenceFor(QStringLiteral("archive"))), + QStringLiteral("archive")); + + // An action the user left alone still shows its default. + QCOMPARE(map.sequenceFor(QStringLiteral("delete")), + KeyMap::defaultSequenceFor(QStringLiteral("delete"))); +} + +void TestKeyMap::defaultsDoNotCollide() +{ + // Two defaults on one sequence means one of them is unreachable, and the + // QHash would silently keep whichever was inserted last. + KeyMap map; + map.loadDefaults(); + + QSet<QString> actions; + for (const QString &action : KeyMap::knownActions()) { + const QKeySequence seq = map.defaultSequenceFor(action); + if (seq.isEmpty()) + continue; // Not every action carries a default. + QVERIFY2(map.actionFor(seq) == action, + qPrintable(QStringLiteral("default '%1' for '%2' resolves to '%3'") + .arg(seq.toString(), action, map.actionFor(seq)))); + actions.insert(action); + } + QVERIFY(!actions.isEmpty()); +} + +void TestKeyMap::everyDefaultIsAKnownAction() +{ + // A default bound to a name loadOverrides() would reject as unknown. + KeyMap map; + map.loadDefaults(); + const QStringList known = KeyMap::knownActions(); + for (const QString &action : known) + QVERIFY(!action.isEmpty()); + + for (const QString &action : map.defaultActions()) { + QVERIFY2(known.contains(action), + qPrintable(QStringLiteral("default binds unknown action '%1'") + .arg(action))); + } +} + void TestKeyMap::iniOverridesDefault() { QTemporaryDir dir; @@ -51,7 +176,7 @@ void TestKeyMap::iniOverridesDefault() { QSettings s(path, QSettings::IniFormat); s.beginGroup(QStringLiteral("keys")); - s.setValue(QStringLiteral("j"), QStringLiteral("archive")); + s.setValue(QStringLiteral("Ctrl+J"), QStringLiteral("archive")); s.endGroup(); } @@ -60,10 +185,10 @@ void TestKeyMap::iniOverridesDefault() QSettings s(path, QSettings::IniFormat); map.loadOverrides(s); - QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("j"))), + QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("Ctrl+J"))), QStringLiteral("archive")); // An untouched default survives. - QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("k"))), + QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("Ctrl+K"))), QStringLiteral("prev_thread")); } @@ -148,16 +273,22 @@ void TestKeyMap::invalidSequenceIsReported() void TestKeyMap::collidingOverridesAreReported() { - // "y" and "Y" both normalize to the same QKeySequence ("Y"), so binding - // both in [keys] is a genuine collision that must not silently drop one. + // Two spellings of one sequence. "Ctrl+Y" and "ctrl+y" parse identically, + // so binding both in [keys] is a genuine collision that must not silently + // drop one. + // + // Note "y" and "Y" are NOT a collision any more: normalizeSequence() + // rewrites a bare capital to Shift+Y, which is the key a user actually + // presses, leaving the two distinct. Before that they both folded to + // plain Key_Y and one was lost. { QTemporaryDir dir; const QString path = dir.filePath(QStringLiteral("t.conf")); { QSettings s(path, QSettings::IniFormat); s.beginGroup(QStringLiteral("keys")); - s.setValue(QStringLiteral("y"), QStringLiteral("archive")); - s.setValue(QStringLiteral("Y"), QStringLiteral("delete")); + s.setValue(QStringLiteral("Ctrl+Y"), QStringLiteral("archive")); + s.setValue(QStringLiteral("ctrl+y"), QStringLiteral("delete")); s.endGroup(); } @@ -179,7 +310,7 @@ void TestKeyMap::collidingOverridesAreReported() { QSettings s(path, QSettings::IniFormat); s.beginGroup(QStringLiteral("keys")); - s.setValue(QStringLiteral("j"), QStringLiteral("archive")); + s.setValue(QStringLiteral("Ctrl+J"), QStringLiteral("archive")); s.endGroup(); } diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 57eb763..6bfa925 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -18,18 +18,27 @@ #include <QtTest> +#include <QAction> +#include <QDir> +#include <QSettings> +#include <QTemporaryDir> + +#include "config.h" #include "keymap.h" #include "mainwindow.h" -/// MainWindow is mostly wiring and needs a live QApplication plus a real -/// database, so it is verified manually in Task 13. Two things do not need -/// either, and both are the kind of drift a comment alone does not prevent. +/// MainWindow is mostly wiring, and the parts that need a real database are +/// still verified manually. What is checked here is the action registry: the +/// bindings a user configures reach the QActions the menus and the keyboard +/// both read from, and no action is left unreachable. class TestMainWindow : public QObject { Q_OBJECT private slots: void everyKnownActionIsRegistered(); void everyRegisteredActionIsKnown(); + void everyActionHasAShortcut(); + void configuredBindingReachesTheAction(); void cidPrefixesAreBangFree(); void cidPrefixesAreDistinctPerMessage(); }; @@ -39,8 +48,14 @@ void TestMainWindow::everyKnownActionIsRegistered() // KeyMap::knownActions() is what loadOverrides() validates config bindings // against. An action listed there but never registered means a user can // bind a key in qtmaildir.conf, get no warning, and have it do nothing. + // + // registeredActionNames() is now derived from the QActions themselves, so + // this compares against what the window really installed. + const Config config; + MainWindow window(config); + const QStringList known = KeyMap::knownActions(); - const QStringList registered = MainWindow::registeredActionNames(); + const QStringList registered = window.registeredActionNames(); for (const QString &action : known) { QVERIFY2(registered.contains(action), @@ -53,8 +68,11 @@ void TestMainWindow::everyRegisteredActionIsKnown() { // The reverse drift: an action MainWindow implements but KeyMap rejects. // The user would get "unknown action" for a binding that is really there. + const Config config; + MainWindow window(config); + const QStringList known = KeyMap::knownActions(); - const QStringList registered = MainWindow::registeredActionNames(); + const QStringList registered = window.registeredActionNames(); for (const QString &action : registered) { QVERIFY2(known.contains(action), @@ -63,6 +81,57 @@ void TestMainWindow::everyRegisteredActionIsKnown() } } +void TestMainWindow::everyActionHasAShortcut() +{ + // An action with no binding is unreachable from the keyboard. Every one + // of them carries a default, so an empty shortcut means the default table + // and the action list have drifted apart. + const Config config; + MainWindow window(config); + + for (const QString &name : window.registeredActionNames()) { + const QAction *action = window.findChild<QAction *>(name); + QVERIFY2(action, qPrintable(QStringLiteral("no QAction named '%1'").arg(name))); + QVERIFY2(!action->shortcut().isEmpty(), + qPrintable(QStringLiteral("action '%1' has no shortcut").arg(name))); + } +} + +void TestMainWindow::configuredBindingReachesTheAction() +{ + // The whole point of [keys]: a user's override must end up on the QAction, + // which is what both the keyboard and the menus read. + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("keys")); + s.setValue(QStringLiteral("Ctrl+Alt+A"), QStringLiteral("archive")); + s.endGroup(); + } + + // MainWindow reads its keymap from Config::defaultPath(), so point that + // at the temporary file for this test. + const QString previous = qEnvironmentVariable("XDG_CONFIG_HOME"); + QVERIFY(QDir().mkpath(dir.filePath(QStringLiteral("qtmaildir")))); + QVERIFY(QFile::copy(path, dir.filePath(QStringLiteral("qtmaildir/qtmaildir.conf")))); + qputenv("XDG_CONFIG_HOME", dir.path().toUtf8()); + + { + const Config config; + MainWindow window(config); + const QAction *archive = + window.findChild<QAction *>(QStringLiteral("archive")); + QVERIFY(archive); + QCOMPARE(archive->shortcut(), QKeySequence(QStringLiteral("Ctrl+Alt+A"))); + } + + if (previous.isEmpty()) + qunsetenv("XDG_CONFIG_HOME"); + else + qputenv("XDG_CONFIG_HOME", previous.toUtf8()); +} + void TestMainWindow::cidPrefixesAreBangFree() { // MainWindow is the only producer of cidPrefix in the application. The @@ -89,5 +158,17 @@ void TestMainWindow::cidPrefixesAreDistinctPerMessage() } } -QTEST_MAIN(TestMainWindow) +// Constructing a MainWindow needs a QApplication and a platform plugin. The +// test has no display under ctest, so it runs offscreen unless the caller +// asked for something else. +int main(int argc, char *argv[]) +{ + qputenv("QT_QPA_PLATFORM", qgetenv("QT_QPA_PLATFORM").isEmpty() + ? QByteArray("offscreen") + : qgetenv("QT_QPA_PLATFORM")); + QApplication app(argc, argv); + TestMainWindow test; + return QTest::qExec(&test, argc, argv); +} + #include "test_mainwindow.moc" diff --git a/tests/test_tagcolors.cpp b/tests/test_tagcolors.cpp new file mode 100644 index 0000000..53c0210 --- /dev/null +++ b/tests/test_tagcolors.cpp @@ -0,0 +1,251 @@ +/* + * 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 <QSettings> +#include <QTemporaryDir> +#include <QtTest> + +#include "tagcolors.h" + +class TestTagColors : public QObject +{ + Q_OBJECT +private slots: + void builtInDefaultsExist(); + void prefixColoursWholeHierarchy(); + void exactTagBeatsItsPrefix(); + void configOverridesABuiltIn(); + void unknownTagStillGetsAColour(); + void accountTagsAreRecognised(); + void accountColourComesFromTheAccount(); + void accountLabelDefaultsToTheKey(); + void accountLabelCanBeOverridden(); + void malformedColourIsReported(); + void textContrastsWithItsBackground(); +}; + +void TestTagColors::builtInDefaultsExist() +{ + // The common state tags must be styled out of the box: a user who never + // writes a [tagcolors] section still needs flagged to stand out. + TagColors colours; + const QStringList expected = { QStringLiteral("flagged"), + QStringLiteral("unread"), + QStringLiteral("deleted"), + QStringLiteral("spam"), + QStringLiteral("attachment"), + QStringLiteral("replied") }; + for (const QString &tag : expected) { + QVERIFY2(colours.hasColour(tag), + qPrintable(QStringLiteral("no built-in colour for '%1'").arg(tag))); + } +} + +void TestTagColors::prefixColoursWholeHierarchy() +{ + // 96 tags, many of them shopping/foo and mailing-list/bar. Colouring by + // top-level prefix is what keeps the config from listing every one. + TagColors colours; + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("tagcolors")); + s.setValue(QStringLiteral("shopping"), QStringLiteral("#3366cc")); + s.endGroup(); + } + QSettings s(path, QSettings::IniFormat); + colours.load(s); + + QCOMPARE(colours.colourFor(QStringLiteral("shopping/amazon")), + QColor(QStringLiteral("#3366cc"))); + QCOMPARE(colours.colourFor(QStringLiteral("shopping/nike")), + QColor(QStringLiteral("#3366cc"))); + // The bare prefix itself is a tag too. + QCOMPARE(colours.colourFor(QStringLiteral("shopping")), + QColor(QStringLiteral("#3366cc"))); + // A different hierarchy is unaffected. + QVERIFY(colours.colourFor(QStringLiteral("mailing-list/SBo")) + != QColor(QStringLiteral("#3366cc"))); +} + +void TestTagColors::exactTagBeatsItsPrefix() +{ + // Specific beats general, or you could never single out one child tag. + TagColors colours; + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("tagcolors")); + s.setValue(QStringLiteral("shopping"), QStringLiteral("#3366cc")); + s.setValue(QStringLiteral("shopping/amazon"), QStringLiteral("#ff9900")); + s.endGroup(); + } + QSettings s(path, QSettings::IniFormat); + colours.load(s); + + QCOMPARE(colours.colourFor(QStringLiteral("shopping/amazon")), + QColor(QStringLiteral("#ff9900"))); + QCOMPARE(colours.colourFor(QStringLiteral("shopping/nike")), + QColor(QStringLiteral("#3366cc"))); + + // Regression: QSettings treats '/' as a group separator, so a + // hierarchical tag is a nested key that childKeys() never returns. Reading + // the group with childKeys() silently dropped every tag with a '/' in it, + // which is most of this user's, and they all fell through to their prefix. + QVERIFY(colours.hasColour(QStringLiteral("shopping/amazon"))); +} + +void TestTagColors::configOverridesABuiltIn() +{ + TagColors colours; + const QColor original = colours.colourFor(QStringLiteral("flagged")); + + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("tagcolors")); + s.setValue(QStringLiteral("flagged"), QStringLiteral("#00ff00")); + s.endGroup(); + } + QSettings s(path, QSettings::IniFormat); + colours.load(s); + + QCOMPARE(colours.colourFor(QStringLiteral("flagged")), + QColor(QStringLiteral("#00ff00"))); + QVERIFY(colours.colourFor(QStringLiteral("flagged")) != original); +} + +void TestTagColors::unknownTagStillGetsAColour() +{ + // A chip with no colour would render as an invisible blank, so every tag + // resolves to something even when nothing is configured for it. + TagColors colours; + const QColor colour = colours.colourFor(QStringLiteral("no-such-tag-anywhere")); + QVERIFY(colour.isValid()); + + // Stable across calls: a tag must not change colour as you scroll. + QCOMPARE(colours.colourFor(QStringLiteral("no-such-tag-anywhere")), colour); +} + +void TestTagColors::accountTagsAreRecognised() +{ + // Account tags are a different taxonomy from functional tags: which + // mailbox a thread came from, not what state it is in. They are shown + // separately, so they have to be identifiable. + QVERIFY(TagColors::isAccountTag(QStringLiteral("account-gmail-danixland"))); + QVERIFY(!TagColors::isAccountTag(QStringLiteral("flagged"))); + QVERIFY(!TagColors::isAccountTag(QStringLiteral("shopping/amazon"))); + + // The INI key for [account.gmail-danixland] is what follows "account-". + QCOMPARE(TagColors::accountKeyForTag(QStringLiteral("account-gmail-danixland")), + QStringLiteral("gmail-danixland")); + QVERIFY(TagColors::accountKeyForTag(QStringLiteral("flagged")).isEmpty()); + + // Round trip, since the mapping is derived rather than configured. + QCOMPARE(TagColors::tagForAccountKey(QStringLiteral("gmail-danixland")), + QStringLiteral("account-gmail-danixland")); +} + +void TestTagColors::accountColourComesFromTheAccount() +{ + // Per the account stanza, not [tagcolors]: the colour belongs to the + // account, and the tag name is derived from its key. + TagColors colours; + colours.setAccountColour(QStringLiteral("gmail-danixland"), + QColor(QStringLiteral("#cc0000"))); + + QCOMPARE(colours.colourFor(QStringLiteral("account-gmail-danixland")), + QColor(QStringLiteral("#cc0000"))); +} + +void TestTagColors::accountLabelDefaultsToTheKey() +{ + // Without a configured label the chip shows the account key, which is what + // it did before labels existed. + TagColors colours; + QCOMPARE(colours.labelForAccountTag(QStringLiteral("account-gmail-danixland")), + QStringLiteral("gmail-danixland")); + + // Not an account tag: nothing to label. + QVERIFY(colours.labelForAccountTag(QStringLiteral("flagged")).isEmpty()); +} + +void TestTagColors::accountLabelCanBeOverridden() +{ + // "account-privateemail-danilo.macri" is 33 characters of chip for what is + // really one bit of information, so the label is configurable. + TagColors colours; + colours.setAccountLabel(QStringLiteral("gmail-danixland"), + QStringLiteral("GM-danixland")); + colours.setAccountLabel(QStringLiteral("privateemail-danix"), + QStringLiteral("PE-danix")); + + QCOMPARE(colours.labelForAccountTag(QStringLiteral("account-gmail-danixland")), + QStringLiteral("GM-danixland")); + QCOMPARE(colours.labelForAccountTag(QStringLiteral("account-privateemail-danix")), + QStringLiteral("PE-danix")); + + // An account left unlabelled still falls back to its key. + QCOMPARE(colours.labelForAccountTag(QStringLiteral("account-work")), + QStringLiteral("work")); + + // An empty label is not an override: it would render a blank chip. + colours.setAccountLabel(QStringLiteral("gmail-danixland"), QString()); + QCOMPARE(colours.labelForAccountTag(QStringLiteral("account-gmail-danixland")), + QStringLiteral("GM-danixland")); +} + +void TestTagColors::malformedColourIsReported() +{ + // A typo must be visible rather than silently ignored, matching how the + // rest of the config reports its problems. + TagColors colours; + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("tagcolors")); + s.setValue(QStringLiteral("flagged"), QStringLiteral("not-a-colour")); + s.endGroup(); + } + QSettings s(path, QSettings::IniFormat); + colours.load(s); + + QCOMPARE(colours.warnings().size(), 1); + QVERIFY(colours.warnings().first().contains(QStringLiteral("flagged"))); + // The built-in survives, so one bad line does not leave the tag unstyled. + QVERIFY(colours.colourFor(QStringLiteral("flagged")).isValid()); +} + +void TestTagColors::textContrastsWithItsBackground() +{ + // A chip is coloured text on a coloured fill, so the pair has to stay + // legible whatever colour the user picks. + QCOMPARE(TagColors::textColourOn(QColor(Qt::black)), QColor(Qt::white)); + QCOMPARE(TagColors::textColourOn(QColor(Qt::white)), QColor(Qt::black)); + QCOMPARE(TagColors::textColourOn(QColor(QStringLiteral("#8b2c2c"))), + QColor(Qt::white)); + QCOMPARE(TagColors::textColourOn(QColor(QStringLiteral("#ffee88"))), + QColor(Qt::black)); +} + +QTEST_MAIN(TestTagColors) +#include "test_tagcolors.moc" diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp index 24ba9e2..e8a5fa8 100644 --- a/tests/test_threadlistmodel.cpp +++ b/tests/test_threadlistmodel.cpp @@ -33,6 +33,14 @@ private slots: void reportsSubjectAndAuthors(); void subjectShowsMessageCountOnlyForRealThreads(); void unreadThreadsRenderBold(); + void tagsAreTheFirstColumnAndSubjectTheLast(); + void accountTagBecomesAChipLabel(); + void unreadStylingSurvivesAnAccountChip(); + void accountChipUsesTheConfiguredColour(); + void deletedThreadsAreRedAndStruckThrough(); + void spamThreadsAreOrangeAndStruckThrough(); + void doomedStylingCoversEveryColumn(); + void ordinaryThreadsCarryNoRowColour(); void threadIdIsReachableFromAnIndex(); void invalidIndexesReturnNothing(); void threadAtOutOfRangeIsSafe(); @@ -113,9 +121,11 @@ void TestThreadListModel::reportsSubjectAndAuthors() const QModelIndex date = model.index(0, ThreadListModel::DateColumn); QVERIFY(!model.data(date, Qt::DisplayRole).toString().isEmpty()); - const QModelIndex tags = model.index(0, ThreadListModel::TagsColumn); - QCOMPARE(model.data(tags, Qt::DisplayRole).toString(), - QStringLiteral("inbox unread")); + // Tags are no longer a column; they reach the strip under the message + // pane through a role instead. + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QCOMPARE(model.data(subject, ThreadListModel::TagsRole).toStringList(), + QStringList({ QStringLiteral("inbox"), QStringLiteral("unread") })); } void TestThreadListModel::subjectShowsMessageCountOnlyForRealThreads() @@ -153,6 +163,172 @@ void TestThreadListModel::unreadThreadsRenderBold() QVERIFY(unreadFont.value<QFont>().bold()); } +void TestThreadListModel::tagsAreTheFirstColumnAndSubjectTheLast() +{ + // Subject stretches to fill the view, so whatever sits after it is pushed + // off-screen. Tags used to be there, which is why acting on a thread + // looked like it did nothing: the only column that changed was invisible. + QCOMPARE(ThreadListModel::SubjectColumn, ThreadListModel::ColumnCount - 1); + + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("hello")) }); + QCOMPARE(model.headerData(ThreadListModel::SubjectColumn, Qt::Horizontal, + Qt::DisplayRole).toString(), + QStringLiteral("Subject")); + + // No tags column at all: spelling out a dozen tags per row consumed most + // of the list's width and was unreadable. + for (int column = 0; column < ThreadListModel::ColumnCount; ++column) { + QVERIFY(model.headerData(column, Qt::Horizontal, Qt::DisplayRole) + .toString() != QStringLiteral("Tags")); + } +} + +void TestThreadListModel::accountTagBecomesAChipLabel() +{ + // The account tag is a different taxonomy from a functional one: which + // mailbox the thread arrived in. It renders as a chip in front of the + // subject, so the model exposes its label and colour separately. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("hello")); + thread.tags = QStringList{ QStringLiteral("inbox"), + QStringLiteral("account-gmail-danixland") }; + model.appendBatch({ thread }); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QCOMPARE(model.data(subject, ThreadListModel::AccountLabelRole).toString(), + QStringLiteral("gmail-danixland")); + QVERIFY(model.data(subject, ThreadListModel::AccountColourRole) + .value<QColor>().isValid()); + + // A thread with no account tag gets no chip rather than an empty one. + ThreadListModel plain; + ThreadSummary untagged = makeThread(QStringLiteral("t2"), QStringLiteral("hi")); + untagged.tags = QStringList{ QStringLiteral("inbox") }; + plain.appendBatch({ untagged }); + QVERIFY(plain.data(plain.index(0, ThreadListModel::SubjectColumn), + ThreadListModel::AccountLabelRole).toString().isEmpty()); +} + +void TestThreadListModel::unreadStylingSurvivesAnAccountChip() +{ + // The subject cell is drawn by a delegate when the thread has an account + // chip. The delegate paints the text itself, so it has to keep honouring + // the model's font: otherwise an unread thread stops rendering bold for + // exactly those threads that carry an account tag, which is all of them. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("hello")); + thread.tags = QStringList{ QStringLiteral("inbox"), QStringLiteral("unread"), + QStringLiteral("account-gmail-danixland") }; + model.appendBatch({ thread }); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QVERIFY(!model.data(subject, ThreadListModel::AccountLabelRole) + .toString().isEmpty()); + + const QVariant font = model.data(subject, Qt::FontRole); + QVERIFY2(font.isValid(), "unread thread with an account tag has no font"); + QVERIFY2(font.value<QFont>().bold(), "unread thread is not bold"); +} + +void TestThreadListModel::accountChipUsesTheConfiguredColour() +{ + // The colour comes from the account's own stanza, so a configured one must + // reach the chip rather than the generated fallback. + TagColors colours; + colours.setAccountColour(QStringLiteral("gmail-danixland"), + QColor(QStringLiteral("#cc0000"))); + + ThreadListModel model; + model.setTagColors(&colours); + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("hello")); + thread.tags = QStringList{ QStringLiteral("account-gmail-danixland") }; + model.appendBatch({ thread }); + + QCOMPARE(model.data(model.index(0, ThreadListModel::SubjectColumn), + ThreadListModel::AccountColourRole).value<QColor>(), + QColor(QStringLiteral("#cc0000"))); +} + +void TestThreadListModel::deletedThreadsAreRedAndStruckThrough() +{ + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("doomed")); + thread.tags = QStringList{ QStringLiteral("inbox") }; + model.appendBatch({ thread }); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QVERIFY(!model.data(subject, Qt::BackgroundRole).isValid()); + + model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); + + const QVariant background = model.data(subject, Qt::BackgroundRole); + QVERIFY(background.isValid()); + QCOMPARE(background.value<QBrush>().color(), ThreadListModel::deletedColour()); + + // White text on the fill, and struck through so the state reads even in a + // screenshot with the colours stripped. + QCOMPARE(model.data(subject, Qt::ForegroundRole).value<QBrush>().color(), + QColor(Qt::white)); + QVERIFY(model.data(subject, Qt::FontRole).value<QFont>().strikeOut()); +} + +void TestThreadListModel::spamThreadsAreOrangeAndStruckThrough() +{ + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("junk")); + thread.tags = QStringList{ QStringLiteral("inbox") }; + model.appendBatch({ thread }); + + model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("spam") }, {}); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QCOMPARE(model.data(subject, Qt::BackgroundRole).value<QBrush>().color(), + ThreadListModel::spamColour()); + QVERIFY(model.data(subject, Qt::FontRole).value<QFont>().strikeOut()); + + // Spam and deleted must be distinguishable, not two shades of one colour. + QVERIFY(ThreadListModel::spamColour() != ThreadListModel::deletedColour()); +} + +void TestThreadListModel::doomedStylingCoversEveryColumn() +{ + // A cue on one column would vanish the moment that column scrolled out of + // view, which is the bug this whole change exists to fix. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("doomed")); + thread.tags = QStringList{ QStringLiteral("inbox") }; + model.appendBatch({ thread }); + + model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); + + for (int column = 0; column < ThreadListModel::ColumnCount; ++column) { + const QModelIndex index = model.index(0, column); + QVERIFY2(model.data(index, Qt::BackgroundRole).isValid(), + qPrintable(QStringLiteral("column %1 has no background").arg(column))); + QVERIFY2(model.data(index, Qt::FontRole).value<QFont>().strikeOut(), + qPrintable(QStringLiteral("column %1 is not struck through").arg(column))); + } +} + +void TestThreadListModel::ordinaryThreadsCarryNoRowColour() +{ + // Undo has to restore the plain look, not merely drop the tag. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("normal")); + thread.tags = QStringList{ QStringLiteral("inbox") }; + model.appendBatch({ thread }); + + model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); + model.applyTagChange(QStringLiteral("t1"), {}, { QStringLiteral("deleted") }); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QVERIFY(!model.data(subject, Qt::BackgroundRole).isValid()); + QVERIFY(!model.data(subject, Qt::ForegroundRole).isValid()); + const QVariant font = model.data(subject, Qt::FontRole); + QVERIFY(!font.isValid() || !font.value<QFont>().strikeOut()); +} + void TestThreadListModel::threadIdIsReachableFromAnIndex() { // The view hands MainWindow a QModelIndex; the worker needs a thread id. |
