DiffEditor: Fix sync toggle

Use more QActions in the ToolBar. Those seem to work better
than the bare widgets.

Move "ignore WS" after the Context Lines spinbox so that we
can make the editor smaller while keeping the pop-up menu working.

Change-Id: I00d707c6ee1fb8d90fef5aa5ec5abb7149d4e6a0
Reviewed-by: Jarek Kobus <jaroslaw.kobus@theqtcompany.com>

src/plugins/diffeditor/diffeditor.cpp

@@ -62,8 +62,7 @@
 
 static const char settingsGroupC[] = "DiffEditor";
 static const char descriptionVisibleKeyC[] = "DescriptionVisible";
-static const char horizontalScrollBarSynchronizationKeyC[] =
+static const char horizontalScrollBarSynchronizationKeyC[] = "HorizontalScrollBarSynchronization";
-        "HorizontalScrollBarSynchronization";
 static const char contextLineCountKeyC[] = "ContextLineNumbers";
 static const char ignoreWhitespaceKeyC[] = "IgnoreWhitespace";
 
@@ -74,6 +73,19 @@ static const char useDiffEditorKeyC[] = "UseDiffEditor";
 
 using namespace TextEditor;
 
+namespace {
+
+class Guard
+{
+public:
+    Guard(int *state) : m_state(state) { ++(*state); }
+    ~Guard() { --(*m_state); QTC_ASSERT(*m_state >= 0, return); }
+private:
+    int *m_state;
+};
+
+} // namespace
+
 namespace DiffEditor {
 namespace Internal {
 
@@ -210,19 +222,19 @@ DiffEditor::DiffEditor(const QSharedPointer<DiffEditorDocument> &doc)
     , m_stackedWidget(0)
     , m_toolBar(0)
     , m_entriesComboBox(0)
+    , m_contextSpinBox(0)
     , m_toggleSyncAction(0)
     , m_whitespaceButtonAction(0)
-    , m_contextLabelAction(0)
-    , m_contextSpinBoxAction(0)
     , m_toggleDescriptionAction(0)
     , m_reloadAction(0)
-    , m_diffEditorSwitcher(0)
+    , m_viewSwitcherAction(0)
     , m_currentViewIndex(-1)
     , m_currentDiffFileIndex(-1)
+    , m_ignoreChanges(0)
     , m_sync(false)
     , m_showDescription(true)
-    , m_ignoreChanges(true)
 {
+    Guard guard(&m_ignoreChanges);
     QTC_ASSERT(m_document, return);
     setDuplicateSupported(true);
 
@@ -255,8 +267,6 @@ DiffEditor::DiffEditor(const QSharedPointer<DiffEditorDocument> &doc)
 
     loadSettings();
     updateDescription();
-
-    m_ignoreChanges = false;
 }
 
 DiffEditor::~DiffEditor()
@@ -314,58 +324,46 @@ QWidget *DiffEditor::toolBar()
             this, &DiffEditor::setCurrentDiffFileIndex);
     m_toolBar->addWidget(m_entriesComboBox);
 
-    m_whitespaceButton = new QToolButton(m_toolBar);
+    m_contextLabel = new QLabel(m_toolBar);
-    m_whitespaceButton->setText(tr("Ignore Whitespace"));
-    m_whitespaceButton->setCheckable(true);
-    m_whitespaceButton->setChecked(m_document->ignoreWhitespace());
-    m_whitespaceButtonAction = m_toolBar->addWidget(m_whitespaceButton);
 
-    QLabel *contextLabel = new QLabel(m_toolBar);
+    m_contextLabel->setText(tr("Context Lines:"));
-    contextLabel->setText(tr("Context Lines:"));
+    m_contextLabel->setContentsMargins(6, 0, 6, 0);
-    contextLabel->setContentsMargins(6, 0, 6, 0);
+    m_toolBar->addWidget(m_contextLabel);
-    m_contextLabelAction = m_toolBar->addWidget(contextLabel);
 
     m_contextSpinBox = new QSpinBox(m_toolBar);
     m_contextSpinBox->setRange(1, 100);
     m_contextSpinBox->setValue(m_document->contextLineCount());
     m_contextSpinBox->setFrame(false);
-    m_contextSpinBox->setSizePolicy(QSizePolicy::Minimum,
+    m_contextSpinBox->setSizePolicy(QSizePolicy::Minimum, QSizePolicy::Expanding); // Mac Qt5
-                                    QSizePolicy::Expanding); // Mac Qt5
+    m_toolBar->addWidget(m_contextSpinBox);
-    m_contextSpinBoxAction = m_toolBar->addWidget(m_contextSpinBox);
 
-    QToolButton *toggleDescription = new QToolButton(m_toolBar);
+    m_whitespaceButtonAction = m_toolBar->addAction(tr("Ignore Whitespace"));
-    toggleDescription->setIcon(QIcon(QLatin1String(Constants::ICON_TOP_BAR)));
+    m_whitespaceButtonAction->setCheckable(true);
-    toggleDescription->setCheckable(true);
+    m_whitespaceButtonAction->setChecked(m_document->ignoreWhitespace());
-    toggleDescription->setChecked(m_showDescription);
-    m_toggleDescriptionAction = m_toolBar->addWidget(toggleDescription);
-    updateDescription();
 
-    QToolButton *reloadButton = new QToolButton(m_toolBar);
+    m_toggleDescriptionAction = m_toolBar->addAction(QIcon(QLatin1String(Constants::ICON_TOP_BAR)),
-    reloadButton->setIcon(QIcon(QLatin1String(Core::Constants::ICON_RELOAD_GRAY)));
+                                               QString());
-    reloadButton->setToolTip(tr("Reload Editor"));
+    m_toggleDescriptionAction->setCheckable(true);
-    m_reloadAction = m_toolBar->addWidget(reloadButton);
+
+    m_reloadAction = m_toolBar->addAction(QIcon(QLatin1String(Core::Constants::ICON_RELOAD_GRAY)),
+                                          tr("Reload Diff"));
+    m_reloadAction->setToolTip(tr("Reload Diff"));
     documentStateChanged();
 
-    QToolButton *toggleSync = new QToolButton(m_toolBar);
+    m_toggleSyncAction = m_toolBar->addAction(QIcon(QLatin1String(Core::Constants::ICON_LINK)),
-    toggleSync->setIcon(QIcon(QLatin1String(Core::Constants::ICON_LINK)));
+                                        QString());
-    toggleSync->setCheckable(true);
+    m_toggleSyncAction->setCheckable(true);
-    m_toggleSyncAction = m_toolBar->addWidget(toggleSync);
 
-    m_diffEditorSwitcher = new QToolButton(m_toolBar);
+    m_viewSwitcherAction = m_toolBar->addAction(QIcon(), QString());
-    m_toolBar->addWidget(m_diffEditorSwitcher);
     updateDiffEditorSwitcher();
 
-    connect(m_whitespaceButton, &QToolButton::clicked,
+    connect(m_whitespaceButtonAction, &QAction::toggled, this, &DiffEditor::ignoreWhitespaceHasChanged);
-            this, &DiffEditor::ignoreWhitespaceHasChanged);
     connect(m_contextSpinBox, static_cast<void (QSpinBox::*)(int)>(&QSpinBox::valueChanged),
             this, &DiffEditor::contextLineCountHasChanged);
-    connect(toggleSync, &QAbstractButton::clicked, this, &DiffEditor::toggleSync);
+    connect(m_toggleSyncAction, &QAction::toggled, this, &DiffEditor::toggleSync);
-    connect(toggleDescription, &QAbstractButton::clicked,
+    connect(m_toggleDescriptionAction, &QAction::toggled, this, &DiffEditor::toggleDescription);
-            this, &DiffEditor::toggleDescription);
+    connect(m_viewSwitcherAction, &QAction::triggered, this, [this]() { showDiffView(nextView()); });
-    connect(m_diffEditorSwitcher, &QAbstractButton::clicked,
+    connect(m_reloadAction, &QAction::triggered, this, [this]() { m_document->reload(); });
-            this, [this]() { showDiffView(nextView()); });
-
-    connect(reloadButton, &QAbstractButton::clicked, this, [this]() { m_document->reload(); });
     connect(m_document.data(), &DiffEditorDocument::temporaryStateChanged,
             this, &DiffEditor::documentStateChanged);
 
@@ -374,13 +372,15 @@ QWidget *DiffEditor::toolBar()
 
 void DiffEditor::documentHasChanged()
 {
-    m_ignoreChanges = true;
+    int index = 0;
+    {
+        Guard guard(&m_ignoreChanges);
         const QList<FileData> diffFileList = m_document->diffFiles();
 
+        updateDescription();
         currentView()->setDiff(diffFileList, m_document->baseDirectory());
 
         m_entriesComboBox->clear();
-    int index = 0;
         const int count = diffFileList.count();
         for (int i = 0; i < count; i++) {
             const DiffFileInfo leftEntry = diffFileList.at(i).leftFileInfo;
@@ -432,14 +432,16 @@ void DiffEditor::documentHasChanged()
             m_entriesComboBox->setItemData(m_entriesComboBox->count() - 1,
                                            itemToolTip, Qt::ToolTipRole);
         }
-
+    }
-    m_ignoreChanges = false;
 
     setCurrentDiffFileIndex(m_entriesComboBox->count() > 0 ? index : -1);
 }
 
 void DiffEditor::toggleDescription()
 {
+    if (m_ignoreChanges > 0)
+        return;
+
     m_showDescription = !m_showDescription;
     saveSetting(QLatin1String(descriptionVisibleKeyC), m_showDescription);
     updateDescription();
@@ -447,15 +449,17 @@ void DiffEditor::toggleDescription()
 
 void DiffEditor::updateDescription()
 {
+    QTC_ASSERT(m_toolBar, return);
+
     QString description = m_document->description();
     m_descriptionWidget->setPlainText(description);
     m_descriptionWidget->setVisible(m_showDescription && !description.isEmpty());
 
-    QTC_ASSERT(m_toolBar, return);
+    Guard guard(&m_ignoreChanges);
-    QTC_ASSERT(m_toggleDescriptionAction, return);
+    m_toggleDescriptionAction->setChecked(m_showDescription);
-
+    m_toggleDescriptionAction->setToolTip(m_showDescription ? tr("Hide Change Description")
-    QWidget *toggle = m_toolBar->widgetForAction(m_toggleDescriptionAction);
+                                                      : tr("Show Change Description"));
-    toggle->setToolTip(m_showDescription ? tr("Hide Change Description")
+    m_toggleDescriptionAction->setText(m_showDescription ? tr("Hide Change Description")
                                                    : tr("Show Change Description"));
     m_toggleDescriptionAction->setVisible(!description.isEmpty());
 }
@@ -463,7 +467,7 @@ void DiffEditor::updateDescription()
 void DiffEditor::contextLineCountHasChanged(int lines)
 {
     QTC_ASSERT(!m_document->isContextLineCountForced(), return);
-    if (m_ignoreChanges || lines == m_document->contextLineCount())
+    if (m_ignoreChanges > 0 || lines == m_document->contextLineCount())
         return;
 
     m_document->setContextLineCount(lines);
@@ -472,13 +476,15 @@ void DiffEditor::contextLineCountHasChanged(int lines)
     m_document->reload();
 }
 
-void DiffEditor::ignoreWhitespaceHasChanged(bool ignore)
+void DiffEditor::ignoreWhitespaceHasChanged()
 {
-    if (m_ignoreChanges || ignore == m_document->ignoreWhitespace())
+    const bool ignore = m_whitespaceButtonAction->isChecked();
-        return;
 
+    if (m_ignoreChanges > 0 || ignore == m_document->ignoreWhitespace())
+        return;
     m_document->setIgnoreWhitespace(ignore);
     saveSetting(QLatin1String(ignoreWhitespaceKeyC), ignore);
+
     m_document->reload();
 }
 
@@ -496,10 +502,11 @@ void DiffEditor::prepareForReload()
         m_currentFileChunk = qMakePair(QString(), QString());
     }
 
-    m_ignoreChanges = true;
+    {
+        Guard guard(&m_ignoreChanges);
         m_contextSpinBox->setValue(m_document->contextLineCount());
-    m_whitespaceButton->setChecked(m_document->ignoreWhitespace());
+        m_whitespaceButtonAction->setChecked(m_document->ignoreWhitespace());
-    m_ignoreChanges = false;
+    }
     currentView()->beginOperation();
 }
 
@@ -522,20 +529,17 @@ void DiffEditor::updateEntryToolTip()
 
 void DiffEditor::setCurrentDiffFileIndex(int index)
 {
-    if (m_ignoreChanges)
+    if (m_ignoreChanges > 0)
         return;
 
     QTC_ASSERT((index < 0) != (m_entriesComboBox->count() > 0), return);
 
-    m_ignoreChanges = true;
+    Guard guard(&m_ignoreChanges);
-
     m_currentDiffFileIndex = index;
     currentView()->setCurrentDiffFileIndex(index);
 
     m_entriesComboBox->setCurrentIndex(m_entriesComboBox->count() > 0 ? qMax(0, index) : -1);
     updateEntryToolTip();
-
-    m_ignoreChanges = false;
 }
 
 void DiffEditor::documentStateChanged()
@@ -544,22 +548,26 @@ void DiffEditor::documentStateChanged()
     const bool contextVisible = !m_document->isContextLineCountForced();
 
     m_whitespaceButtonAction->setVisible(canReload);
-    m_contextLabelAction->setVisible(canReload && contextVisible);
+    m_contextLabel->setVisible(canReload && contextVisible);
-    m_contextSpinBoxAction->setVisible(canReload && contextVisible);
+    m_contextSpinBox->setVisible(canReload && contextVisible);
     m_reloadAction->setVisible(canReload);
 }
 
 void DiffEditor::updateDiffEditorSwitcher()
 {
-    if (!m_diffEditorSwitcher)
+    if (!m_viewSwitcherAction)
         return;
     IDiffView *next = nextView();
-    m_diffEditorSwitcher->setIcon(next->icon());
+    m_viewSwitcherAction->setIcon(next->icon());
-    m_diffEditorSwitcher->setToolTip(next->toolTip());
+    m_viewSwitcherAction->setToolTip(next->toolTip());
+    m_viewSwitcherAction->setText(next->toolTip());
 }
 
 void DiffEditor::toggleSync()
 {
+    if (m_ignoreChanges > 0)
+        return;
+
     QTC_ASSERT(currentView(), return);
     m_sync = !m_sync;
     saveSetting(QLatin1String(horizontalScrollBarSynchronizationKeyC), m_sync);
@@ -587,10 +595,8 @@ void DiffEditor::loadSettings()
 
     // Read current settings:
     s->beginGroup(QLatin1String(settingsGroupC));
-    m_showDescription = s->value(QLatin1String(descriptionVisibleKeyC),
+    m_showDescription = s->value(QLatin1String(descriptionVisibleKeyC), true).toBool();
-                                 true).toBool();
+    m_sync = s->value(QLatin1String(horizontalScrollBarSynchronizationKeyC), true).toBool();
-    m_sync = s->value(QLatin1String(horizontalScrollBarSynchronizationKeyC),
-                                true).toBool();
     m_document->setIgnoreWhitespace(s->value(QLatin1String(ignoreWhitespaceKeyC), false).toBool());
     m_document->setContextLineCount(s->value(QLatin1String(contextLineCountKeyC), 3).toInt());
     Core::Id id = Core::Id::fromSetting(s->value(QLatin1String(diffViewKeyC)));
@@ -651,9 +657,13 @@ void DiffEditor::setupView(IDiffView *view)
 
     saveSetting(QLatin1String(diffViewKeyC), currentView()->id().toSetting());
 
+    {
+        Guard guard(&m_ignoreChanges);
         m_toggleSyncAction->setVisible(currentView()->supportsSync());
         m_toggleSyncAction->setToolTip(currentView()->syncToolTip());
+        m_toggleSyncAction->setText(currentView()->syncToolTip());
         m_toggleSyncAction->setChecked(m_sync);
+    }
 
     view->setDocument(m_document.data());
     view->setSync(m_sync);

src/plugins/diffeditor/diffeditor.h

@@ -38,6 +38,7 @@
 
 QT_BEGIN_NAMESPACE
 class QComboBox;
+class QLabel;
 class QSpinBox;
 class QToolBar;
 class QToolButton;
@@ -76,7 +77,7 @@ private slots:
     void toggleDescription();
     void updateDescription();
     void contextLineCountHasChanged(int lines);
-    void ignoreWhitespaceHasChanged(bool ignore);
+    void ignoreWhitespaceHasChanged();
     void prepareForReload();
     void reloadHasFinished(bool success);
     void setCurrentDiffFileIndex(int index);
@@ -102,21 +103,19 @@ private:
     QVector<IDiffView *> m_views;
     QToolBar *m_toolBar;
     QComboBox *m_entriesComboBox;
-    QToolButton *m_whitespaceButton;
     QSpinBox *m_contextSpinBox;
     QAction *m_toggleSyncAction;
     QAction *m_whitespaceButtonAction;
-    QAction *m_contextLabelAction;
-    QAction *m_contextSpinBoxAction;
     QAction *m_toggleDescriptionAction;
     QAction *m_reloadAction;
-    QToolButton *m_diffEditorSwitcher;
+    QLabel *m_contextLabel;
+    QAction *m_viewSwitcherAction;
     QPair<QString, QString> m_currentFileChunk;
     int m_currentViewIndex;
     int m_currentDiffFileIndex;
+    int m_ignoreChanges;
     bool m_sync;
     bool m_showDescription;
-    bool m_ignoreChanges;
 };
 
 } // namespace Internal