ClangTools: Move checkbox from view's header to toolbar

Avoid the following issues with the diagnostic view's header:
* Clicking on the header to reverse the sorting is somewhat pointless as
  there is only one column.
* It takes vertical space.
* The checkbox to select/unselect all fixits for application is hacky,
  not drawn nicely on Windows and macOS and its position is somewhat
  problematic as on hover the dock widgets handles are popping up.
* To check the check box, one needs to click within the check box
  rectangle, which is a pretty small area of the screen.

Instead, add a proper checkbox with a label to the toolbar (apparently
this needs some adaptions to our ManhattenStyle). By positioning it
before the "Apply Fixits" button, we can streamline the work flow.

Change-Id: I4ff40c3641487428feb1cd8305470dc5219d048c
Reviewed-by: Cristian Adam <cristian.adam@qt.io>

6 files changed, 140 insertions, 124 deletions

diff --git a/src/plugins/clangtools/clangtool.cpp b/src/plugins/clangtools/clangtool.cpp
index be93ef804f3..bad06028518 100644
--- a/src/plugins/clangtools/clangtool.cpp
+++ b/src/plugins/clangtools/clangtool.cpp
@@ -64,6 +64,7 @@
 #include <utils/utilsicons.h>
 
 #include <QAction>
+#include <QCheckBox>
 #include <QFileDialog>
 #include <QLabel>
 #include <QSortFilterProxyModel>
@@ -80,6 +81,17 @@ namespace Internal {
 
 static ClangTool *s_instance;
 
+class SelectFixitsCheckBox : public QCheckBox
+{
+    Q_OBJECT
+
+private:
+    void nextCheckState() final override
+    {
+        setCheckState(checkState() == Qt::Checked ? Qt::Unchecked : Qt::Checked);
+    }
+};
+
 class ApplyFixIts
 {
 public:
@@ -381,16 +393,36 @@ ClangTool::ClangTool()
             QRegExp(filter, Qt::CaseSensitive, QRegExp::WildcardUnix));
     });
 
+    // Schedule/Unschedule all fixits
+    m_selectFixitsCheckBox = new SelectFixitsCheckBox;
+    m_selectFixitsCheckBox->setText("Select Fixits");
+    m_selectFixitsCheckBox->setEnabled(false);
+    m_selectFixitsCheckBox->setTristate(true);
+    connect(m_selectFixitsCheckBox, &QCheckBox::clicked, this, [this]() {
+        auto view = static_cast<DiagnosticView *>(m_diagnosticView.data());
+        view->scheduleAllFixits(m_selectFixitsCheckBox->isChecked());
+    });
+
     // Apply fixits button
     m_applyFixitsButton = new QToolButton;
     m_applyFixitsButton->setText(tr("Apply Fixits"));
     m_applyFixitsButton->setEnabled(false);
-    connect(m_diagnosticModel,
-            &ClangToolsDiagnosticModel::fixItsToApplyCountChanged,
-            [this](int c) {
-        m_applyFixitsButton->setEnabled(c);
-        static_cast<DiagnosticView *>(m_diagnosticView.data())->setSelectedFixItsCount(c);
-    });
+
+    connect(m_diagnosticModel, &ClangToolsDiagnosticModel::fixitStatusChanged,
+            m_diagnosticFilterModel, &DiagnosticFilterModel::onFixitStatusChanged);
+    connect(m_diagnosticFilterModel, &DiagnosticFilterModel::fixitStatisticsChanged,
+            this,
+            [this](int scheduled, int scheduableTotal){
+                m_selectFixitsCheckBox->setEnabled(scheduableTotal > 0);
+                m_applyFixitsButton->setEnabled(scheduled > 0);
+
+                if (scheduled == 0)
+                    m_selectFixitsCheckBox->setCheckState(Qt::Unchecked);
+                else if (scheduled == scheduableTotal)
+                    m_selectFixitsCheckBox->setCheckState(Qt::Checked);
+                else
+                    m_selectFixitsCheckBox->setCheckState(Qt::PartiallyChecked);
+            });
     connect(m_applyFixitsButton, &QToolButton::clicked, [this]() {
         QVector<DiagnosticItem *> diagnosticItems;
         m_diagnosticModel->forItemsAtLevel<2>([&](DiagnosticItem *item){
@@ -443,6 +475,8 @@ ClangTool::ClangTool()
     m_perspective.addToolBarAction(m_goBack);
     m_perspective.addToolBarAction(m_goNext);
     m_perspective.addToolBarWidget(m_filterLineEdit);
+    m_perspective.addToolbarSeparator();
+    m_perspective.addToolBarWidget(m_selectFixitsCheckBox);
     m_perspective.addToolBarWidget(m_applyFixitsButton);
 
     updateRunActions();
@@ -511,6 +545,7 @@ void ClangTool::startTool(ClangTool::FileSelection fileSelection,
     m_diagnosticModel->clear();
 
     m_diagnosticFilterModel->setProject(project);
+    m_selectFixitsCheckBox->setEnabled(false);
     m_applyFixitsButton->setEnabled(false);
     m_running = true;
 
@@ -787,3 +822,5 @@ void ClangTool::setToolBusy(bool busy)
 
 } // namespace Internal
 } // namespace ClangTools
+
+#include "clangtool.moc"
diff --git a/src/plugins/clangtools/clangtool.h b/src/plugins/clangtools/clangtool.h
index d994a6b2f9c..7835bdcd678 100644
--- a/src/plugins/clangtools/clangtool.h
+++ b/src/plugins/clangtools/clangtool.h
@@ -56,6 +56,7 @@ class ClangToolsDiagnosticModel;
 class Diagnostic;
 class DiagnosticFilterModel;
 class RunSettings;
+class SelectFixitsCheckBox;
 
 const char ClangTidyClazyPerspectiveId[] = "ClangTidyClazy.Perspective";
 
@@ -127,6 +128,7 @@ private:
     DiagnosticFilterModel *m_diagnosticFilterModel = nullptr;
 
     Utils::FancyLineEdit *m_filterLineEdit = nullptr;
+    SelectFixitsCheckBox *m_selectFixitsCheckBox = nullptr;
     QToolButton *m_applyFixitsButton = nullptr;
 
     QAction *m_openProjectSettings = nullptr;
diff --git a/src/plugins/clangtools/clangtoolsdiagnosticmodel.cpp b/src/plugins/clangtools/clangtoolsdiagnosticmodel.cpp
index 6608bb3ea4e..f45a7abef4d 100644
--- a/src/plugins/clangtools/clangtoolsdiagnosticmodel.cpp
+++ b/src/plugins/clangtools/clangtoolsdiagnosticmodel.cpp
@@ -84,7 +84,6 @@ ClangToolsDiagnosticModel::ClangToolsDiagnosticModel(QObject *parent)
     : ClangToolsDiagnosticModelBase(parent)
     , m_filesWatcher(std::make_unique<QFileSystemWatcher>())
 {
-    setHeader({tr("Diagnostic")});
     connectFileWatcher();
 }
 
@@ -101,13 +100,10 @@ QDebug operator<<(QDebug debug, const Diagnostic &d)
 
 void ClangToolsDiagnosticModel::addDiagnostics(const Diagnostics &diagnostics)
 {
-    const auto onFixitStatusChanged = [this](FixitStatus oldStatus, FixitStatus newStatus) {
-        if (newStatus == FixitStatus::Scheduled)
-            ++m_fixItsToApplyCount;
-        else if (oldStatus == FixitStatus::Scheduled)
-            --m_fixItsToApplyCount;
-        emit fixItsToApplyCountChanged(m_fixItsToApplyCount);
-    };
+    const auto onFixitStatusChanged =
+        [this](const QModelIndex &index, FixitStatus oldStatus, FixitStatus newStatus) {
+            emit fixitStatusChanged(index, oldStatus, newStatus);
+        };
 
     for (const Diagnostic &d : diagnostics) {
         // Check for duplicates
@@ -141,10 +137,12 @@ QSet<Diagnostic> ClangToolsDiagnosticModel::diagnostics() const
 
 void ClangToolsDiagnosticModel::clear()
 {
+    beginResetModel();
     m_filePathToItem.clear();
     m_diagnostics.clear();
     clearAndSetupCache();
     ClangToolsDiagnosticModelBase::clear();
+    endResetModel();
 }
 
 void ClangToolsDiagnosticModel::updateItems(const DiagnosticItem *changedItem)
@@ -463,7 +461,7 @@ void DiagnosticItem::setFixItStatus(const FixitStatus &status)
     m_fixitStatus = status;
     update();
     if (m_onFixitStatusChanged && status != oldStatus)
-        m_onFixitStatusChanged(oldStatus, status);
+        m_onFixitStatusChanged(index(), oldStatus, status);
 }
 
 void DiagnosticItem::setFixitOperations(const ReplacementOperations &replacements)
@@ -554,6 +552,21 @@ DiagnosticFilterModel::DiagnosticFilterModel(QObject *parent)
                 if (!m_project && project->projectDirectory() == m_lastProjectDirectory)
                     setProject(project);
             });
+    connect(this, &QAbstractItemModel::modelReset, this, [this]() {
+        m_fixItsScheduled = 0;
+        m_fixItsScheduableInTotal = 0;
+        emit fixitStatisticsChanged(m_fixItsScheduled, m_fixItsScheduableInTotal);
+    });
+    connect(this, &QAbstractItemModel::rowsInserted,
+            this, [this](const QModelIndex &parent, int first, int last) {
+        m_fixItsScheduableInTotal += diagnosticsWithFixits(parent, first, last);
+        emit fixitStatisticsChanged(m_fixItsScheduled, m_fixItsScheduableInTotal);
+    });
+    connect(this, &QAbstractItemModel::rowsAboutToBeRemoved,
+            this, [this](const QModelIndex &parent, int first, int last) {
+        m_fixItsScheduableInTotal -= diagnosticsWithFixits(parent, first, last);
+        emit fixitStatisticsChanged(m_fixItsScheduled, m_fixItsScheduableInTotal);
+    });
 }
 
 void DiagnosticFilterModel::setProject(ProjectExplorer::Project *project)
@@ -572,8 +585,7 @@ void DiagnosticFilterModel::setProject(ProjectExplorer::Project *project)
     handleSuppressedDiagnosticsChanged();
 }
 
-void DiagnosticFilterModel::addSuppressedDiagnostic(
-        const SuppressedDiagnostic &diag)
+void DiagnosticFilterModel::addSuppressedDiagnostic(const SuppressedDiagnostic &diag)
 {
     QTC_ASSERT(!m_project, return);
     m_suppressedDiagnostics << diag;
@@ -585,8 +597,45 @@ void DiagnosticFilterModel::invalidateFilter()
     QSortFilterProxyModel::invalidateFilter();
 }
 
-bool DiagnosticFilterModel::filterAcceptsRow(int sourceRow,
-        const QModelIndex &sourceParent) const
+void DiagnosticFilterModel::onFixitStatusChanged(const QModelIndex &sourceIndex,
+                                                 FixitStatus oldStatus,
+                                                 FixitStatus newStatus)
+{
+    if (!mapFromSource(sourceIndex).isValid())
+        return;
+
+    if (newStatus == FixitStatus::Scheduled)
+        ++m_fixItsScheduled;
+    else if (oldStatus == FixitStatus::Scheduled) {
+        --m_fixItsScheduled;
+        if (newStatus != FixitStatus::NotScheduled)
+            --m_fixItsScheduableInTotal;
+    }
+
+    emit fixitStatisticsChanged(m_fixItsScheduled, m_fixItsScheduableInTotal);
+}
+
+int DiagnosticFilterModel::diagnosticsWithFixits(const QModelIndex &parent,
+                                                 int first,
+                                                 int last) const
+{
+    if (!parent.isValid())
+        return 0;
+
+    int count = 0;
+    auto model = static_cast<ClangToolsDiagnosticModel *>(sourceModel());
+    for (int idx = first; idx <= last; ++idx) {
+        Utils::TreeItem *treeItem = model->itemForIndex(mapToSource(index(idx, 0, parent)));
+        if (treeItem->level() == 2) {
+            if (static_cast<DiagnosticItem *>(treeItem)->diagnostic().hasFixits)
+                ++count;
+        }
+    }
+
+    return count;
+}
+
+bool DiagnosticFilterModel::filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const
 {
     auto model = static_cast<ClangToolsDiagnosticModel *>(sourceModel());
 
diff --git a/src/plugins/clangtools/clangtoolsdiagnosticmodel.h b/src/plugins/clangtools/clangtoolsdiagnosticmodel.h
index d9fb8b4cfb9..1ecfc0e7913 100644
--- a/src/plugins/clangtools/clangtoolsdiagnosticmodel.h
+++ b/src/plugins/clangtools/clangtoolsdiagnosticmodel.h
@@ -71,7 +71,8 @@ private:
 class DiagnosticItem : public Utils::TreeItem
 {
 public:
-    using OnFixitStatusChanged = std::function<void(FixitStatus oldStatus, FixitStatus newStatus)>;
+    using OnFixitStatusChanged
+        = std::function<void(const QModelIndex &index, FixitStatus oldStatus, FixitStatus newStatus)>;
     DiagnosticItem(const Diagnostic &diag,
                    const OnFixitStatusChanged &onFixitStatusChanged,
                    ClangToolsDiagnosticModel *parent);
@@ -128,7 +129,7 @@ public:
     void addWatchedPath(const QString &path);
 
 signals:
-    void fixItsToApplyCountChanged(int count);
+    void fixitStatusChanged(const QModelIndex &index, FixitStatus oldStatus, FixitStatus newStatus);
 
 private:
     void connectFileWatcher();
@@ -141,7 +142,6 @@ private:
     QSet<Diagnostic> m_diagnostics;
     std::map<QVector<ExplainingStep>, QVector<DiagnosticItem *>> stepsToItemsCache;
     std::unique_ptr<QFileSystemWatcher> m_filesWatcher;
-    int m_fixItsToApplyCount = 0;
 };
 
 class DiagnosticFilterModel : public QSortFilterProxyModel
@@ -157,14 +157,26 @@ public:
 
     void invalidateFilter();
 
+    void onFixitStatusChanged(const QModelIndex &sourceIndex,
+                              FixitStatus oldStatus,
+                              FixitStatus newStatus);
+
+signals:
+    void fixitStatisticsChanged(int scheduled, int scheduableTotal);
+
 private:
     bool filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const override;
     bool lessThan(const QModelIndex &l, const QModelIndex &r) const override;
+
+    int diagnosticsWithFixits(const QModelIndex &parent, int first, int last) const;
     void handleSuppressedDiagnosticsChanged();
 
     QPointer<ProjectExplorer::Project> m_project;
     Utils::FilePath m_lastProjectDirectory;
     SuppressedDiagnosticsList m_suppressedDiagnostics;
+
+    int m_fixItsScheduableInTotal = 0;
+    int m_fixItsScheduled = 0;
 };
 
 } // namespace Internal
diff --git a/src/plugins/clangtools/clangtoolsdiagnosticview.cpp b/src/plugins/clangtools/clangtoolsdiagnosticview.cpp
index 769266b5e01..5df289b8e86 100644
--- a/src/plugins/clangtools/clangtoolsdiagnosticview.cpp
+++ b/src/plugins/clangtools/clangtoolsdiagnosticview.cpp
@@ -46,65 +46,6 @@ using namespace Debugger;
 namespace ClangTools {
 namespace Internal {
 
-// A header view that puts a check box in front of a given column.
-class HeaderWithCheckBoxInColumn : public QHeaderView
-{
-    Q_OBJECT
-
-public:
-    HeaderWithCheckBoxInColumn(Qt::Orientation orientation,
-                               int column = 0,
-                               QWidget *parent = nullptr)
-        : QHeaderView(orientation, parent)
-        , m_column(column)
-    {
-        setDefaultAlignment(Qt::AlignLeft);
-    }
-
-    void setState(QFlags<QStyle::StateFlag> newState) { state = newState; }
-
-protected:
-    void paintSection(QPainter *painter, const QRect &rect, int logicalIndex) const override
-    {
-        painter->save();
-        QHeaderView::paintSection(painter, rect, logicalIndex);
-        painter->restore();
-        if (logicalIndex == m_column) {
-            QStyleOptionButton option;
-            const int side = sizeHint().height();
-            option.rect = QRect(rect.left() + 1, 1, side - 3, side - 3);
-            option.state = state;
-            painter->save();
-            const int shift = side - 2;
-            painter->translate(QPoint(shift, 0));
-            QHeaderView::paintSection(painter, rect.adjusted(0, 0, -shift, 0), logicalIndex);
-            painter->restore();
-            style()->drawPrimitive(QStyle::PE_IndicatorCheckBox, &option, painter);
-        }
-    }
-
-    void mouseReleaseEvent(QMouseEvent *event) override
-    {
-        const int x = event->localPos().x();
-        const int columnX = sectionPosition(m_column);
-        const bool isWithinCheckBox = x > columnX && x < columnX + sizeHint().height() - 3;
-        if (isWithinCheckBox) {
-            state = (state != QStyle::State_On) ? QStyle::State_On : QStyle::State_Off;
-            viewport()->update();
-            emit checkBoxClicked(state == QStyle::State_On);
-            return; // Avoid changing sort order
-        }
-        QHeaderView::mouseReleaseEvent(event);
-    }
-
-signals:
-    void checkBoxClicked(bool checked);
-
-private:
-    const int m_column = 0;
-    QFlags<QStyle::StateFlag> state = QStyle::State_Off;
-};
-
 static QString getBaseStyleName()
 {
     QStyle *style = QApplication::style();
@@ -180,6 +121,7 @@ DiagnosticView::DiagnosticView(QWidget *parent)
     , m_style(new DiagnosticViewStyle)
     , m_delegate(new DiagnosticViewDelegate(m_style.get()))
 {
+    header()->hide();
     m_suppressAction = new QAction(tr("Suppress This Diagnostic"), this);
     connect(m_suppressAction, &QAction::triggered,
             this, &DiagnosticView::suppressCurrentDiagnostic);
@@ -189,6 +131,22 @@ DiagnosticView::DiagnosticView(QWidget *parent)
     setItemDelegate(m_delegate.get());
 }
 
+void DiagnosticView::scheduleAllFixits(bool schedule)
+{
+    const auto proxyModel = static_cast<QSortFilterProxyModel *>(model());
+    for (int i = 0, count = proxyModel->rowCount(); i < count; ++i) {
+        const QModelIndex filePathItemIndex = proxyModel->index(i, 0);
+        for (int j = 0, count = proxyModel->rowCount(filePathItemIndex); j < count; ++j) {
+            const QModelIndex proxyIndex = proxyModel->index(j, 0, filePathItemIndex);
+            const QModelIndex diagnosticItemIndex = proxyModel->mapToSource(proxyIndex);
+            auto item = static_cast<DiagnosticItem *>(diagnosticItemIndex.internalPointer());
+            item->setData(DiagnosticView::DiagnosticColumn,
+                          schedule ? Qt::Checked : Qt::Unchecked,
+                          Qt::CheckStateRole);
+        }
+    }
+}
+
 DiagnosticView::~DiagnosticView() = default;
 
 void DiagnosticView::suppressCurrentDiagnostic()
@@ -293,17 +251,6 @@ void DiagnosticView::mouseDoubleClickEvent(QMouseEvent *event)
     Debugger::DetailedErrorView::mouseDoubleClickEvent(event);
 }
 
-void DiagnosticView::setSelectedFixItsCount(int fixItsCount)
-{
-    if (m_ignoreSetSelectedFixItsCount)
-        return;
-    auto checkBoxHeader = static_cast<HeaderWithCheckBoxInColumn *>(header());
-    checkBoxHeader->setState(fixItsCount
-                                 ? (QStyle::State_NoChange | QStyle::State_On | QStyle::State_Off)
-                                 : QStyle::State_Off);
-    checkBoxHeader->viewport()->update();
-}
-
 void DiagnosticView::openEditorForCurrentIndex()
 {
     const QVariant v = model()->data(currentIndex(), Debugger::DetailedErrorView::LocationRole);
@@ -312,36 +259,6 @@ void DiagnosticView::openEditorForCurrentIndex()
         Core::EditorManager::openEditorAt(loc.filePath, loc.line, loc.column - 1);
 }
 
-void DiagnosticView::setModel(QAbstractItemModel *theProxyModel)
-{
-    const auto proxyModel = static_cast<QSortFilterProxyModel *>(theProxyModel);
-    Debugger::DetailedErrorView::setModel(proxyModel);
-
-    auto *header = new HeaderWithCheckBoxInColumn(Qt::Horizontal,
-                                                  DiagnosticView::DiagnosticColumn,
-                                                  this);
-    connect(header, &HeaderWithCheckBoxInColumn::checkBoxClicked, this, [=](bool checked) {
-        m_ignoreSetSelectedFixItsCount = true;
-        for (int i = 0, count = proxyModel->rowCount(); i < count; ++i) {
-            const QModelIndex filePathItemIndex = proxyModel->index(i, 0);
-            for (int j = 0, count = proxyModel->rowCount(filePathItemIndex); j < count; ++j) {
-                const QModelIndex proxyIndex = proxyModel->index(j, 0, filePathItemIndex);
-                const QModelIndex diagnosticItemIndex = proxyModel->mapToSource(proxyIndex);
-                auto item = static_cast<DiagnosticItem *>(diagnosticItemIndex.internalPointer());
-                item->setData(DiagnosticView::DiagnosticColumn,
-                              checked ? Qt::Checked : Qt::Unchecked,
-                              Qt::CheckStateRole);
-            }
-        }
-        m_ignoreSetSelectedFixItsCount = false;
-    });
-    setHeader(header);
-    header->setSectionResizeMode(DiagnosticView::DiagnosticColumn, QHeaderView::Stretch);
-    const int columnWidth = header->sectionSizeHint(DiagnosticView::DiagnosticColumn);
-    const int checkboxWidth = header->height();
-    header->setMinimumSectionSize(columnWidth + 1.2 * checkboxWidth);
-}
-
 } // namespace Internal
 } // namespace ClangTools
 
diff --git a/src/plugins/clangtools/clangtoolsdiagnosticview.h b/src/plugins/clangtools/clangtoolsdiagnosticview.h
index 6bc61351016..d8698f846f2 100644
--- a/src/plugins/clangtools/clangtoolsdiagnosticview.h
+++ b/src/plugins/clangtools/clangtoolsdiagnosticview.h
@@ -43,7 +43,7 @@ public:
     DiagnosticView(QWidget *parent = nullptr);
     ~DiagnosticView() override;
 
-    void setSelectedFixItsCount(int fixItsCount);
+    void scheduleAllFixits(bool schedule);
 
 private:
     void openEditorForCurrentIndex();
@@ -58,7 +58,6 @@ private:
     QList<QAction *> customActions() const override;
     bool eventFilter(QObject *watched, QEvent *event) override;
     void mouseDoubleClickEvent(QMouseEvent *event) override;
-    void setModel(QAbstractItemModel *theProxyModel) override;
 
     QAction *m_suppressAction;
     std::unique_ptr<DiagnosticViewStyle> m_style;