BaseFileFind/FindInFiles: Clean up enabled / valid state handling

IFindFilter:  Separate enabledChanged and validChanged
SearchEngine: Add enabledChanged
GitGrep:      Enable widget based on enabled state
FindInFiles:  Update validity on search engine enabledChanged instead of
              hardcoding it to when the search path changed

Change-Id: I0c684423b871a3a4f1f164574f42e23b64cf9da0
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/coreplugin/find/findtoolwindow.cpp

@@ -196,12 +196,17 @@ void FindToolWindow::setCurrentFilter(int index)
         QWidget *configWidget = m_configWidgets.at(i);
         if (i == index) {
             m_configWidget = configWidget;
-            if (m_currentFilter)
+            if (m_currentFilter) {
                 disconnect(m_currentFilter, &IFindFilter::enabledChanged,
                            this, &FindToolWindow::updateButtonStates);
+                disconnect(m_currentFilter, &IFindFilter::validChanged,
+                           this, &FindToolWindow::updateButtonStates);
+            }
             m_currentFilter = m_filters.at(i);
             connect(m_currentFilter, &IFindFilter::enabledChanged,
                     this, &FindToolWindow::updateButtonStates);
+            connect(m_currentFilter, &IFindFilter::validChanged,
+                    this, &FindToolWindow::updateButtonStates);
             updateButtonStates();
             if (m_configWidget)
                 m_ui.configWidget->layout()->addWidget(m_configWidget);

src/plugins/coreplugin/find/ifindfilter.h

@@ -64,6 +64,7 @@ public:
     static QString descriptionForFindFlags(FindFlags flags);
 signals:
     void enabledChanged(bool enabled);
+    void validChanged(bool valid);
     void displayNameChanged();
 };
 

src/plugins/git/gitgrep.cpp

@@ -198,7 +198,7 @@ private:
 
 } // namespace
 
-static bool validateDirectory(const QString &path)
+static bool isGitDirectory(const QString &path)
 {
     static IVersionControl *gitVc = VcsManager::versionControl(VcsBase::Constants::VCS_ID_GIT);
     QTC_ASSERT(gitVc, return false);
@@ -221,8 +221,9 @@ GitGrep::GitGrep()
     QTC_ASSERT(findInFiles, return);
     connect(findInFiles, &TextEditor::FindInFiles::pathChanged,
             m_widget, [this](const QString &path) {
-        m_widget->setEnabled(validateDirectory(path));
+        setEnabled(isGitDirectory(path));
     });
+    connect(this, &SearchEngine::enabledChanged, m_widget, &QWidget::setEnabled);
     findInFiles->addSearchEngine(this);
 }
 
@@ -249,11 +250,6 @@ QWidget *GitGrep::widget() const
     return m_widget;
 }
 
-bool GitGrep::isEnabled() const
-{
-    return m_widget->isEnabled();
-}
-
 QVariant GitGrep::parameters() const
 {
     GitGrepParameters params;

src/plugins/git/gitgrep.h

@@ -44,7 +44,6 @@ public:
     QString title() const override;
     QString toolTip() const override;
     QWidget *widget() const override;
-    bool isEnabled() const override;
     QVariant parameters() const override;
     void readSettings(QSettings *settings) override;
     void writeSettings(QSettings *settings) const override;

src/plugins/texteditor/basefilefind.cpp

@@ -66,7 +66,6 @@ public:
     QString title() const override { return tr("Internal"); }
     QString toolTip() const override { return QString(); }
     QWidget *widget() const override { return m_widget; }
-    bool isEnabled() const override { return true; }
     QVariant parameters() const override { return QVariant(); }
     void readSettings(QSettings * /*settings*/) override {}
     void writeSettings(QSettings * /*settings*/) const override {}
@@ -93,8 +92,13 @@ public:
 private:
     QWidget *m_widget;
 };
-}
+} // namespace
 
+class SearchEnginePrivate
+{
+public:
+    bool isEnabled = true;
+};
 
 class CountingLabel : public QLabel
 {
@@ -116,13 +120,36 @@ public:
     QPointer<QComboBox> m_filterCombo;
     QVector<SearchEngine *> m_searchEngines;
     SearchEngine *m_internalSearchEngine;
-    int m_currentSearchEngineIndex = 0;
+    int m_currentSearchEngineIndex = -1;
 };
 
 } // namespace Internal
 
 using namespace Internal;
 
+SearchEngine::SearchEngine()
+    : d(new SearchEnginePrivate)
+{
+}
+
+SearchEngine::~SearchEngine()
+{
+    delete d;
+}
+
+bool SearchEngine::isEnabled() const
+{
+    return d->isEnabled;
+}
+
+void SearchEngine::setEnabled(bool enabled)
+{
+    if (enabled == d->isEnabled)
+        return;
+    d->isEnabled = enabled;
+    emit enabledChanged(d->isEnabled);
+}
+
 BaseFileFind::BaseFileFind() : d(new BaseFileFindPrivate)
 {
     d->m_internalSearchEngine = new InternalEngine;
@@ -186,7 +213,10 @@ QVector<SearchEngine *> BaseFileFind::searchEngines() const
 
 void BaseFileFind::setCurrentSearchEngine(int index)
 {
+    if (d->m_currentSearchEngineIndex == index)
+        return;
     d->m_currentSearchEngineIndex = index;
+    emit currentSearchEngineChanged();
 }
 
 void BaseFileFind::runNewSearch(const QString &txt, FindFlags findFlags,

src/plugins/texteditor/basefilefind.h

@@ -48,7 +48,10 @@ class SearchResultItem;
 
 namespace TextEditor {
 
-namespace Internal { class BaseFileFindPrivate; }
+namespace Internal {
+class BaseFileFindPrivate;
+class SearchEnginePrivate;
+} // Internal
 
 class TEXTEDITOR_EXPORT FileFindParameters
 {
@@ -65,12 +68,13 @@ class BaseFileFind;
 
 class TEXTEDITOR_EXPORT SearchEngine : public QObject
 {
+    Q_OBJECT
 public:
-    virtual ~SearchEngine() {}
+    SearchEngine();
+    ~SearchEngine();
     virtual QString title() const = 0;
     virtual QString toolTip() const = 0; // add %1 placeholder where the find flags should be put
     virtual QWidget *widget() const = 0;
-    virtual bool isEnabled() const = 0;
     virtual QVariant parameters() const = 0;
     virtual void readSettings(QSettings *settings) = 0;
     virtual void writeSettings(QSettings *settings) const = 0;
@@ -78,7 +82,14 @@ public:
             const FileFindParameters &parameters, BaseFileFind *baseFileFind) = 0;
     virtual Core::IEditor *openEditor(const Core::SearchResultItem &item,
                                       const FileFindParameters &parameters) = 0;
+    bool isEnabled() const;
+    void setEnabled(bool enabled);
 
+signals:
+    void enabledChanged(bool enabled);
+
+private:
+    Internal::SearchEnginePrivate *d;
 };
 
 class TEXTEDITOR_EXPORT BaseFileFind : public Core::IFindFilter
@@ -122,6 +133,9 @@ protected:
     void setCurrentSearchEngine(int index);
     virtual void syncSearchEngineCombo(int /*selectedSearchEngineIndex*/) {}
 
+signals:
+    void currentSearchEngineChanged();
+
 private:
     void displayResult(int index);
     void searchFinished();

src/plugins/texteditor/findinfiles.cpp

@@ -66,7 +66,7 @@ FindInFiles::~FindInFiles()
 
 bool FindInFiles::isValid() const
 {
-    return m_directory->isValid() && currentSearchEngine()->isEnabled();
+    return m_isValid;
 }
 
 QString FindInFiles::id() const
@@ -124,7 +124,14 @@ void FindInFiles::syncSearchEngineCombo(int selectedSearchEngineIndex)
                && selectedSearchEngineIndex < searchEngines().size(), return);
 
     m_searchEngineCombo->setCurrentIndex(selectedSearchEngineIndex);
-    searchEnginesSelectionChanged(selectedSearchEngineIndex);
+}
+
+void FindInFiles::setValid(bool valid)
+{
+    if (valid == m_isValid)
+        return;
+    m_isValid = valid;
+    emit validChanged(m_isValid);
 }
 
 void FindInFiles::searchEnginesSelectionChanged(int index)
@@ -144,11 +151,9 @@ QWidget *FindInFiles::createConfigWidget()
         int row = 0;
         auto searchEngineLabel = new QLabel(tr("Search engine:"));
         gridLayout->addWidget(searchEngineLabel, row, 0, Qt::AlignRight);
-
         m_searchEngineCombo = new QComboBox;
         auto cc = static_cast<void (QComboBox::*)(int)>(&QComboBox::currentIndexChanged);
         connect(m_searchEngineCombo, cc, this, &FindInFiles::searchEnginesSelectionChanged);
-        connect(m_searchEngineCombo, cc, this, &FindInFiles::enabledChanged);
         searchEngineLabel->setBuddy(m_searchEngineCombo);
         gridLayout->addWidget(m_searchEngineCombo, row, 1);
 
@@ -165,10 +170,7 @@ QWidget *FindInFiles::createConfigWidget()
         m_directory->setExpectedKind(PathChooser::ExistingDirectory);
         m_directory->setPromptDialogTitle(tr("Directory to Search"));
         connect(m_directory.data(), &PathChooser::pathChanged,
-                this, [this](const QString &path) {
+                this, &FindInFiles::pathChanged);
-            emit FindInFiles::pathChanged(path);
-            emit FindInFiles::enabledChanged(isEnabled());
-        });
         m_directory->setHistoryCompleter(QLatin1String(HistoryKey),
                                          /*restoreLastItemFromHistory=*/ true);
         if (!HistoryCompleter::historyExistsFor(QLatin1String(HistoryKey))) {
@@ -190,6 +192,16 @@ QWidget *FindInFiles::createConfigWidget()
         gridLayout->addWidget(filePatternLabel, row, 0);
         gridLayout->addWidget(patternWidget, row++, 1, 1, 2);
         m_configWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Preferred);
+
+        // validity
+        auto updateValidity = [this]() {
+            setValid(currentSearchEngine()->isEnabled() && m_directory->isValid());
+        };
+        connect(this, &BaseFileFind::currentSearchEngineChanged, this, updateValidity);
+        foreach (SearchEngine *searchEngine, searchEngines())
+            connect(searchEngine, &SearchEngine::enabledChanged, this, updateValidity);
+        connect(m_directory.data(), &PathChooser::validChanged, this, updateValidity);
+        updateValidity();
     }
     return m_configWidget;
 }

src/plugins/texteditor/findinfiles.h

@@ -73,6 +73,7 @@ protected:
     void syncSearchEngineCombo(int selectedSearchEngineIndex) override;
 
 private:
+    void setValid(bool valid);
     void searchEnginesSelectionChanged(int index);
     Utils::FileName path() const;
 
@@ -80,6 +81,7 @@ private:
     QPointer<Utils::PathChooser> m_directory;
     QStackedWidget *m_searchEngineWidget = nullptr;
     QComboBox *m_searchEngineCombo = nullptr;
+    bool m_isValid = false;
 };
 
 } // namespace TextEditor