TextEditor: Use FilePath in HighlighterSettings

Change-Id: I1002d3a0b16e403e1650475ec316adf797251a1f
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/texteditor/highlighter.cpp

@@ -62,7 +62,7 @@ static KSyntaxHighlighting::Repository *highlightRepository()
     static KSyntaxHighlighting::Repository *repository = nullptr;
     if (!repository) {
         repository = new KSyntaxHighlighting::Repository();
-        repository->addCustomSearchPath(TextEditorSettings::highlighterSettings().definitionFilesPath());
+        repository->addCustomSearchPath(TextEditorSettings::highlighterSettings().definitionFilesPath().toString());
         const FilePath dir = Core::ICore::resourcePath("generic-highlighter/syntax");
         if (dir.exists())
             repository->addCustomSearchPath(dir.parentDir().path());

src/plugins/texteditor/highlightersettings.cpp

@@ -35,21 +35,16 @@
 #include <utils/stringutils.h>
 
 #include <QSettings>
-#include <QLatin1String>
-#include <QLatin1Char>
-#include <QDir>
-#include <QFile>
 #include <QStringList>
 
+using namespace Utils;
+
 namespace TextEditor {
 namespace Internal {
 
-QString findFallbackDefinitionsLocation()
+FilePath findFallbackDefinitionsLocation()
 {
-    QDir dir;
+    if (HostOsInfo::isAnyUnixHost() && !HostOsInfo::isMacHost()) {
-    dir.setNameFilters(QStringList(QLatin1String("*.xml")));
-
-    if (Utils::HostOsInfo::isAnyUnixHost() && !Utils::HostOsInfo::isMacHost()) {
         static const QLatin1String kateSyntaxPaths[] = {
             QLatin1String("/share/apps/katepart/syntax"),
             QLatin1String("/share/kde4/apps/katepart/syntax")
@@ -57,69 +52,61 @@ QString findFallbackDefinitionsLocation()
 
         // Some wild guesses.
         for (const auto &kateSyntaxPath : kateSyntaxPaths) {
-            const QStringList paths = {
+            const FilePath paths[] = {
-                QLatin1String("/usr") + kateSyntaxPath,
+                FilePath("/usr") / kateSyntaxPath,
-                QLatin1String("/usr/local") + kateSyntaxPath,
+                FilePath("/usr/local") / kateSyntaxPath,
-                QLatin1String("/opt") + kateSyntaxPath
+                FilePath("/opt") / kateSyntaxPath
             };
-            for (const auto &path : paths) {
+            for (const FilePath &path : paths) {
-                dir.setPath(path);
+                if (path.exists() && !path.dirEntries({"*.xml"}).isEmpty())
-                if (dir.exists() && !dir.entryInfoList().isEmpty())
+                    return path;
-                    return dir.path();
             }
         }
 
         // Try kde-config.
-        const Utils::FilePath programs[] = {"kde-config", "kde4-config"};
+        const FilePath programs[] = {"kde-config", "kde4-config"};
-        for (const auto &program : programs) {
+        for (const FilePath &program : programs) {
-            Utils::QtcProcess process;
+            QtcProcess process;
             process.setTimeoutS(5);
             process.setCommand({program, {"--prefix"}});
             process.runBlocking();
-            if (process.result() == Utils::QtcProcess::FinishedWithSuccess) {
+            if (process.result() == QtcProcess::FinishedWithSuccess) {
                 QString output = process.stdOut();
-                output.remove(QLatin1Char('\n'));
+                output.remove('\n');
+                const FilePath dir = FilePath::fromString(output);
                 for (auto &kateSyntaxPath : kateSyntaxPaths) {
-                    dir.setPath(output + kateSyntaxPath);
+                    const FilePath path = dir / kateSyntaxPath;
-                    if (dir.exists() && !dir.entryInfoList().isEmpty())
+                    if (path.exists() && !path.dirEntries({"*.xml"}).isEmpty())
-                        return dir.path();
+                        return path;
                 }
             }
         }
     }
 
-    dir.setPath(Core::ICore::resourcePath("generic-highlighter").toString());
+    const FilePath dir = Core::ICore::resourcePath("generic-highlighter");
-    if (dir.exists() && !dir.entryInfoList().isEmpty())
+    if (dir.exists() && !dir.dirEntries({"*.xml"}).isEmpty())
-        return dir.path();
+        return dir;
 
-    return QString();
+    return {};
 }
 
 } // namespace Internal
-} // namespace TextEditor
 
-namespace {
+const QLatin1String kDefinitionFilesPath("UserDefinitionFilesPath");
+const QLatin1String kIgnoredFilesPatterns("IgnoredFilesPatterns");
 
-static const QLatin1String kDefinitionFilesPath("UserDefinitionFilesPath");
+static QString groupSpecifier(const QString &postFix, const QString &category)
-static const QLatin1String kIgnoredFilesPatterns("IgnoredFilesPatterns");
-
-QString groupSpecifier(const QString &postFix, const QString &category)
 {
     if (category.isEmpty())
         return postFix;
     return QString(category + postFix);
 }
 
-} // namespace anonymous
-
-using namespace TextEditor;
-using namespace Internal;
-
 void HighlighterSettings::toSettings(const QString &category, QSettings *s) const
 {
     const QString &group = groupSpecifier(Constants::HIGHLIGHTER_SETTINGS_CATEGORY, category);
     s->beginGroup(group);
-    s->setValue(kDefinitionFilesPath, m_definitionFilesPath);
+    s->setValue(kDefinitionFilesPath, m_definitionFilesPath.toVariant());
     s->setValue(kIgnoredFilesPatterns, ignoredFilesPatterns());
     s->endGroup();
 }
@@ -128,11 +115,11 @@ void HighlighterSettings::fromSettings(const QString &category, QSettings *s)
 {
     const QString &group = groupSpecifier(Constants::HIGHLIGHTER_SETTINGS_CATEGORY, category);
     s->beginGroup(group);
-    m_definitionFilesPath = s->value(kDefinitionFilesPath, QString()).toString();
+    m_definitionFilesPath = FilePath::fromVariant(s->value(kDefinitionFilesPath));
     if (!s->contains(kDefinitionFilesPath))
         assignDefaultDefinitionsPath();
     else
-        m_definitionFilesPath = s->value(kDefinitionFilesPath).toString();
+        m_definitionFilesPath = FilePath::fromVariant(s->value(kDefinitionFilesPath));
     if (!s->contains(kIgnoredFilesPatterns))
         assignDefaultIgnoredPatterns();
     else
@@ -142,31 +129,29 @@ void HighlighterSettings::fromSettings(const QString &category, QSettings *s)
 
 void HighlighterSettings::setIgnoredFilesPatterns(const QString &patterns)
 {
-    setExpressionsFromList(patterns.split(QLatin1Char(','), Qt::SkipEmptyParts));
+    setExpressionsFromList(patterns.split(',', Qt::SkipEmptyParts));
 }
 
 QString HighlighterSettings::ignoredFilesPatterns() const
 {
-    return listFromExpressions().join(QLatin1Char(','));
+    return listFromExpressions().join(',');
 }
 
 void HighlighterSettings::assignDefaultIgnoredPatterns()
 {
-    QStringList patterns;
+    setExpressionsFromList({"*.txt",
-    patterns << QLatin1String("*.txt")
+                            "LICENSE*",
-        << QLatin1String("LICENSE*")
+                            "README",
-        << QLatin1String("README")
+                            "INSTALL",
-        << QLatin1String("INSTALL")
+                            "COPYING",
-        << QLatin1String("COPYING")
+                            "NEWS",
-        << QLatin1String("NEWS")
+                            "qmldir"});
-        << QLatin1String("qmldir");
-    setExpressionsFromList(patterns);
 }
 
 void HighlighterSettings::assignDefaultDefinitionsPath()
 {
-    const QString path = Core::ICore::userResourcePath("generic-highlighter").toString();
+    const FilePath path = Core::ICore::userResourcePath("generic-highlighter");
-    if (QFile::exists(path) || QDir().mkpath(path))
+    if (path.exists() || path.ensureWritableDir())
         m_definitionFilesPath = path;
 }
 
@@ -203,3 +188,5 @@ QStringList HighlighterSettings::listFromExpressions() const
         patterns.append(regExp.pattern());
     return patterns;
 }
+
+} // TextEditor

src/plugins/texteditor/highlightersettings.h

@@ -25,6 +25,8 @@
 
 #pragma once
 
+#include <utils/filepath.h>
+
 #include <QString>
 #include <QStringList>
 #include <QList>
@@ -44,8 +46,8 @@ public:
     void toSettings(const QString &category, QSettings *s) const;
     void fromSettings(const QString &category, QSettings *s);
 
-    void setDefinitionFilesPath(const QString &path) { m_definitionFilesPath = path; }
+    void setDefinitionFilesPath(const Utils::FilePath &path) { m_definitionFilesPath = path; }
-    const QString &definitionFilesPath() const { return m_definitionFilesPath; }
+    const Utils::FilePath &definitionFilesPath() const { return m_definitionFilesPath; }
 
     void setIgnoredFilesPatterns(const QString &patterns);
     QString ignoredFilesPatterns() const;
@@ -53,6 +55,12 @@ public:
 
     bool equals(const HighlighterSettings &highlighterSettings) const;
 
+    friend bool operator==(const HighlighterSettings &a, const HighlighterSettings &b)
+    { return a.equals(b); }
+
+    friend bool operator!=(const HighlighterSettings &a, const HighlighterSettings &b)
+    { return !a.equals(b); }
+
 private:
     void assignDefaultIgnoredPatterns();
     void assignDefaultDefinitionsPath();
@@ -60,18 +68,12 @@ private:
     void setExpressionsFromList(const QStringList &patterns);
     QStringList listFromExpressions() const;
 
-    QString m_definitionFilesPath;
+    Utils::FilePath m_definitionFilesPath;
     QList<QRegularExpression> m_ignoredFiles;
 };
 
-inline bool operator==(const HighlighterSettings &a, const HighlighterSettings &b)
-{ return a.equals(b); }
-
-inline bool operator!=(const HighlighterSettings &a, const HighlighterSettings &b)
-{ return !a.equals(b); }
-
 namespace Internal {
-QString findFallbackDefinitionsLocation();
+Utils::FilePath findFallbackDefinitionsLocation();
 }
 
 } // namespace TextEditor

src/plugins/texteditor/highlightersettingspage.cpp

@@ -34,20 +34,23 @@
 #include <QMessageBox>
 #include <QPointer>
 
-using namespace TextEditor;
+using namespace TextEditor::Internal;
-using namespace Internal;
+using namespace Utils;
 
-struct HighlighterSettingsPage::HighlighterSettingsPagePrivate
+namespace TextEditor {
+
+class HighlighterSettingsPage::HighlighterSettingsPagePrivate
 {
     Q_DECLARE_TR_FUNCTIONS(TextEditor::Internal::HighlighterSettingsPage)
 
 public:
-    HighlighterSettingsPagePrivate();
+    HighlighterSettingsPagePrivate() = default;
+
     void ensureInitialized();
     void migrateGenericHighlighterFiles();
 
     bool m_initialized = false;
-    const QString m_settingsPrefix;
+    const QString m_settingsPrefix{"Text"};
 
     HighlighterSettings m_settings;
 
@@ -55,13 +58,10 @@ public:
     Ui::HighlighterSettingsPage *m_page = nullptr;
 };
 
-HighlighterSettingsPage::HighlighterSettingsPagePrivate::HighlighterSettingsPagePrivate()
-    : m_settingsPrefix("Text")
-{}
 
 void HighlighterSettingsPage::HighlighterSettingsPagePrivate::migrateGenericHighlighterFiles()
 {
-    QDir userDefinitionPath(m_settings.definitionFilesPath());
+    QDir userDefinitionPath(m_settings.definitionFilesPath().toString());
     if (userDefinitionPath.mkdir("syntax")) {
         const auto link = Utils::HostOsInfo::isAnyUnixHost()
                               ? static_cast<bool(*)(const QString &, const QString &)>(&QFile::link)
@@ -82,7 +82,7 @@ void HighlighterSettingsPage::HighlighterSettingsPagePrivate::ensureInitialized(
 }
 
 HighlighterSettingsPage::HighlighterSettingsPage()
-    : m_d(new HighlighterSettingsPagePrivate)
+    : d(new HighlighterSettingsPagePrivate)
 {
     setId(Constants::TEXT_EDITOR_HIGHLIGHTER_SETTINGS);
     setDisplayName(HighlighterSettingsPagePrivate::tr("Generic Highlighter"));
@@ -93,40 +93,40 @@ HighlighterSettingsPage::HighlighterSettingsPage()
 
 HighlighterSettingsPage::~HighlighterSettingsPage()
 {
-    delete m_d;
+    delete d;
 }
 
 QWidget *HighlighterSettingsPage::widget()
 {
-    if (!m_d->m_widget) {
+    if (!d->m_widget) {
-        m_d->m_widget = new QWidget;
+        d->m_widget = new QWidget;
-        m_d->m_page = new Ui::HighlighterSettingsPage;
+        d->m_page = new Ui::HighlighterSettingsPage;
-        m_d->m_page->setupUi(m_d->m_widget);
+        d->m_page->setupUi(d->m_widget);
-        m_d->m_page->definitionFilesPath->setExpectedKind(Utils::PathChooser::ExistingDirectory);
+        d->m_page->definitionFilesPath->setExpectedKind(Utils::PathChooser::ExistingDirectory);
-        m_d->m_page->definitionFilesPath->setHistoryCompleter(QLatin1String("TextEditor.Highlighter.History"));
+        d->m_page->definitionFilesPath->setHistoryCompleter(QLatin1String("TextEditor.Highlighter.History"));
-        connect(m_d->m_page->downloadDefinitions,
+        connect(d->m_page->downloadDefinitions,
                 &QPushButton::pressed,
-                [label = QPointer<QLabel>(m_d->m_page->updateStatus)]() {
+                [label = QPointer<QLabel>(d->m_page->updateStatus)]() {
                     Highlighter::downloadDefinitions([label](){
                         if (label)
                             label->setText(HighlighterSettingsPagePrivate::tr("Download finished"));
                     });
                 });
-        connect(m_d->m_page->reloadDefinitions, &QPushButton::pressed, []() {
+        connect(d->m_page->reloadDefinitions, &QPushButton::pressed, []() {
             Highlighter::reload();
         });
-        connect(m_d->m_page->resetCache, &QPushButton::clicked, []() {
+        connect(d->m_page->resetCache, &QPushButton::clicked, []() {
             Highlighter::clearDefinitionForDocumentCache();
         });
 
         settingsToUI();
     }
-    return m_d->m_widget;
+    return d->m_widget;
 }
 
 void HighlighterSettingsPage::apply()
 {
-    if (!m_d->m_page) // page was not shown
+    if (!d->m_page) // page was not shown
         return;
     if (settingsChanged())
         settingsFromUI();
@@ -134,37 +134,39 @@ void HighlighterSettingsPage::apply()
 
 void HighlighterSettingsPage::finish()
 {
-    delete m_d->m_widget;
+    delete d->m_widget;
-    if (!m_d->m_page) // page was not shown
+    if (!d->m_page) // page was not shown
         return;
-    delete m_d->m_page;
+    delete d->m_page;
-    m_d->m_page = nullptr;
+    d->m_page = nullptr;
 }
 
 const HighlighterSettings &HighlighterSettingsPage::highlighterSettings() const
 {
-    m_d->ensureInitialized();
+    d->ensureInitialized();
-    return m_d->m_settings;
+    return d->m_settings;
 }
 
 void HighlighterSettingsPage::settingsFromUI()
 {
-    m_d->ensureInitialized();
+    d->ensureInitialized();
-    m_d->m_settings.setDefinitionFilesPath(m_d->m_page->definitionFilesPath->filePath().toString());
+    d->m_settings.setDefinitionFilesPath(d->m_page->definitionFilesPath->filePath());
-    m_d->m_settings.setIgnoredFilesPatterns(m_d->m_page->ignoreEdit->text());
+    d->m_settings.setIgnoredFilesPatterns(d->m_page->ignoreEdit->text());
-    m_d->m_settings.toSettings(m_d->m_settingsPrefix, Core::ICore::settings());
+    d->m_settings.toSettings(d->m_settingsPrefix, Core::ICore::settings());
 }
 
 void HighlighterSettingsPage::settingsToUI()
 {
-    m_d->ensureInitialized();
+    d->ensureInitialized();
-    m_d->m_page->definitionFilesPath->setPath(m_d->m_settings.definitionFilesPath());
+    d->m_page->definitionFilesPath->setFilePath(d->m_settings.definitionFilesPath());
-    m_d->m_page->ignoreEdit->setText(m_d->m_settings.ignoredFilesPatterns());
+    d->m_page->ignoreEdit->setText(d->m_settings.ignoredFilesPatterns());
 }
 
 bool HighlighterSettingsPage::settingsChanged() const
 {
-    m_d->ensureInitialized();
+    d->ensureInitialized();
-    return (m_d->m_settings.definitionFilesPath() != m_d->m_page->definitionFilesPath->filePath().toString())
+    return d->m_settings.definitionFilesPath() != d->m_page->definitionFilesPath->filePath()
-            || (m_d->m_settings.ignoredFilesPatterns() != m_d->m_page->ignoreEdit->text());
+        || d->m_settings.ignoredFilesPatterns() != d->m_page->ignoreEdit->text();
 }
+
+} // TextEditor

src/plugins/texteditor/highlightersettingspage.h

@@ -49,8 +49,8 @@ private:
 
     bool settingsChanged() const;
 
-    struct HighlighterSettingsPagePrivate;
+    class HighlighterSettingsPagePrivate;
-    HighlighterSettingsPagePrivate *m_d;
+    HighlighterSettingsPagePrivate *d;
 };
 
 } // namespace TextEditor