Core: simplify ILocator interface

Use data members instead of virtual functions for id, displayName and
priority, use Core::Id, not QStrings for id, de-pimpl CommandLocator.

Change-Id: Id8b41f184cb995138b2d76c923d6d3ae02b7e3f5
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@digia.com>
Reviewed-by: hjk <qthjk@ovi.com>

src/plugins/cmakeprojectmanager/cmakelocatorfilter.cpp

@@ -45,6 +45,8 @@ using namespace Utils;
 
 CMakeLocatorFilter::CMakeLocatorFilter()
 {
+    setId("Build CMake target");
+    setDisplayName(tr("Build CMake target"));
     setShortcutString(QLatin1String("cm"));
 
     ProjectExplorer::SessionManager *sm = ProjectExplorer::ProjectExplorerPlugin::instance()->session();

src/plugins/cmakeprojectmanager/cmakelocatorfilter.h

@@ -45,10 +45,6 @@ public:
     CMakeLocatorFilter();
     ~CMakeLocatorFilter();
 
-    QString displayName() const { return tr("Build CMake target"); }
-    QString id() const { return QLatin1String("Build CMake target"); }
-    Priority priority() const { return Medium; }
-
     QList<Locator::FilterEntry> matchesFor(QFutureInterface<Locator::FilterEntry> &future, const QString &entry);
     void accept(Locator::FilterEntry selection) const;
     void refresh(QFutureInterface<void> &future);

src/plugins/coreplugin/dialogs/settingsdialog.cpp

@@ -270,9 +270,9 @@ public:
 // Helpers to sort by category. id
 bool optionsPageLessThan(const IOptionsPage *p1, const IOptionsPage *p2)
 {
-    if (const int cc = p1->category().toString().compare(p2->category().toString()))
-        return cc < 0;
-    return p1->id().toString().compare(p2->id().toString()) < 0;
+    if (p1->category() != p2->category())
+        return p1->category().alphabeticallyBefore(p2->category());
+    return p1->id().alphabeticallyBefore(p2->id());
 }
 
 static inline QList<Core::IOptionsPage*> sortedOptionsPages()

src/plugins/coreplugin/id.cpp

@@ -323,4 +323,9 @@ CORE_EXPORT const char *nameForId(int id)
     return stringFromId.value(id).str;
 }
 
+bool Id::alphabeticallyBefore(Id other) const
+{
+    return toString().compare(other.toString(), Qt::CaseInsensitive) < 0;
+}
+
 } // namespace Core

src/plugins/coreplugin/id.h

@@ -64,6 +64,7 @@ public:
     bool operator!=(const char *name) const { return !operator==(name); }
     bool operator<(Id id) const { return m_id < id.m_id; }
     bool operator>(Id id) const { return m_id > id.m_id; }
+    bool alphabeticallyBefore(Id other) const;
     int uniqueIdentifier() const { return m_id; }
     static Id fromUniqueIdentifier(int uid) { return Id(uid); }
     static Id fromString(const QString &str); // FIXME: avoid.

src/plugins/cpptools/cppclassesfilter.cpp

@@ -35,8 +35,10 @@ using namespace CppTools::Internal;
 CppClassesFilter::CppClassesFilter(CppModelManager *manager)
     : CppLocatorFilter(manager)
 {
+    setId("Classes");
     setShortcutString(QLatin1String("c"));
     setIncludedByDefault(false);
+    setDisplayName(tr("C++ Classes"));
 
     search.setSymbolsToSearchFor(SymbolSearcher::Classes);
     search.setSeparateScope(true);

src/plugins/cpptools/cppclassesfilter.h

@@ -42,10 +42,6 @@ class CPPTOOLS_EXPORT CppClassesFilter : public Internal::CppLocatorFilter
 public:
     CppClassesFilter(Internal::CppModelManager *manager);
     ~CppClassesFilter();
-
-    QString displayName() const { return tr("C++ Classes"); }
-    QString id() const { return QLatin1String("Classes"); }
-    Priority priority() const { return Medium; }
 };
 
 } // namespace CppTools

src/plugins/cpptools/cppcurrentdocumentfilter.cpp

@@ -40,6 +40,8 @@ using namespace CPlusPlus;
 CppCurrentDocumentFilter::CppCurrentDocumentFilter(CppModelManager *manager, Core::EditorManager *editorManager)
     : m_modelManager(manager)
 {
+    setId("Methods in current Document");
+    setDisplayName(tr("C++ Methods in Current Document"));
     setShortcutString(QString(QLatin1Char('.')));
     setIncludedByDefault(false);
 

src/plugins/cpptools/cppcurrentdocumentfilter.h

@@ -50,9 +50,6 @@ public:
     CppCurrentDocumentFilter(CppModelManager *manager, Core::EditorManager *editorManager);
     ~CppCurrentDocumentFilter() {}
 
-    QString displayName() const { return tr("C++ Methods in Current Document"); }
-    QString id() const { return QLatin1String("Methods in current Document"); }
-    Priority priority() const { return Medium; }
     QList<Locator::FilterEntry> matchesFor(QFutureInterface<Locator::FilterEntry> &future, const QString &entry);
     void accept(Locator::FilterEntry selection) const;
     void refresh(QFutureInterface<void> &future);

src/plugins/cpptools/cppfunctionsfilter.cpp

@@ -34,6 +34,8 @@ using namespace CppTools::Internal;
 CppFunctionsFilter::CppFunctionsFilter(CppModelManager *manager)
     : CppLocatorFilter(manager)
 {
+    setId("Methods");
+    setDisplayName(tr("C++ Methods and Functions"));
     setShortcutString(QString(QLatin1Char('m')));
     setIncludedByDefault(false);
 

src/plugins/cpptools/cppfunctionsfilter.h

@@ -42,10 +42,6 @@ class CppFunctionsFilter : public CppLocatorFilter
 public:
     CppFunctionsFilter(CppModelManager *manager);
     ~CppFunctionsFilter();
-
-    QString displayName() const { return tr("C++ Methods and Functions"); }
-    QString id() const { return QLatin1String("Methods"); }
-    Priority priority() const { return Medium; }
 };
 
 } // namespace Internal

src/plugins/cpptools/cpplocatorfilter.cpp

@@ -43,6 +43,8 @@ CppLocatorFilter::CppLocatorFilter(CppModelManager *manager)
     : m_manager(manager),
     m_forceNewSearchList(true)
 {
+    setId("Classes and Methods");
+    setDisplayName(tr("C++ Classes and Methods"));
     setShortcutString(QString(QLatin1Char(':')));
     setIncludedByDefault(false);
 

src/plugins/cpptools/cpplocatorfilter.h

@@ -41,13 +41,11 @@ class CppModelManager;
 class CppLocatorFilter : public Locator::ILocatorFilter
 {
     Q_OBJECT
+
 public:
     CppLocatorFilter(CppModelManager *manager);
     ~CppLocatorFilter();
 
-    QString displayName() const { return tr("C++ Classes and Methods"); }
-    QString id() const { return QLatin1String("Classes and Methods"); }
-    Priority priority() const { return Medium; }
     QList<Locator::FilterEntry> matchesFor(QFutureInterface<Locator::FilterEntry> &future, const QString &entry);
     void accept(Locator::FilterEntry selection) const;
     void refresh(QFutureInterface<void> &future);

src/plugins/help/helpindexfilter.cpp

@@ -46,8 +46,11 @@ Q_DECLARE_METATYPE(ILocatorFilter*)
 
 HelpIndexFilter::HelpIndexFilter()
 {
+    setId("HelpIndexFilter");
+    setDisplayName(tr("Help Index"));
     setIncludedByDefault(false);
     setShortcutString(QString(QLatin1Char('?')));
+
     m_icon = QIcon(QLatin1String(":/help/images/bookmark.png"));
 }
 
@@ -55,21 +58,6 @@ HelpIndexFilter::~HelpIndexFilter()
 {
 }
 
-QString HelpIndexFilter::displayName() const
-{
-    return tr("Help Index");
-}
-
-QString HelpIndexFilter::id() const
-{
-    return QLatin1String("HelpIndexFilter");
-}
-
-ILocatorFilter::Priority HelpIndexFilter::priority() const
-{
-    return Medium;
-}
-
 QList<FilterEntry> HelpIndexFilter::matchesFor(QFutureInterface<Locator::FilterEntry> &future, const QString &entry)
 {
     QStringList keywords;

src/plugins/help/helpindexfilter.h

@@ -46,9 +46,6 @@ public:
     ~HelpIndexFilter();
 
     // ILocatorFilter
-    QString displayName() const;
-    QString id() const;
-    Priority priority() const;
     QList<Locator::FilterEntry> matchesFor(QFutureInterface<Locator::FilterEntry> &future, const QString &entry);
     void accept(Locator::FilterEntry selection) const;
     void refresh(QFutureInterface<void> &future);

src/plugins/help/remotehelpfilter.cpp

@@ -80,8 +80,11 @@ void RemoteFilterOptions::updateRemoveButton()
 
 RemoteHelpFilter::RemoteHelpFilter()
 {
+    setId("RemoteHelpFilter");
+    setDisplayName(tr("Web Search"));
     setIncludedByDefault(false);
     setShortcutString(QLatin1String("r"));
+
     m_remoteUrls.append(QLatin1String("http://www.bing.com/search?q=%1"));
     m_remoteUrls.append(QLatin1String("http://www.google.com/search?q=%1"));
     m_remoteUrls.append(QLatin1String("http://search.yahoo.com/search?p=%1"));
@@ -93,21 +96,6 @@ RemoteHelpFilter::~RemoteHelpFilter()
 {
 }
 
-QString RemoteHelpFilter::displayName() const
-{
-    return tr("Web Search");
-}
-
-QString RemoteHelpFilter::id() const
-{
-    return QLatin1String("RemoteHelpFilter");
-}
-
-Locator::ILocatorFilter::Priority RemoteHelpFilter::priority() const
-{
-    return Medium;
-}
-
 QList<Locator::FilterEntry> RemoteHelpFilter::matchesFor(QFutureInterface<Locator::FilterEntry> &future, const QString &pattern)
 {
     QList<Locator::FilterEntry> entries;

src/plugins/help/remotehelpfilter.h

@@ -47,9 +47,6 @@ public:
     ~RemoteHelpFilter();
 
     // ILocatorFilter
-    QString displayName() const;
-    QString id() const;
-    Priority priority() const;
     QList<Locator::FilterEntry> matchesFor(QFutureInterface<Locator::FilterEntry> &future, const QString &entry);
     void accept(Locator::FilterEntry selection) const;
     void refresh(QFutureInterface<void> &future);

src/plugins/locator/commandlocator.cpp

@@ -31,6 +31,7 @@
 
 #include <coreplugin/actionmanager/command.h>
 #include <coreplugin/icore.h>
+#include <coreplugin/id.h>
 
 #include <utils/qtcassert.h>
 
@@ -38,14 +39,8 @@
 
 namespace Locator {
 
-struct CommandLocatorPrivate {
-    CommandLocatorPrivate(const QString &prefix,
-                          const QString &displayName) :
-        m_prefix(prefix), m_displayName(displayName) {}
-
-    const QString m_prefix;
-    const QString m_displayName;
-
+struct CommandLocatorPrivate
+{
     QList<Core::Command *> commands;
 };
 
@@ -54,8 +49,10 @@ CommandLocator::CommandLocator(const QString &prefix,
                                const QString &shortCutString,
                                QObject *parent) :
     Locator::ILocatorFilter(parent),
-    d(new CommandLocatorPrivate(prefix, displayName))
+    d(new CommandLocatorPrivate)
 {
+    setId(Core::Id::fromString(prefix));
+    setDisplayName(displayName);
     setShortcutString(shortCutString);
 }
 
@@ -69,21 +66,6 @@ void CommandLocator::appendCommand(Core::Command *cmd)
     d->commands.push_back(cmd);
 }
 
-QString CommandLocator::displayName() const
-{
-    return d->m_displayName;
-}
-
-QString CommandLocator::id() const
-{
-    return d->m_prefix;
-}
-
-ILocatorFilter::Priority CommandLocator::priority() const
-{
-    return Medium;
-}
-
 QList<Locator::FilterEntry> CommandLocator::matchesFor(QFutureInterface<Locator::FilterEntry> &future, const QString &entry)
 {
     QList<FilterEntry> filters;

src/plugins/locator/commandlocator.h

@@ -32,15 +32,8 @@
 
 #include "locator_global.h"
 #include "ilocatorfilter.h"
-#include <QList>
 
-QT_BEGIN_NAMESPACE
-class QAction;
-QT_END_NAMESPACE
-
-namespace Core {
-    class Command;
-}
+namespace Core { class Command; }
 
 namespace Locator {
 /* Command locators: Provides completion for a set of
@@ -57,27 +50,18 @@ public:
                             const QString &displayName,
                             const QString &shortCutString,
                             QObject *parent = 0);
-    virtual ~CommandLocator();
+    ~CommandLocator();
 
     void appendCommand(Core::Command *cmd);
 
-    virtual QString displayName() const;
-    virtual QString id() const;
-    virtual Priority priority() const;
-    virtual QList<FilterEntry> matchesFor(QFutureInterface<Locator::FilterEntry> &future, const QString &entry);
-    virtual void accept(FilterEntry selection) const;
-    virtual void refresh(QFutureInterface<void> &future);
+    QList<FilterEntry> matchesFor(QFutureInterface<Locator::FilterEntry> &future, const QString &entry);
+    void accept(FilterEntry selection) const;
+    void refresh(QFutureInterface<void> &future);
 
 private:
     CommandLocatorPrivate *d;
 };
 
-inline CommandLocator &operator<<(CommandLocator &locator, Core::Command *cmd)
-{
-    locator.appendCommand(cmd);
-    return locator;
-}
-
 } // namespace Locator
 
 #endif // COMMANDLOCATOR_H

src/plugins/locator/directoryfilter.cpp

@@ -43,11 +43,16 @@ using namespace Locator::Internal;
 
 DirectoryFilter::DirectoryFilter()
   : m_name(tr("Generic Directory Filter")),
-    m_filters(QStringList() << QLatin1String("*.h") << QLatin1String("*.cpp")
-                            << QLatin1String("*.ui") << QLatin1String("*.qrc")),
     m_dialog(0)
 {
+    setId(Core::Id::fromString(m_name));
     setIncludedByDefault(true);
+    setDisplayName(m_name);
+
+    m_filters.append(QLatin1String("*.h"));
+    m_filters.append(QLatin1String("*.cpp"));
+    m_filters.append(QLatin1String("*.ui"));
+    m_filters.append(QLatin1String("*.qrc"));
 }
 
 QByteArray DirectoryFilter::saveState() const

src/plugins/locator/directoryfilter.h

@@ -48,9 +48,6 @@ class DirectoryFilter : public BaseFileFilter
 
 public:
     DirectoryFilter();
-    QString displayName() const { return m_name; }
-    QString id() const { return m_name; }
-    Locator::ILocatorFilter::Priority priority() const { return Locator::ILocatorFilter::Medium; }
     QByteArray saveState() const;
     bool restoreState(const QByteArray &state);
     bool openConfigDialog(QWidget *parent, bool &needsRefresh);

src/plugins/locator/executefilter.cpp

@@ -41,6 +41,8 @@ using namespace Locator::Internal;
 
 ExecuteFilter::ExecuteFilter()
 {
+    setId("Execute custom commands");
+    setDisplayName(tr("Execute Custom Commands"));
     setShortcutString(QString(QLatin1Char('!')));
     setIncludedByDefault(false);
 

src/plugins/locator/executefilter.h

@@ -53,9 +53,6 @@ class ExecuteFilter : public Locator::ILocatorFilter
 
 public:
     ExecuteFilter();
-    QString displayName() const { return tr("Execute Custom Commands"); }
-    QString id() const { return QLatin1String("Execute custom commands"); }
-    Locator::ILocatorFilter::Priority priority() const { return Locator::ILocatorFilter::Medium; }
     QList<Locator::FilterEntry> matchesFor(QFutureInterface<Locator::FilterEntry> &future,
                                            const QString &entry);
     void accept(Locator::FilterEntry selection) const;

src/plugins/locator/filesystemfilter.cpp

@@ -41,6 +41,8 @@ using namespace Locator::Internal;
 FileSystemFilter::FileSystemFilter(EditorManager *editorManager, LocatorWidget *locatorWidget)
         : m_editorManager(editorManager), m_locatorWidget(locatorWidget), m_includeHidden(true)
 {
+    setId("Files in file system");
+    setDisplayName(tr("Files in File System"));
     setShortcutString(QString(QLatin1Char('f')));
     setIncludedByDefault(false);
 }

src/plugins/locator/filesystemfilter.h

@@ -38,9 +38,7 @@
 #include <QByteArray>
 #include <QFutureInterface>
 
-namespace Core {
-    class EditorManager;
-}
+namespace Core { class EditorManager; }
 
 namespace Locator {
 namespace Internal {
@@ -53,9 +51,6 @@ class FileSystemFilter : public Locator::ILocatorFilter
 
 public:
     FileSystemFilter(Core::EditorManager *editorManager, LocatorWidget *locatorWidget);
-    QString displayName() const { return tr("Files in File System"); }
-    QString id() const { return QLatin1String("Files in file system"); }
-    Locator::ILocatorFilter::Priority priority() const { return Locator::ILocatorFilter::Medium; }
     QList<Locator::FilterEntry> matchesFor(QFutureInterface<Locator::FilterEntry> &future, const QString &entry);
     void accept(Locator::FilterEntry selection) const;
     QByteArray saveState() const;

src/plugins/locator/ilocatorfilter.cpp

@@ -40,9 +40,11 @@ using namespace Locator;
 
 ILocatorFilter::ILocatorFilter(QObject *parent):
     QObject(parent),
+    m_priority(Medium),
     m_includedByDefault(false),
     m_hidden(false),
-    m_enabled(true)
+    m_enabled(true),
+    m_isConfigurable(true)
 {
 }
 
@@ -114,9 +116,25 @@ bool ILocatorFilter::openConfigDialog(QWidget *parent, bool &needsRefresh)
     return false;
 }
 
+QString ILocatorFilter::trimWildcards(const QString &str)
+{
+    if (str.isEmpty())
+        return str;
+    int first = 0, last = str.size()-1;
+    const QChar asterisk = QLatin1Char('*');
+    const QChar question = QLatin1Char('?');
+    while (first < str.size() && (str.at(first) == asterisk || str.at(first) == question))
+        ++first;
+    while (last >= 0 && (str.at(last) == asterisk || str.at(last) == question))
+        --last;
+    if (first > last)
+        return QString();
+    return str.mid(first, last-first+1);
+}
+
 bool ILocatorFilter::isConfigurable() const
 {
-    return true;
+    return m_isConfigurable;
 }
 
 bool ILocatorFilter::isIncludedByDefault() const
@@ -144,7 +162,42 @@ bool ILocatorFilter::isEnabled() const
     return m_enabled;
 }
 
+Core::Id ILocatorFilter::id() const
+{
+    return m_id;
+}
+
+QString ILocatorFilter::displayName() const
+{
+    return m_displayName;
+}
+
+ILocatorFilter::Priority ILocatorFilter::priority() const
+{
+    return m_priority;
+}
+
 void ILocatorFilter::setEnabled(bool enabled)
 {
     m_enabled = enabled;
 }
+
+void ILocatorFilter::setId(Core::Id id)
+{
+    m_id = id;
+}
+
+void ILocatorFilter::setPriority(Priority priority)
+{
+    m_priority = priority;
+}
+
+void ILocatorFilter::setDisplayName(const QString &displayString)
+{
+    m_displayName = displayString;
+}
+
+void ILocatorFilter::setConfigurable(bool configurable)
+{
+    m_isConfigurable = configurable;
+}

src/plugins/locator/ilocatorfilter.h

@@ -32,6 +32,8 @@
 
 #include "locator_global.h"
 
+#include <coreplugin/id.h>
+
 #include <QVariant>
 #include <QFutureInterface>
 #include <QIcon>
@@ -88,14 +90,14 @@ public:
     ILocatorFilter(QObject *parent = 0);
     virtual ~ILocatorFilter() {}
 
-    /* Visible name. */
-    virtual QString displayName() const = 0;
+    /* Internal Id. */
+    Core::Id id() const;
 
-    /* Internal name. */
-    virtual QString id() const = 0;
+    /* Visible name. */
+    QString displayName() const;
 
     /* Selection list order in case of multiple active filters (high goes on top). */
-    virtual Priority priority() const = 0;
+    Priority priority() const;
 
     /* String to type to use this filter exclusively. */
     QString shortcutString() const;
@@ -124,7 +126,7 @@ public:
 
     /* If there is a configure dialog available for this filter. The default
      * implementation returns true. */
-    virtual bool isConfigurable() const;
+    bool isConfigurable() const;
 
     /* Is this filter used also when the shortcutString is not used? */
     bool isIncludedByDefault() const;
@@ -135,20 +137,7 @@ public:
     /* Returns whether the filter should be enabled and used in menus. */
     bool isEnabled() const;
 
-    static QString trimWildcards(const QString &str) {
-        if (str.isEmpty())
-            return str;
-        int first = 0, last = str.size()-1;
-        const QChar asterisk = QLatin1Char('*');
-        const QChar question = QLatin1Char('?');
-        while (first < str.size() && (str.at(first) == asterisk || str.at(first) == question))
-            ++first;
-        while (last >= 0 && (str.at(last) == asterisk || str.at(last) == question))
-            --last;
-        if (first > last)
-            return QString();
-        return str.mid(first, last-first+1);
-    }
+    static QString trimWildcards(const QString &str);
 
 public slots:
     /* Enable or disable the filter. */
@@ -158,12 +147,20 @@ protected:
     void setShortcutString(const QString &shortcut);
     void setIncludedByDefault(bool includedByDefault);
     void setHidden(bool hidden);
+    void setId(Core::Id id);
+    void setPriority(Priority priority);
+    void setDisplayName(const QString &displayString);
+    void setConfigurable(bool configurable);
 
 private:
+    Core::Id m_id;
     QString m_shortcut;
+    Priority m_priority;
+    QString m_displayName;
     bool m_includedByDefault;
     bool m_hidden;
     bool m_enabled;
+    bool m_isConfigurable;
 };
 
 } // namespace Locator

src/plugins/locator/locatorfiltersfilter.cpp

@@ -44,23 +44,12 @@ LocatorFiltersFilter::LocatorFiltersFilter(LocatorPlugin *plugin,
     m_locatorWidget(locatorWidget),
     m_icon(QIcon(QLatin1String(Core::Constants::ICON_NEXT)))
 {
+    setId("FiltersFilter");
+    setDisplayName(tr("Available filters"));
     setIncludedByDefault(true);
     setHidden(true);
-}
-
-QString LocatorFiltersFilter::displayName() const
-{
-    return tr("Available filters");
-}
-
-QString LocatorFiltersFilter::id() const
-{
-    return QLatin1String("FiltersFilter");
-}
-
-ILocatorFilter::Priority LocatorFiltersFilter::priority() const
-{
-    return High;
+    setPriority(High);
+    setConfigurable(false);
 }
 
 QList<FilterEntry> LocatorFiltersFilter::matchesFor(QFutureInterface<Locator::FilterEntry> &future, const QString &entry)
@@ -104,8 +93,3 @@ void LocatorFiltersFilter::refresh(QFutureInterface<void> &future)
     Q_UNUSED(future)
     // Nothing to refresh