summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-04 11:50:11 +0200
committerDanilo M. <danix@danix.xyz>2026-08-04 12:54:56 +0200
commit260b64d1d21afc2b124b98d13cce157e5b9d2f2d (patch)
tree4145e0c21fe71e4e739be0275020f7e7d7e8fa77 /src
parent0a160651cfb9a0f580bcc446941058a339e55643 (diff)
downloadqtmaildir-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.cpp71
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);