From a22dc36aaf612735d09a51d4fb5865fc64297d02 Mon Sep 17 00:00:00 2001 From: Eike Ziller Date: Tue, 3 Feb 2015 16:47:03 +0100 Subject: [PATCH] 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 --- .../highlightersettingspage.cpp | 16 +- .../highlightersettingspage.h | 2 +- .../texteditor/generichighlighter/manager.cpp | 213 ++++++++---------- .../texteditor/generichighlighter/manager.h | 14 +- src/plugins/texteditor/highlighterutils.cpp | 30 +-- src/plugins/texteditor/highlighterutils.h | 4 +- src/plugins/texteditor/texteditor.cpp | 5 +- src/plugins/texteditor/texteditorplugin.cpp | 2 +- 8 files changed, 112 insertions(+), 174 deletions(-) diff --git a/src/plugins/texteditor/generichighlighter/highlightersettingspage.cpp b/src/plugins/texteditor/generichighlighter/highlightersettingspage.cpp index 4a34bd6b320..ea904d7286e 100644 --- a/src/plugins/texteditor/generichighlighter/highlightersettingspage.cpp +++ b/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) { - Manager::instance()->registerMimeTypes(); - m_requestMimeTypeRegistration = false; + if (m_requestHighlightFileRegistration) { + Manager::instance()->registerHighlightingFiles(); + 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 QListm_page->definitionFilesPath->path() + QLatin1Char('/'), m_d->m_page->definitionFilesPath->buttonAtIndex(1)->window()); - if (dialog.exec() && !m_requestMimeTypeRegistration) - m_requestMimeTypeRegistration = true; + if (dialog.exec() && !m_requestHighlightFileRegistration) + m_requestHighlightFileRegistration = true; setDownloadDefinitionsState(m_d->m_page->definitionFilesPath->isValid()); } diff --git a/src/plugins/texteditor/generichighlighter/highlightersettingspage.h b/src/plugins/texteditor/generichighlighter/highlightersettingspage.h index f1bd8f6c4a2..4c19e67fd05 100644 --- a/src/plugins/texteditor/generichighlighter/highlightersettingspage.h +++ b/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; diff --git a/src/plugins/texteditor/generichighlighter/manager.cpp b/src/plugins/texteditor/generichighlighter/manager.cpp index e7fdf65a34e..08dc24856b5 100644 --- a/src/plugins/texteditor/generichighlighter/manager.cpp +++ b/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; - foreach (const QString &mimeType, mimeTypes) { - definitionId = definitionIdByMimeType(mimeType); - if (!definitionId.isEmpty()) - break; + QList queue; + queue.append(mimeType); + while (!queue.isEmpty()) { + const Core::MimeType mt = queue.takeFirst(); + 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 > > &future); + void process(QFutureInterface &future); QStringList m_definitionsPaths; - QSet m_knownMimeTypes; - QSet m_knownSuffixes; - QHash m_userModified; static const int kMaxProgress; }; const int ManagerProcessor::kMaxProgress = 200; ManagerProcessor::ManagerProcessor() - : m_knownSuffixes(QSet::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 > > &future) +void ManagerProcessor::process(QFutureInterface &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 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 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)) - // Name already exists... This is a fallback item, do not consider it. - continue; - - const QString &id = metaData->id; - data.m_idByName.insert(metaData->name, id); - data.m_definitionsMetaData.insert(id, metaData); - - static const QStringList textPlain(QLatin1String("text/plain")); - - // A definition can specify multiple MIME types and file extensions/patterns, - // but all on a single string. So associate all patterns with all MIME types. - QList globPatterns; - foreach (const QString &type, metaData->mimeTypes) { - if (data.m_idByMimeType.contains(type)) - continue; - - 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::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)); - } - } - mimeType.setGlobPatterns(globPatterns); - } else { - mimeType.setGlobPatterns(it.value().globPatterns()); - mimeType.setMagicRuleMatchers(it.value().magicRuleMatchers()); + const DefinitionMetaDataPtr &metaData = + Manager::parseMetadata(fileInfo); + // skip failing or already existing definitions + if (!metaData.isNull() && !data.m_idByName.contains(metaData->name)) { + const QString id = metaData->id; + data.m_idByName.insert(metaData->name, id); + data.m_definitionsMetaData.insert(id, metaData); + foreach (const QString &mt, metaData->mimeTypes) { + bool insert = true; + // check if there is already a definition registered with higher priority + const QString existingDefinition = data.m_idByMimeType.value(mt); + if (!existingDefinition.isEmpty()) { + // check priorities + DefinitionMetaDataPtr existingMetaData = + data.m_definitionsMetaData.value(existingDefinition); + if (!existingMetaData.isNull() && existingMetaData->priority > metaData->priority) + insert = false; } - - newMimeTypes.append(mimeType); + if (insert) + data.m_idByMimeType.insert(mt, id); } } } } - future.reportResult(qMakePair(data, newMimeTypes)); + future.reportResult(data); } -void Manager::registerMimeTypes() +void Manager::registerHighlightingFiles() { if (!m_registeringWatcher.isRunning()) { clear(); ManagerProcessor *processor = new ManagerProcessor; - QFuture > > future = + QFuture 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 > &result = m_registeringWatcher.result(); - m_register = result.first; + m_register = m_registeringWatcher.result(); - foreach (const MimeType &mimeType, result.second) - MimeDatabase::addMimeType(mimeType); - - emit mimeTypesRegistered(); + emit highlightingFilesRegistered(); } } 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; } } diff --git a/src/plugins/texteditor/generichighlighter/manager.h b/src/plugins/texteditor/generichighlighter/manager.h index 0b8bf4dca94..4105f65b646 100644 --- a/src/plugins/texteditor/generichighlighter/manager.h +++ b/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 definitionIdByAnyMimeType(const QStringList &mimeTypes) const; + QString definitionIdByMimeType(const Core::MimeType &mimeType) 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 > > m_registeringWatcher; + QFutureWatcher m_registeringWatcher; friend class ManagerProcessor; signals: diff --git a/src/plugins/texteditor/highlighterutils.cpp b/src/plugins/texteditor/highlighterutils.cpp index 0e84c347e73..2c0dc7b6faa 100644 --- a/src/plugins/texteditor/highlighterutils.cpp +++ b/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()->definitionIdByMimeType(type); - if (definitionId.isEmpty()) - definitionId = findDefinitionId(mimeType, true); - + QString definitionId = Manager::instance()->definitionIdByMimeTypeAndFile(mimeType, filePath); if (!definitionId.isEmpty()) { const QSharedPointer &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; -} diff --git a/src/plugins/texteditor/highlighterutils.h b/src/plugins/texteditor/highlighterutils.h index b0e1b70b699..0d9a9b09423 100644 --- a/src/plugins/texteditor/highlighterutils.h +++ b/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 diff --git a/src/plugins/texteditor/texteditor.cpp b/src/plugins/texteditor/texteditor.cpp index c9c824f19a0..56adaf2829f 100644 --- a/src/plugins/texteditor/texteditor.cpp +++ b/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); diff --git a/src/plugins/texteditor/texteditorplugin.cpp b/src/plugins/texteditor/texteditorplugin.cpp index 0d3b11e1440..efa8d8b4bdc 100644 --- a/src/plugins/texteditor/texteditorplugin.cpp +++ b/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);