Generic highlighter: Fix matching the right definition

No longer uses artificial mime types for highlighting files that do not
specify a mime type. No longer registers mime types that are specified
but that Qt Creator does not know about.

Instead, try to match the mime type, and if that fails, or if the result
does not match the file name pattern, try to match file name patterns
instead.

This also fixes the potential problem that mime types were always only
added, never removed, even if the user removed definitions and triggered
a reparse.

Also fixes that a highlight definition in the fallback location could
overwrite a highlight definition in the preferred location, if it has a
higher priority setting.

Task-number: QTCREATORBUG-13912
Change-Id: I86ce10f4f4341f6add0d2b58a04f080501d0cbf4
Reviewed-by: Eike Ziller <eike.ziller@theqtcompany.com>

src/plugins/texteditor/generichighlighter/highlightersettingspage.cpp

@@ -77,7 +77,7 @@ void HighlighterSettingsPage::HighlighterSettingsPagePrivate::ensureInitialized(
 
 HighlighterSettingsPage::HighlighterSettingsPage(Core::Id id, QObject *parent) :
     TextEditorOptionsPage(parent),
-    m_requestMimeTypeRegistration(false),
+    m_requestHighlightFileRegistration(false),
     m_d(new HighlighterSettingsPagePrivate(id))
 {
     setId(m_d->m_id);
@@ -122,9 +122,9 @@ void HighlighterSettingsPage::apply()
     if (settingsChanged())
         settingsFromUI();
 
-    if (m_requestMimeTypeRegistration) {
+    if (m_requestHighlightFileRegistration) {
-        Manager::instance()->registerMimeTypes();
+        Manager::instance()->registerHighlightingFiles();
-        m_requestMimeTypeRegistration = false;
+        m_requestHighlightFileRegistration = false;
     }
 }
 
@@ -146,12 +146,12 @@ const HighlighterSettings &HighlighterSettingsPage::highlighterSettings() const
 void HighlighterSettingsPage::settingsFromUI()
 {
     m_d->ensureInitialized();
-    if (!m_requestMimeTypeRegistration && (
+    if (!m_requestHighlightFileRegistration && (
         m_d->m_settings.definitionFilesPath() != m_d->m_page->definitionFilesPath->path() ||
         m_d->m_settings.fallbackDefinitionFilesPath() !=
             m_d->m_page->fallbackDefinitionFilesPath->path() ||
         m_d->m_settings.useFallbackLocation() != m_d->m_page->useFallbackLocation->isChecked())) {
-        m_requestMimeTypeRegistration = true;
+        m_requestHighlightFileRegistration = true;
     }
 
     m_d->m_settings.setDefinitionFilesPath(m_d->m_page->definitionFilesPath->path());
@@ -212,8 +212,8 @@ void HighlighterSettingsPage::manageDefinitions(const QList<DefinitionMetaDataPt
     ManageDefinitionsDialog dialog(metaData,
                                    m_d->m_page->definitionFilesPath->path() + QLatin1Char('/'),
                                    m_d->m_page->definitionFilesPath->buttonAtIndex(1)->window());
-    if (dialog.exec() && !m_requestMimeTypeRegistration)
+    if (dialog.exec() && !m_requestHighlightFileRegistration)
-        m_requestMimeTypeRegistration = true;
+        m_requestHighlightFileRegistration = true;
     setDownloadDefinitionsState(m_d->m_page->definitionFilesPath->isValid());
 }
 

src/plugins/texteditor/generichighlighter/highlightersettingspage.h

@@ -71,7 +71,7 @@ private:
 
     bool settingsChanged() const;
 
-    bool m_requestMimeTypeRegistration;
+    bool m_requestHighlightFileRegistration;
 
     struct HighlighterSettingsPagePrivate;
     HighlighterSettingsPagePrivate *m_d;

src/plugins/texteditor/generichighlighter/manager.cpp

@@ -115,7 +115,7 @@ Manager::Manager() :
     m_multiDownloader(0),
     m_hasQueuedRegistration(false)
 {
-    connect(&m_registeringWatcher, SIGNAL(finished()), this, SLOT(registerMimeTypesFinished()));
+    connect(&m_registeringWatcher, SIGNAL(finished()), this, SLOT(registerHighlightingFilesFinished()));
 }
 
 Manager::~Manager()
@@ -138,20 +138,68 @@ QString Manager::definitionIdByName(const QString &name) const
     return m_register.m_idByName.value(name);
 }
 
-QString Manager::definitionIdByMimeType(const QString &mimeType) const
+static bool matchesPattern(const QString &fileName, DefinitionMetaDataPtr metaData)
 {
-    return m_register.m_idByMimeType.value(mimeType);
+    if (metaData.isNull())
+        return false;
+    foreach (const QString &pattern, metaData->patterns) {
+        QRegExp reg(pattern, Qt::CaseSensitive, QRegExp::Wildcard);
+        if (reg.exactMatch(fileName))
+            return true;
+    }
+    return false;
 }
 
-QString Manager::definitionIdByAnyMimeType(const QStringList &mimeTypes) const
+QString Manager::definitionIdByMimeType(const Core::MimeType &mimeType) const
 {
-    QString definitionId;
+    QList<Core::MimeType> queue;
-    foreach (const QString &mimeType, mimeTypes) {
+    queue.append(mimeType);
-        definitionId = definitionIdByMimeType(mimeType);
+    while (!queue.isEmpty()) {
-        if (!definitionId.isEmpty())
+        const Core::MimeType mt = queue.takeFirst();
-            break;
+        const QString id = m_register.m_idByMimeType.value(mt.type());
+        if (!id.isEmpty())
+            return id;
+        foreach (const QString &parent, mt.subClassesOf()) {
+            const Core::MimeType parentMt = Core::MimeDatabase::findByType(parent);
+            if (!parentMt.isNull())
+                queue.append(parentMt);
         }
-    return definitionId;
+    }
+    return QString();
+}
+
+QString Manager::definitionIdByFile(const QString &filePath) const
+{
+    const QString fileName = QFileInfo(filePath).fileName();
+    // find best match
+    QString bestId;
+    int bestPriority = -1;
+    auto it = m_register.m_definitionsMetaData.constBegin();
+    while (it != m_register.m_definitionsMetaData.constEnd()) {
+        DefinitionMetaDataPtr metaData = it.value();
+        if (metaData->priority > bestPriority && matchesPattern(fileName, metaData)) {
+            bestId = metaData->id;
+            bestPriority = metaData->priority;
+        }
+        ++it;
+    }
+    return bestId;
+}
+
+QString Manager::definitionIdByMimeTypeAndFile(const MimeType &mimeType, const QString &filePath) const
+{
+    QString id = definitionIdByMimeType(mimeType);
+    if (!filePath.isEmpty()) {
+        QString idByFile;
+        const QString fileName = QFileInfo(filePath).fileName();
+        // if mime type check returned no result, or doesn't match the patterns,
+        // prefer a match by pattern
+        if (id.isEmpty() || !matchesPattern(fileName, m_register.m_definitionsMetaData.value(id)))
+            idByFile = definitionIdByFile(filePath);
+        if (!idByFile.isEmpty())
+            id = idByFile;
+    }
+    return id;
 }
 
 DefinitionMetaDataPtr Manager::availableDefinitionByName(const QString &name) const
@@ -206,51 +254,28 @@ class ManagerProcessor : public QObject
     Q_OBJECT
 public:
     ManagerProcessor();
-    void process(QFutureInterface<QPair<Manager::RegisterData,
+    void process(QFutureInterface<Manager::RegisterData> &future);
-                                        QList<MimeType> > > &future);
 
     QStringList m_definitionsPaths;
-    QSet<QString> m_knownMimeTypes;
-    QSet<QString> m_knownSuffixes;
-    QHash<QString, MimeType> m_userModified;
     static const int kMaxProgress;
 };
 
 const int ManagerProcessor::kMaxProgress = 200;
 
 ManagerProcessor::ManagerProcessor()
-    : m_knownSuffixes(QSet<QString>::fromList(MimeDatabase::suffixes()))
 {
     const HighlighterSettings &settings = TextEditorSettings::highlighterSettings();
     m_definitionsPaths.append(settings.definitionFilesPath());
     if (settings.useFallbackLocation())
         m_definitionsPaths.append(settings.fallbackDefinitionFilesPath());
-
-    foreach (const MimeType &userMimeType, MimeDatabase::readUserModifiedMimeTypes())
-        m_userModified.insert(userMimeType.type(), userMimeType);
-    foreach (const MimeType &mimeType, MimeDatabase::mimeTypes())
-        m_knownMimeTypes.insert(mimeType.type());
 }
 
-void ManagerProcessor::process(QFutureInterface<QPair<Manager::RegisterData,
+void ManagerProcessor::process(QFutureInterface<Manager::RegisterData> &future)
-                                                      QList<MimeType> > > &future)
 {
     future.setProgressRange(0, kMaxProgress);
 
-    // @TODO: Improve MIME database to handle the following limitation.
-    // The generic highlighter only register its types after all other plugins
-    // have populated Creator's MIME database (so it does not override anything).
-    // When the generic highlighter settings change only its internal data is cleaned-up
-    // and rebuilt. Creator's MIME database is not touched. So depending on how the
-    // user plays around with the generic highlighter file definitions (changing
-    // duplicated patterns, for example), some changes might not be reflected.
-    // A definitive implementation would require some kind of re-load or update
-    // (considering hierarchies, aliases, etc) of the MIME database whenever there
-    // is a change in the generic highlighter settings.
-
     Manager::RegisterData data;
-    QList<MimeType> newMimeTypes;
+    // iterate through paths in order, high priority > low priority
-
     foreach (const QString &path, m_definitionsPaths) {
         if (path.isEmpty())
             continue;
@@ -258,93 +283,47 @@ void ManagerProcessor::process(QFutureInterface<QPair<Manager::RegisterData,
         QDir definitionsDir(path);
         QStringList filter(QLatin1String("*.xml"));
         definitionsDir.setNameFilters(filter);
-        QList<DefinitionMetaDataPtr> allMetaData;
         foreach (const QFileInfo &fileInfo, definitionsDir.entryInfoList()) {
-            const DefinitionMetaDataPtr &metaData =
-                    Manager::parseMetadata(fileInfo);
-            if (!metaData.isNull())
-                allMetaData.append(metaData);
-        }
-
-        // Consider definitions with higher priority first.
-        Utils::sort(allMetaData, [](const DefinitionMetaDataPtr &l,
-                                    const DefinitionMetaDataPtr &r) {
-            return l->priority > r->priority;
-        });
-
-        foreach (const DefinitionMetaDataPtr &metaData, allMetaData) {
             if (future.isCanceled())
                 return;
             if (future.progressValue() < kMaxProgress - 1)
                 future.setProgressValue(future.progressValue() + 1);
 
-            if (data.m_idByName.contains(metaData->name))
+            const DefinitionMetaDataPtr &metaData =
-                // Name already exists... This is a fallback item, do not consider it.
+                    Manager::parseMetadata(fileInfo);
-                continue;
+            // skip failing or already existing definitions
-
+            if (!metaData.isNull() && !data.m_idByName.contains(metaData->name)) {
-            const QString &id = metaData->id;
+                const QString id = metaData->id;
                 data.m_idByName.insert(metaData->name, id);
                 data.m_definitionsMetaData.insert(id, metaData);
-
+                foreach (const QString &mt, metaData->mimeTypes) {
-            static const QStringList textPlain(QLatin1String("text/plain"));
+                    bool insert = true;
-
+                    // check if there is already a definition registered with higher priority
-            // A definition can specify multiple MIME types and file extensions/patterns,
+                    const QString existingDefinition = data.m_idByMimeType.value(mt);
-            // but all on a single string. So associate all patterns with all MIME types.
+                    if (!existingDefinition.isEmpty()) {
-            QList<MimeGlobPattern> globPatterns;
+                        // check priorities
-            foreach (const QString &type, metaData->mimeTypes) {
+                        DefinitionMetaDataPtr existingMetaData =
-                if (data.m_idByMimeType.contains(type))
+                                data.m_definitionsMetaData.value(existingDefinition);
-                    continue;
+                        if (!existingMetaData.isNull() && existingMetaData->priority > metaData->priority)
-
+                            insert = false;
-                data.m_idByMimeType.insert(type, id);
-                if (!m_knownMimeTypes.contains(type)) {
-                    m_knownMimeTypes.insert(type);
-
-                    MimeType mimeType;
-                    mimeType.setType(type);
-                    mimeType.setSubClassesOf(textPlain);
-                    mimeType.setComment(metaData->name);
-
-                    // If there's a user modification for this mime type, we want to use the
-                    // modified patterns and rule-based matchers. If not, just consider what
-                    // is specified in the definition file.
-                    QHash<QString, MimeType>::const_iterator it =
-                        m_userModified.find(mimeType.type());
-                    if (it == m_userModified.end()) {
-                        if (globPatterns.isEmpty()) {
-                            foreach (const QString &pattern, metaData->patterns) {
-                                static const QLatin1String mark("*.");
-                                if (pattern.startsWith(mark)) {
-                                    const QString &suffix = pattern.right(pattern.length() - 2);
-                                    if (!m_knownSuffixes.contains(suffix))
-                                        m_knownSuffixes.insert(suffix);
-                                    else
-                                        continue;
                     }
-                                globPatterns.append(MimeGlobPattern(pattern, 50));
+                    if (insert)
-                            }
+                        data.m_idByMimeType.insert(mt, id);
-                        }
-                        mimeType.setGlobPatterns(globPatterns);
-                    } else {
-                        mimeType.setGlobPatterns(it.value().globPatterns());
-                        mimeType.setMagicRuleMatchers(it.value().magicRuleMatchers());
-                    }
-
-                    newMimeTypes.append(mimeType);
                 }
             }
         }
     }
 
-    future.reportResult(qMakePair(data, newMimeTypes));
+    future.reportResult(data);
 }
 
-void Manager::registerMimeTypes()
+void Manager::registerHighlightingFiles()
 {
     if (!m_registeringWatcher.isRunning()) {
         clear();
 
         ManagerProcessor *processor = new ManagerProcessor;
-        QFuture<QPair<RegisterData, QList<MimeType> > > future =
+        QFuture<RegisterData> future =
             QtConcurrent::run(&ManagerProcessor::process, processor);
         connect(&m_registeringWatcher, SIGNAL(finished()), processor, SLOT(deleteLater()));
         m_registeringWatcher.setFuture(future);
@@ -354,29 +333,22 @@ void Manager::registerMimeTypes()
     }
 }
 
-void Manager::registerMimeTypesFinished()
+void Manager::registerHighlightingFilesFinished()
 {
     if (m_hasQueuedRegistration) {
         m_hasQueuedRegistration = false;
-        registerMimeTypes();
+        registerHighlightingFiles();
     } else if (!m_registeringWatcher.isCanceled()) {
-        const QPair<RegisterData, QList<MimeType> > &result = m_registeringWatcher.result();
+        m_register = m_registeringWatcher.result();
-        m_register = result.first;
 
-        foreach (const MimeType &mimeType, result.second)
+        emit highlightingFilesRegistered();
-            MimeDatabase::addMimeType(mimeType);
-
-        emit mimeTypesRegistered();
     }
 }
 
 DefinitionMetaDataPtr Manager::parseMetadata(const QFileInfo &fileInfo)
 {
     static const QLatin1Char kSemiColon(';');
-    static const QLatin1Char kSpace(' ');
-    static const QLatin1Char kDash('-');
     static const QLatin1String kLanguage("language");
-    static const QLatin1String kArtificial("text/x-artificial-");
 
     QFile definitionFile(fileInfo.absoluteFilePath());
     if (!definitionFile.open(QIODevice::ReadOnly | QIODevice::Text))
@@ -397,17 +369,8 @@ DefinitionMetaDataPtr Manager::parseMetadata(const QFileInfo &fileInfo)
             metaData->patterns = atts.value(QLatin1String(kExtensions))
                                   .toString().split(kSemiColon, QString::SkipEmptyParts);
 
-            QStringList mimeTypes = atts.value(QLatin1String(kMimeType)).
+            metaData->mimeTypes = atts.value(QLatin1String(kMimeType)).
                                     toString().split(kSemiColon, QString::SkipEmptyParts);
-            if (mimeTypes.isEmpty()) {
-                // There are definitions which do not specify a MIME type, but specify file
-                // patterns. Creating an artificial MIME type is a workaround.
-                QString artificialType(kArtificial);
-                artificialType.append(metaData->name.trimmed().replace(kSpace, kDash));
-                mimeTypes.append(artificialType);
-            }
-            metaData->mimeTypes = mimeTypes;
-
             break;
         }
     }

src/plugins/texteditor/generichighlighter/manager.h

@@ -69,8 +69,10 @@ public:
     static Manager *instance();
 
     QString definitionIdByName(const QString &name) const;
-    QString definitionIdByMimeType(const QString &mimeType) const;
+    QString definitionIdByMimeType(const Core::MimeType &mimeType) const;
-    QString definitionIdByAnyMimeType(const QStringList &mimeTypes) const;
+    QString definitionIdByFile(const QString &filePath) const;
+    QString definitionIdByMimeTypeAndFile(const Core::MimeType &mimeType,
+                                          const QString &filePath) const;
     DefinitionMetaDataPtr availableDefinitionByName(const QString &name) const;
 
     bool isBuildingDefinition(const QString &id) const;
@@ -84,15 +86,15 @@ public:
     static DefinitionMetaDataPtr parseMetadata(const QFileInfo &fileInfo);
 
 public slots:
-    void registerMimeTypes();
+    void registerHighlightingFiles();
 
 private slots:
-    void registerMimeTypesFinished();
+    void registerHighlightingFilesFinished();
     void downloadAvailableDefinitionsListFinished();
     void downloadDefinitionsFinished();
 
 signals:
-    void mimeTypesRegistered();
+    void highlightingFilesRegistered();
 
 private:
     Manager();
@@ -114,7 +116,7 @@ private:
     };
     RegisterData m_register;
     bool m_hasQueuedRegistration;
-    QFutureWatcher<QPair<RegisterData, QList<Core::MimeType> > > m_registeringWatcher;
+    QFutureWatcher<RegisterData> m_registeringWatcher;
     friend class ManagerProcessor;
 
 signals:

src/plugins/texteditor/highlighterutils.cpp

@@ -37,30 +37,10 @@
 using namespace TextEditor;
 using namespace Internal;
 
-QString TextEditor::findDefinitionId(const Core::MimeType &mimeType,
-                         bool considerParents)
-{
-    QString definitionId = Manager::instance()->definitionIdByAnyMimeType(mimeType.aliases());
-    if (definitionId.isEmpty() && considerParents) {
-        definitionId = Manager::instance()->definitionIdByAnyMimeType(mimeType.subClassesOf());
-        if (definitionId.isEmpty()) {
-            foreach (const QString &parent, mimeType.subClassesOf()) {
-                const Core::MimeType &parentMimeType = Core::MimeDatabase::findByType(parent);
-                definitionId = findDefinitionId(parentMimeType, considerParents);
-            }
-        }
-    }
-    return definitionId;
-}
-
 void TextEditor::setMimeTypeForHighlighter(Highlighter *highlighter, const Core::MimeType &mimeType,
-                                           QString *foundDefinitionId)
+                                           const QString &filePath, QString *foundDefinitionId)
 {
-    const QString type = mimeType.type();
+    QString definitionId = Manager::instance()->definitionIdByMimeTypeAndFile(mimeType, filePath);
-    QString definitionId = Manager::instance()->definitionIdByMimeType(type);
-    if (definitionId.isEmpty())
-        definitionId = findDefinitionId(mimeType, true);
-
     if (!definitionId.isEmpty()) {
         const QSharedPointer<HighlightDefinition> &definition =
             Manager::instance()->definition(definitionId);
@@ -72,9 +52,3 @@ void TextEditor::setMimeTypeForHighlighter(Highlighter *highlighter, const Core:
         *foundDefinitionId = definitionId;
 }
 
-SyntaxHighlighter *TextEditor::createGenericSyntaxHighlighter(const Core::MimeType &mimeType)
-{
-    Highlighter *highlighter = new Highlighter();
-    setMimeTypeForHighlighter(highlighter, mimeType);
-    return highlighter;
-}

src/plugins/texteditor/highlighterutils.h

@@ -43,12 +43,10 @@ namespace Core { class MimeType; }
 namespace TextEditor {
 
 class Highlighter;
-class SyntaxHighlighter;
 
 void setMimeTypeForHighlighter(Highlighter *highlighter, const Core::MimeType &mimeType,
+                               const QString &filePath,
                                QString *foundDefinitionId = 0);
-QString findDefinitionId(const Core::MimeType &mimeType, bool considerParents);
-TEXTEDITOR_EXPORT SyntaxHighlighter *createGenericSyntaxHighlighter(const Core::MimeType &mimeType);
 
 } // namespace TextEditor
 

src/plugins/texteditor/texteditor.cpp

@@ -7178,7 +7178,8 @@ void TextEditorWidget::configureGenericHighlighter()
         d->m_isMissingSyntaxDefinition = true;
 
         QString definitionId;
-        setMimeTypeForHighlighter(highlighter, mimeType, &definitionId);
+        setMimeTypeForHighlighter(highlighter, mimeType, textDocument()->filePath().toString(),
+                                  &definitionId);
 
         if (!definitionId.isEmpty()) {
             d->m_isMissingSyntaxDefinition = false;
@@ -7218,7 +7219,7 @@ void TextEditorWidget::setupGenericHighlighter()
     connect(textDocument(), &IDocument::filePathChanged,
             d, &TextEditorWidgetPrivate::reconfigure);
 
-    connect(Manager::instance(), &Manager::mimeTypesRegistered,
+    connect(Manager::instance(), &Manager::highlightingFilesRegistered,
             d, &TextEditorWidgetPrivate::reconfigure);
 
     updateEditorInfoBar(this);

src/plugins/texteditor/texteditorplugin.cpp

@@ -125,7 +125,7 @@ bool TextEditorPlugin::initialize(const QStringList &arguments, QString *errorMe
     });
 
     // Generic highlighter.
-    connect(ICore::instance(), &ICore::coreOpened, Manager::instance(), &Manager::registerMimeTypes);
+    connect(ICore::instance(), &ICore::coreOpened, Manager::instance(), &Manager::registerHighlightingFiles);
 
     // Add text snippet provider.
     addAutoReleasedObject(new PlainTextSnippetProvider);