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 <jaroslaw.kobus@qt.io>
Reviewed-by: David Schulz <david.schulz@qt.io>

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<KSyntaxHighlighting::Theme::TextStyle>().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)

src/plugins/texteditor/highlighter.h

@@ -9,6 +9,8 @@
 
 #include <KSyntaxHighlighting/AbstractHighlighter>
 
+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<KSyntaxHighlighting::Repository> m_repository;
 };
 
 } // namespace TextEditor

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<QTextLayout::FormatRange> 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<QTextLayout::FormatRange> 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<QTextLayout::FormatRange> 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));
 }

src/plugins/texteditor/syntaxhighlighter.h

@@ -112,7 +112,7 @@ public:
     void setExtraFormats(const QTextBlock &block, const QList<QTextLayout::FormatRange> &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();

src/plugins/texteditor/syntaxhighlighterrunner.cpp

@@ -3,8 +3,8 @@
 
 #include "syntaxhighlighterrunner.h"
 
-#include <texteditor/fontsettings.h>
+#include "fontsettings.h"
-#include <texteditor/textdocumentlayout.h>
+#include "textdocumentlayout.h"
 
 #include <utils/textutils.h>
 
@@ -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"

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;
 };

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] {
+    const QString definitionFilesPath
-        auto highlighter = new Highlighter();
+        = TextEditorSettings::highlighterSettings().definitionFilesPath().toString();
-        if (definition.isValid())
+    m_document->resetSyntaxHighlighter([definitionFilesPath] {
-            highlighter->setDefinition(definition);
+        return new Highlighter(definitionFilesPath);
-        return highlighter;
+    });
-    }, false);
+    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 {};
 }