From e396f84d10aef973e752e7519f85de80d7dcdb86 Mon Sep 17 00:00:00 2001 From: Artem Sokolovskii Date: Tue, 28 Nov 2023 14:58:58 +0100 Subject: [PATCH] SyntaxHighlighter: Move generic highlighter to separate thread Adds a full copy of KSyntaxHighlighting::Repository class to the Highlighter class, enabling the relocation of the Highlighter class to a separate thread. This adjustment ensures that all Definitions come from the copy of the repository, making them immutable from external code. The "reload Definition" function stays supported, as triggering it results in the recreation of the highlighter for the document, thereby reconstructing the Repository class. Change-Id: Id7a4d865228c7e7e20e4770601a3fde55b8a6513 Reviewed-by: Jarek Kobus Reviewed-by: David Schulz --- src/plugins/texteditor/highlighter.cpp | 15 +++++++-- src/plugins/texteditor/highlighter.h | 10 ++++-- src/plugins/texteditor/highlighter_test.cpp | 6 ++-- src/plugins/texteditor/syntaxhighlighter.h | 2 +- .../texteditor/syntaxhighlighterrunner.cpp | 32 ++++++++++--------- .../texteditor/syntaxhighlighterrunner.h | 6 ++-- src/plugins/texteditor/texteditor.cpp | 16 +++++----- 7 files changed, 52 insertions(+), 35 deletions(-) diff --git a/src/plugins/texteditor/highlighter.cpp b/src/plugins/texteditor/highlighter.cpp index 49276a61c76..16ccc27370c 100644 --- a/src/plugins/texteditor/highlighter.cpp +++ b/src/plugins/texteditor/highlighter.cpp @@ -68,15 +68,24 @@ TextStyle categoryForTextStyle(int style) return C_TEXT; } -Highlighter::Highlighter() +Highlighter::Highlighter(const QString &definitionFilesPath) + : m_repository(new KSyntaxHighlighting::Repository()) { + m_repository->addCustomSearchPath(definitionFilesPath); + const Utils::FilePath dir = Core::ICore::resourcePath("generic-highlighter/syntax"); + if (dir.exists()) + m_repository->addCustomSearchPath(dir.parentDir().path()); + m_repository->reload(); + setTextFormatCategories(QMetaEnum::fromType().keyCount(), &categoryForTextStyle); } -KSyntaxHighlighting::Definition Highlighter::getDefinition() +Highlighter::~Highlighter() = default; + +void Highlighter::setDefinitionName(const QString &name) { - return definition(); + KSyntaxHighlighting::AbstractHighlighter::setDefinition(m_repository->definitionForName(name)); } static bool isOpeningParenthesis(QChar c) diff --git a/src/plugins/texteditor/highlighter.h b/src/plugins/texteditor/highlighter.h index 3cb3edce620..89e908968cf 100644 --- a/src/plugins/texteditor/highlighter.h +++ b/src/plugins/texteditor/highlighter.h @@ -9,6 +9,8 @@ #include +namespace KSyntaxHighlighting { class Repository; } + namespace TextEditor { class TextDocument; @@ -17,14 +19,18 @@ class Highlighter : public SyntaxHighlighter, public KSyntaxHighlighting::Abstra Q_OBJECT Q_INTERFACES(KSyntaxHighlighting::AbstractHighlighter) public: - Highlighter(); + Highlighter(const QString &definitionFilesPath); + ~Highlighter() override; - KSyntaxHighlighting::Definition getDefinition() override; + void setDefinitionName(const QString &name) override; protected: void highlightBlock(const QString &text) override; void applyFormat(int offset, int length, const KSyntaxHighlighting::Format &format) override; void applyFolding(int offset, int length, KSyntaxHighlighting::FoldingRegion region) override; + +private: + std::unique_ptr m_repository; }; } // namespace TextEditor diff --git a/src/plugins/texteditor/highlighter_test.cpp b/src/plugins/texteditor/highlighter_test.cpp index ef0718f65b4..151e31fc1bb 100644 --- a/src/plugins/texteditor/highlighter_test.cpp +++ b/src/plugins/texteditor/highlighter_test.cpp @@ -156,9 +156,9 @@ void GenerigHighlighterTests::testHighlight() QTextBlock block = m_editor->textDocument()->document()->findBlockByNumber(blockNumber); QVERIFY(block.isValid()); + QTRY_COMPARE(block.layout()->formats().size(), formatRanges.size()); const QList actualFormats = block.layout()->formats(); // full hash calculation for QTextCharFormat fails so just check the important entries of format - QCOMPARE(actualFormats.size(), formatRanges.size()); for (int i = 0; i < formatRanges.size(); ++i) compareFormats(actualFormats.at(i), formatRanges.at(i)); } @@ -178,9 +178,9 @@ void GenerigHighlighterTests::testChange() const FormatRanges formatRanges = {{0, 4, toFormat(C_VISUAL_WHITESPACE)}, {4, 1, toFormat(C_TEXT)}}; + QTRY_COMPARE(block.layout()->formats().size(), formatRanges.size()); const QList actualFormats = block.layout()->formats(); // full hash calculation for QTextCharFormat fails so just check the important entries of format - QCOMPARE(actualFormats.size(), formatRanges.size()); for (int i = 0; i < formatRanges.size(); ++i) compareFormats(actualFormats.at(i), formatRanges.at(i)); } @@ -204,9 +204,9 @@ void GenerigHighlighterTests::testPreeditText() {14, 1, toFormat(C_VISUAL_WHITESPACE)}, {15, 6, toFormat(C_STRING)}, {21, 1, toFormat(C_FUNCTION)}}; + QTRY_COMPARE(block.layout()->formats().size(), formatRanges.size()); const QList actualFormats = block.layout()->formats(); // full hash calculation for QTextCharFormat fails so just check the important entries of format - QCOMPARE(actualFormats.size(), formatRanges.size()); for (int i = 0; i < formatRanges.size(); ++i) compareFormats(actualFormats.at(i), formatRanges.at(i)); } diff --git a/src/plugins/texteditor/syntaxhighlighter.h b/src/plugins/texteditor/syntaxhighlighter.h index 1e9ffdabffc..e68ca74ef1b 100644 --- a/src/plugins/texteditor/syntaxhighlighter.h +++ b/src/plugins/texteditor/syntaxhighlighter.h @@ -112,7 +112,7 @@ public: void setExtraFormats(const QTextBlock &block, const QList &formats); virtual void setLanguageFeaturesFlags(unsigned int /*flags*/) {}; // needed for CppHighlighting virtual void setEnabled(bool /*enabled*/) {}; // needed for DiffAndLogHighlighter - virtual KSyntaxHighlighting::Definition getDefinition() { return {}; } + virtual void setDefinitionName(const QString & /*definitionName*/) {} // needed for Highlighter public slots: virtual void rehighlight(); diff --git a/src/plugins/texteditor/syntaxhighlighterrunner.cpp b/src/plugins/texteditor/syntaxhighlighterrunner.cpp index 9ba547da946..c992d1373e0 100644 --- a/src/plugins/texteditor/syntaxhighlighterrunner.cpp +++ b/src/plugins/texteditor/syntaxhighlighterrunner.cpp @@ -3,8 +3,8 @@ #include "syntaxhighlighterrunner.h" -#include -#include +#include "fontsettings.h" +#include "textdocumentlayout.h" #include @@ -81,7 +81,10 @@ public: m_highlighter->rehighlight(); } - KSyntaxHighlighting::Definition getDefinition() { return m_highlighter->getDefinition(); } + void setDefinitionName(const QString &name) + { + return m_highlighter->setDefinitionName(name); + } void setLanguageFeaturesFlags(unsigned int flags) { @@ -195,9 +198,18 @@ void BaseSyntaxHighlighterRunner::rehighlight() QMetaObject::invokeMethod(d.get(), [this] { d->rehighlight(); }); } -KSyntaxHighlighting::Definition BaseSyntaxHighlighterRunner::getDefinition() +QString BaseSyntaxHighlighterRunner::definitionName() { - return d->getDefinition(); + return m_definitionName; +} + +void BaseSyntaxHighlighterRunner::setDefinitionName(const QString &name) +{ + if (name.isEmpty()) + return; + + m_definitionName = name; + QMetaObject::invokeMethod(d.get(), [this, name] { d->setDefinitionName(name); }); } // --------------------------- ThreadedSyntaxHighlighterRunner ------------------------------------ @@ -240,16 +252,6 @@ ThreadedSyntaxHighlighterRunner::~ThreadedSyntaxHighlighterRunner() m_thread.wait(); } -KSyntaxHighlighting::Definition ThreadedSyntaxHighlighterRunner::getDefinition() -{ - KSyntaxHighlighting::Definition definition; - QMetaObject::invokeMethod( - d.get(), - [this, &definition] { definition = d->getDefinition(); }, - Qt::BlockingQueuedConnection); - return definition; -} - } // namespace TextEditor #include "syntaxhighlighterrunner.moc" diff --git a/src/plugins/texteditor/syntaxhighlighterrunner.h b/src/plugins/texteditor/syntaxhighlighterrunner.h index 984b227e77b..de2d8b5e423 100644 --- a/src/plugins/texteditor/syntaxhighlighterrunner.h +++ b/src/plugins/texteditor/syntaxhighlighterrunner.h @@ -39,7 +39,8 @@ public: void setEnabled(bool enabled); void rehighlight(); - virtual KSyntaxHighlighting::Definition getDefinition(); + QString definitionName(); + void setDefinitionName(const QString &name); QTextDocument *document() const { return m_document; } SyntaxHighLighterCreator creator() const { return m_creator; } @@ -56,6 +57,7 @@ protected: private: SyntaxHighLighterCreator m_creator; + QString m_definitionName; }; class TEXTEDITOR_EXPORT ThreadedSyntaxHighlighterRunner : public BaseSyntaxHighlighterRunner @@ -65,8 +67,6 @@ public: QTextDocument *document); ~ThreadedSyntaxHighlighterRunner(); - KSyntaxHighlighting::Definition getDefinition() override; - private: QThread m_thread; }; diff --git a/src/plugins/texteditor/texteditor.cpp b/src/plugins/texteditor/texteditor.cpp index 742a2a748a8..1495003c79c 100644 --- a/src/plugins/texteditor/texteditor.cpp +++ b/src/plugins/texteditor/texteditor.cpp @@ -1933,7 +1933,7 @@ void TextEditorWidgetPrivate::foldLicenseHeader() QStringList docMarker; HighlighterHelper::Definition def; if (BaseSyntaxHighlighterRunner *highlighter = q->textDocument()->syntaxHighlighterRunner()) - def = highlighter->getDefinition(); + def = HighlighterHelper::definitionForName(highlighter->definitionName()); if (def.isValid()) { for (const QString &marker : @@ -3709,12 +3709,12 @@ void TextEditorWidgetPrivate::configureGenericHighlighter( q->setCodeFoldingSupported(false); } - m_document->resetSyntaxHighlighter([definition] { - auto highlighter = new Highlighter(); - if (definition.isValid()) - highlighter->setDefinition(definition); - return highlighter; - }, false); + const QString definitionFilesPath + = TextEditorSettings::highlighterSettings().definitionFilesPath().toString(); + m_document->resetSyntaxHighlighter([definitionFilesPath] { + return new Highlighter(definitionFilesPath); + }); + m_document->syntaxHighlighterRunner()->setDefinitionName(definition.name()); m_document->setFontSettings(TextEditorSettings::fontSettings()); } @@ -3740,7 +3740,7 @@ void TextEditorWidgetPrivate::setupFromDefinition(const KSyntaxHighlighting::Def KSyntaxHighlighting::Definition TextEditorWidgetPrivate::currentDefinition() { if (BaseSyntaxHighlighterRunner *highlighter = m_document->syntaxHighlighterRunner()) - return highlighter->getDefinition(); + return HighlighterHelper::definitionForName(highlighter->definitionName()); return {}; }