From ca89402d7e244e2616fdf13eb35c083afeb21826 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 6 Aug 2026 20:06:45 +0200 Subject: fix(ui): one Sync control, and a query bar that looks finished Sync had two controls that behaved differently. The QPushButton beside the query bar cleared the log, opened the pane and disabled itself; the QAction behind the toolbar, menu and shortcut called start() and did nothing else, discarding its return value so a rejected start was silent. Worse, item 29's "disable Sync during a background sync" set the button only, so the toolbar entry stayed clickable through a cron sync and could only produce the script's EX_TEMPFAIL skip. startSync() is now the single handler behind every route in, and the enabled state lives on the QAction, which reaches the toolbar, the menu and the shortcut at once. It also reports when no sync command is configured rather than doing nothing. The QPushButton is gone. It read as a Search button given it sat beside a text field, which is the user's own observation and the reason the toolbar one survives instead. Its unavailable-command tooltip moved to the action, since that is the only thing that says why the control is dead. Removing it left the query field running flush to the window edge, so the saved-query buttons move from their own row onto the query row. The bar is now framed by the account dropdown on the left and the saved queries on the right, the empty row is gone, and the thread list gains the space. The field also gains setClearButtonEnabled, which is Qt's own themed clear icon rather than a hand-rolled button. A "Search" button was considered and rejected: Return already runs the query. No overflow handling for [queries], which is unbounded. Three entries fit; item 23 already specifies buttons-plus-menu and is where that belongs. CLAUDE.md's architecture diagram named four widget classes that have never existed, QueryBar, SavedQueryBar, HeaderWidget and AttachmentBar. The query row and the message header are built inline. Corrected, and the components that do exist but were missing from it added. Tests: the new action test was verified red first and load-bearing by mutation. The old button test is deleted rather than repointed, being an exact duplicate of it, and the unobservable-lock test now drives the action. The clear button and the row layout were confirmed by hand; no test clicks the icon, which is a mouse path. Backlog: 45 done and reclassified as a defect rather than a cosmetic redundancy, 47 added for the bar. --- src/mainwindow.cpp | 85 +++++++++++++++++++++++++++++++++++------------------- src/mainwindow.h | 5 +++- 2 files changed, 60 insertions(+), 30 deletions(-) (limited to 'src') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 75549d3..a2174a2 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -401,6 +401,10 @@ void MainWindow::buildUi() m_queryEdit = new QLineEdit(central); m_queryEdit->setPlaceholderText(tr("notmuch query, e.g. tag:inbox")); + // Qt draws the clear button inside the field and shows it only when there + // is text, themed by the desktop. A hand-rolled button beside the bar would + // read as "Search" and duplicate Return, which is how item 45 started. + m_queryEdit->setClearButtonEnabled(true); connect(m_queryEdit, &QLineEdit::returnPressed, this, &MainWindow::runCurrentQuery); @@ -453,24 +457,11 @@ void MainWindow::buildUi() m_syncLogPane->hide(); - m_syncButton = new QPushButton(tr("Sync"), central); - m_syncButton->setObjectName(QStringLiteral("syncButton")); + // Sync is reached from the toolbar, the File menu and the shortcut, all of + // them one QAction. A second QPushButton sat beside the query bar until + // 0.9.x, where it read as a Search button given what it stood next to, and + // carried behaviour the action did not: item 45. m_sync = new MailSync(m_config.syncCommand(), this); - m_syncButton->setEnabled(m_sync->isAvailable()); - if (!m_sync->isAvailable()) { - m_syncButton->setToolTip( - tr("No sync command configured ([sync] command in qtmaildir.conf)")); - } - connect(m_syncButton, &QPushButton::clicked, this, [this]() { - if (!m_sync->start()) { - showTransientStatus(tr("Sync already running")); - return; - } - // Fresh run, fresh output: leaving the previous run's lines in place - // makes a stale failure look like the current one. - m_syncLog->clear(); - setSyncBusy(true); - }); connect(m_sync, &MailSync::finished, this, &MainWindow::onSyncFinished); connect(m_sync, &MailSync::outputReceived, this, [this](const QString &chunk) { m_syncLog->appendPlainText(chunk.trimmed()); @@ -485,23 +476,25 @@ void MainWindow::buildUi() this, &MainWindow::onExternalSyncStateChanged); m_syncMonitor->start(); + // One row: the account dropdown, the query field, then the saved queries. + // The field is the only stretching item, so it is framed on both sides + // rather than running flush to the window edge, which is what the removed + // Sync button used to terminate. + // + // ponytail: no overflow handling. [queries] is unbounded and enough entries + // would squeeze the field, but three is the real-world case today. Item 23 + // already specifies buttons-plus-menu and is where that belongs. queryRow->addWidget(m_accountBox); queryRow->addWidget(m_queryEdit, 1); - queryRow->addWidget(m_syncButton); - layout->addLayout(queryRow); - - // Saved query buttons. - auto *savedRow = new QHBoxLayout; for (const SavedQuery &saved : m_config.savedQueries()) { auto *button = new QPushButton(saved.name, central); connect(button, &QPushButton::clicked, this, [this, saved]() { m_queryEdit->setText(saved.query); runCurrentQuery(); }); - savedRow->addWidget(button); + queryRow->addWidget(button); } - savedRow->addStretch(); - layout->addLayout(savedRow); + layout->addLayout(queryRow); // Thread list and message pane. m_model = new ThreadListModel(this); @@ -735,8 +728,7 @@ void MainWindow::registerActions() }); addAction(QStringLiteral("sync"), tr("&Sync"), tr("Run the configured sync command"), [this]() { - if (m_sync->isAvailable()) - m_sync->start(); + startSync(); }); addAction(QStringLiteral("complete_query"), tr("&Complete query"), tr("Offer completions for the query bar"), [this]() { @@ -870,7 +862,16 @@ void MainWindow::buildMenus() auto *toolBar = addToolBar(tr("Main")); toolBar->setObjectName(QStringLiteral("main_toolbar")); toolBar->setToolButtonStyle(Qt::ToolButtonTextBesideIcon); - toolBar->addAction(m_actions.value(QStringLiteral("sync"))); + QAction *syncAction = m_actions.value(QStringLiteral("sync")); + // Carried over from the QPushButton this replaced: with no command + // configured the control is disabled, and the tooltip is the only thing + // that says why. + if (syncAction && m_sync && !m_sync->isAvailable()) { + syncAction->setEnabled(false); + syncAction->setToolTip( + tr("No sync command configured ([sync] command in qtmaildir.conf)")); + } + toolBar->addAction(syncAction); toolBar->addSeparator(); toolBar->addAction(m_actions.value(QStringLiteral("archive"))); toolBar->addAction(m_actions.value(QStringLiteral("delete"))); @@ -1603,7 +1604,33 @@ void MainWindow::updateSyncControls() // /proc/locks could not be read and nothing was observed, so the button // stays usable: permanently disabling it where the lock cannot be seen is // worse than occasionally offering a run that gets skipped. - m_syncButton->setEnabled(!busy && m_sync && m_sync->isAvailable()); + // The QAction is the only Sync control now, and setEnabled on it reaches + // the toolbar button, the menu entry and the shortcut at once. Item 29 + // originally set a separate QPushButton and missed the action entirely, so + // the toolbar stayed clickable through a background sync. + if (QAction *action = m_actions.value(QStringLiteral("sync"))) + action->setEnabled(!busy && m_sync && m_sync->isAvailable()); +} + +void MainWindow::startSync() +{ + // One handler for every route in: the toolbar, the menu, the shortcut and + // the button. They previously had two, and only the button's cleared the + // log, showed the pane and disabled the control, so a sync started from the + // toolbar ran with no visible sign it had. + if (!m_sync->isAvailable()) { + showTransientStatus( + tr("No sync command configured ([sync] command in qtmaildir.conf)")); + return; + } + if (!m_sync->start()) { + showTransientStatus(tr("Sync already running")); + return; + } + // Fresh run, fresh output: leaving the previous run's lines in place + // makes a stale failure look like the current one. + m_syncLog->clear(); + setSyncBusy(true); } void MainWindow::recordPendingEdit(const QString &messageId, const QString &tag, diff --git a/src/mainwindow.h b/src/mainwindow.h index 96b27f4..e673181 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -217,6 +217,10 @@ private: /// says "working, duration unknown", which is the truth. void setSyncBusy(bool busy); + /// Starts a sync and shows that it started. Every route in goes through + /// here: the toolbar, the menu, the shortcut and the button. + void startSync(); + /// Applies the sync progress bar and button state from BOTH sync sources. /// /// One function of both, never two assignments: with a local and a @@ -322,7 +326,6 @@ private: QMenu *m_threadContextMenu = nullptr; QSplitter *m_splitter = nullptr; QComboBox *m_accountBox = nullptr; - QPushButton *m_syncButton = nullptr; QLabel *m_statusLabel = nullptr; /// Expires a transient status message. See showTransientStatus(). -- cgit v1.2.3