diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-04 11:50:11 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-04 12:54:56 +0200 |
| commit | 260b64d1d21afc2b124b98d13cce157e5b9d2f2d (patch) | |
| tree | 4145e0c21fe71e4e739be0275020f7e7d7e8fa77 /src | |
| parent | 0a160651cfb9a0f580bcc446941058a339e55643 (diff) | |
| download | qtmaildir-260b64d1d21afc2b124b98d13cce157e5b9d2f2d.tar.gz qtmaildir-260b64d1d21afc2b124b98d13cce157e5b9d2f2d.zip | |
fix(tags): complete every tag in the list, not only the first
Reported by the user: in the Edit tags fields, the first tag suggested
existing tags and the second did not. Typing a comma, a space and a
letter offered nothing.
QLineEdit::setCompleter hands completion to the line edit, which
overwrites the completer's prefix with the widget's ENTIRE text on every
keystroke. These fields hold a comma-separated list, so once one reads
"unread, fl" that whole string is matched against the tag names, nothing
matches, and completion silently stops after the first tag. Confirmed
with a probe: the prefix really is "unread, fl" and the completion count
really is zero.
Attach with setWidget instead, which keeps the popup anchored without
ceding control of the prefix, and drive it from the token under the
cursor on every edit. Setting the prefix from a textEdited handler while
leaving setCompleter in place does NOT work, which was the first attempt:
the line edit sets it again afterwards.
Accepting a candidate needed the same treatment, and is the other half of
the fix. QCompleter's own insertion replaces the whole field, so taking
"flagged" from the popup would have discarded every tag already typed.
replaceCurrentToken() overwrites only the token under the cursor and
keeps the separator's spacing, so the result is "unread, flagged" rather
than "unread,flagged".
This is the same defect QueryCompleter hit in c98b179. Having now cost
two debugging rounds, it is written into CLAUDE.md as a Qt trap rather
than a property of either class, together with the reason a test using
setText() passes against it: setText does not drive a completer at all,
so the keys have to be typed.
Both new tests were confirmed to fail against setCompleter before the fix
was kept.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'src')
| -rw-r--r-- | src/tagdialog.cpp | 71 |
1 files changed, 70 insertions, 1 deletions
diff --git a/src/tagdialog.cpp b/src/tagdialog.cpp index 94d2e05..75e6b03 100644 --- a/src/tagdialog.cpp +++ b/src/tagdialog.cpp @@ -18,6 +18,7 @@ #include "tagdialog.h" +#include <QAbstractItemView> #include <QCompleter> #include <QCoreApplication> #include <QDialogButtonBox> @@ -97,6 +98,42 @@ QStringList splitTags(const QString &text) return tags; } +/// The tag the cursor sits in, which is what completion should match against. +/// +/// The field holds a comma-separated list, so the last comma before the cursor +/// bounds the token. Leading space is dropped so "unread, fl" completes on "fl" +/// rather than on " fl", which would match nothing. +QString currentToken(const QLineEdit *edit) +{ + const QString text = edit->text(); + const int cursor = qBound(0, edit->cursorPosition(), int(text.size())); + + const int start = text.lastIndexOf(QLatin1Char(','), qMax(0, cursor - 1)) + 1; + return text.mid(start, cursor - start).trimmed(); +} + +/// Overwrites the token under the cursor with `value`, leaving the rest of the +/// list alone, and puts the caret after what was inserted. +void replaceCurrentToken(QLineEdit *edit, const QString &value) +{ + const QString text = edit->text(); + const int cursor = qBound(0, edit->cursorPosition(), int(text.size())); + + const int start = text.lastIndexOf(QLatin1Char(','), qMax(0, cursor - 1)) + 1; + + // Keep the separator's spacing as the user typed it: replacing from `start` + // would eat the space after the comma and give "unread,flagged". + int tokenStart = start; + while (tokenStart < cursor && text.at(tokenStart).isSpace()) + ++tokenStart; + + QString updated = text; + updated.replace(tokenStart, cursor - tokenStart, value); + + edit->setText(updated); + edit->setCursorPosition(tokenStart + value.size()); +} + } // namespace TagDialog::TagDialog(const QStringList &knownTags, @@ -129,7 +166,39 @@ TagDialog::TagDialog(const QStringList &knownTags, // Hierarchies are the reason this matters: typing "amazon" should find // "shopping/amazon". completer->setFilterMode(Qt::MatchContains); - edit->setCompleter(completer); + + // setWidget, NOT QLineEdit::setCompleter. These fields hold a + // comma-separated LIST, and setCompleter makes the line edit drive + // completion, overwriting the prefix with the widget's ENTIRE text on + // every keystroke. Once the field reads "unread, fl" that whole string + // is matched against the tag names, nothing matches, and completion + // silently stops working after the first tag. Setting the prefix from a + // textEdited handler does not help: the line edit sets it again + // afterwards. + // + // setWidget keeps the popup anchored without ceding control of the + // prefix, which then becomes ours to drive per token. Exactly the fix + // QueryCompleter needed in 01ba356; the trap belongs to + // QLineEdit::setCompleter, not to either class. + completer->setWidget(edit); + + connect(edit, &QLineEdit::textEdited, this, [edit, completer]() { + const QString token = currentToken(edit); + completer->setCompletionPrefix(token); + if (token.isEmpty() || completer->completionCount() == 0) { + completer->popup()->hide(); + return; + } + completer->complete(); + }); + + // Accepting a candidate has to replace the token under the cursor + // rather than the whole field, or taking "flagged" would discard every + // tag already typed. + connect(completer, QOverload<const QString &>::of(&QCompleter::activated), + this, [edit](const QString &value) { + replaceCurrentToken(edit, value); + }); } form->addRow(tr("Add:"), m_addEdit); |
