aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-04 10:10:19 +0200
committerDanilo M. <danix@danix.xyz>2026-08-04 12:54:35 +0200
commitcf75b8ecf7d27fda217023507b8ed4df1f198613 (patch)
treee75025c35540d9422488fe51c3dcd2f53e9c8ff4
parent4df7edfc259fb3bdb3814ffe6549ecd3ee448578 (diff)
downloadqtmaildir-cf75b8ecf7d27fda217023507b8ed4df1f198613.tar.gz
qtmaildir-cf75b8ecf7d27fda217023507b8ed4df1f198613.zip
feat(completion): size the description column by need, not by half
The delegate split each row evenly between the value and its description. That reads as fair but spends the width on the wrong column: the values are short, the longest prefix being "attachment:", while the descriptions are ordinary prose. Measured against the built-in vocabulary, ten of fifteen descriptions elided at a 400px popup and the longest, "directory below the Maildir root", needed a ~650px popup to appear in full, all while the value's half of the row sat mostly empty. The descriptions are the whole reason the popup teaches the query language, so a half they cannot use is a half wasted. Measure the value and lend the description the remainder, capped at 65% of the row. Every built-in description now fits at ~500px instead of ~700px. The cap is what keeps a genuinely long value legible, and the value's own reservation is capped in turn so that "application/vnd.oasis.opendocument.text" elides itself rather than claiming the row and silencing the column that explains what it is. The test renders the real popup and reads the pixels back, since the delegate is private to the .cpp and legibility is a painting question. It asserts the painted width against the width the text needs unelided, not merely that something was drawn: a blank-or-not check passes against the very rule this replaces. 550px is chosen deliberately, being a width where the new rule paints the longest description in full and an even split cannot; against the old code it fails with "painted 261px of the 285px it needs". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
-rw-r--r--src/querycompleter.cpp33
-rw-r--r--tests/test_querycompleter.cpp95
2 files changed, 121 insertions, 7 deletions
diff --git a/src/querycompleter.cpp b/src/querycompleter.cpp
index 7e9dfd9..9848faf 100644
--- a/src/querycompleter.cpp
+++ b/src/querycompleter.cpp
@@ -88,10 +88,10 @@ int tokenEnd(const QString &text, int cursor)
/// Draws the description greyed and right-aligned beside the value.
///
-/// The two are drawn into disjoint halves of the row rather than simply
-/// painted on top of each other: a long value ("application/vnd.oasis..."
-/// exceeds the popup width on its own) would otherwise run underneath the
-/// description and render both unreadable.
+/// The two are drawn into disjoint spans of the row rather than simply painted
+/// on top of each other: a long value ("application/vnd.oasis..." exceeds the
+/// popup width on its own) would otherwise run underneath the description and
+/// render both unreadable. The split is not even, see paint().
class CompletionDelegate : public QStyledItemDelegate
{
public:
@@ -111,11 +111,30 @@ public:
const int gap = 12;
const int rightMargin = 6;
- // The description never takes more than its share, so a long value
- // keeps room to be legible and a long description gets elided too.
+ // Give the description what the value does not need, up to a ceiling.
+ //
+ // An even split reads as fair but spends width on the wrong column:
+ // the values here are short (the longest prefix is "attachment:") while
+ // the descriptions are ordinary prose, so at a 400px popup two thirds
+ // of them elided while the value half sat mostly empty. Measuring the
+ // value and lending the description the remainder clears every built-in
+ // description at ~500px instead of ~700px.
+ //
+ // The ceiling still matters: a long value ("application/vnd.oasis...")
+ // must keep enough room to stay legible rather than be squeezed to an
+ // ellipsis by a description that happens to be wordy.
const int available = option.rect.width() - gap - rightMargin;
+ const QString value = index.data(Qt::DisplayRole).toString();
+ const int valueWidth = metrics.horizontalAdvance(value);
+ const int ceiling = (available * 65) / 100;
+
+ // A value too long to fit would otherwise claim the whole row and
+ // leave no description at all. Cap what it can reserve, so an
+ // "application/vnd.oasis..." elides itself rather than silencing the
+ // column that explains what it is.
+ const int reservedForValue = qMin(valueWidth, available - ceiling);
int descriptionWidth = qMin(metrics.horizontalAdvance(description),
- available / 2);
+ qMin(available - reservedForValue, ceiling));
descriptionWidth = qMax(descriptionWidth, 0);
// Let the base class draw the selection background and the value, but
diff --git a/tests/test_querycompleter.cpp b/tests/test_querycompleter.cpp
index 09eedc3..6cf9651 100644
--- a/tests/test_querycompleter.cpp
+++ b/tests/test_querycompleter.cpp
@@ -20,7 +20,9 @@
#include <QTemporaryDir>
#include <QLineEdit>
+#include <QImage>
#include <QListView>
+#include <QPixmap>
#include "config.h"
#include "querycompleter.h"
@@ -81,6 +83,7 @@ private slots:
void returnRunsTheQueryOnceCompletionIsDone();
void returnRunsTheQueryAfterAMouseAccept();
void returnRunsTheQueryWhenThePopupMatchesNothing();
+ void theDescriptionSurvivesAModestPopupWidth();
};
// Copied from tests/test_config.cpp rather than shared, so the two test files
@@ -781,5 +784,97 @@ void TestQueryCompleter::returnRunsTheQueryWhenThePopupMatchesNothing()
QVERIFY(ran);
}
+void TestQueryCompleter::theDescriptionSurvivesAModestPopupWidth()
+{
+ // The delegate lends the description whatever the value does not need, up
+ // to 65% of the row. Under the previous even split the longest built-in
+ // description needed a ~650px popup to survive; it now needs ~500px, which
+ // is the difference between the column working at an ordinary window size
+ // and being decorative.
+ //
+ // 550px is chosen to sit inside that band: the current rule paints the text
+ // in full there, an even split cannot. A width outside the band would pass
+ // against both rules and prove nothing.
+ //
+ // The delegate is private to the .cpp, so this renders the real popup and
+ // reads the pixels back rather than reaching for the class: whether the text
+ // is legible on screen is a painting question, not an arithmetic one.
+ Config config;
+ QLineEdit edit;
+ edit.resize(550, edit.sizeHint().height());
+ edit.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&edit));
+ edit.setFocus();
+ QueryCompleter completer(&edit, config);
+
+ QTest::keyClicks(&edit, QStringLiteral("path"));
+ QListView *popup = findPopup();
+ QVERIFY(popup && popup->isVisible());
+ QVERIFY(QTest::qWaitForWindowExposed(popup));
+
+ // "path:" carries the longest built-in description, so it is the row that
+ // fails first if the column is starved.
+ const QModelIndex row = popup->model()->index(0, 0);
+ QVERIFY(row.isValid());
+ QCOMPARE(row.data(Qt::DisplayRole).toString(), QStringLiteral("path:"));
+
+ const QRect rect = popup->visualRect(row);
+ QVERIFY(rect.isValid());
+ QPixmap shot = popup->viewport()->grab(rect);
+ QVERIFY(!shot.isNull());
+ const QImage image = shot.toImage();
+
+ // Measure how much of the row carries ink, then compare that against the
+ // width the description needs when it is NOT elided.
+ //
+ // "Something was drawn" is too weak a check: the previous even-split rule
+ // also drew the description, just cut down to an ellipsis, so a blank-or-not
+ // test passes against the very code this replaces. What distinguishes the
+ // two is whether the full text fits, which is a width comparison.
+ const QRgb background = image.pixel(image.width() - 2, image.height() / 2);
+ int rightmostInk = -1;
+ int leftmostInkAfterValue = image.width();
+ for (int x = 0; x < image.width(); ++x) {
+ for (int y = 0; y < image.height(); ++y) {
+ if (image.pixel(x, y) != background) {
+ rightmostInk = qMax(rightmostInk, x);
+ break;
+ }
+ }
+ }
+ QVERIFY2(rightmostInk >= 0, "the row rendered entirely blank");
+
+ // The description is right-aligned, so the ink running to the right edge is
+ // the description itself. Walk left from there over the contiguous run to
+ // find how wide it was actually painted.
+ int x = rightmostInk;
+ int gapRun = 0;
+ while (x > 0 && gapRun < 8) {
+ bool column = false;
+ for (int y = 0; y < image.height(); ++y) {
+ if (image.pixel(x, y) != background) {
+ column = true;
+ break;
+ }
+ }
+ gapRun = column ? 0 : gapRun + 1;
+ if (column)
+ leftmostInkAfterValue = x;
+ --x;
+ }
+
+ const QFontMetrics metrics(popup->font());
+ const QString description = popup->model()->index(0, 1).data().toString();
+ QCOMPARE(description, QStringLiteral("directory below the Maildir root"));
+ const int painted = rightmostInk - leftmostInkAfterValue;
+ const int needed = metrics.horizontalAdvance(description);
+
+ // Allow a little slack for antialiasing at the glyph edges.
+ QVERIFY2(painted >= needed - 4,
+ qPrintable(QStringLiteral("description elided: painted %1px of the "
+ "%2px it needs")
+ .arg(painted).arg(needed)));
+}
+
QTEST_MAIN(TestQueryCompleter)
#include "test_querycompleter.moc"