From 283673a5d99f25ff882aa51aed9e3272bd6d73ba Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Tue, 21 Jun 2022 12:56:13 +0200 Subject: [PATCH] AbstractSettings: Introduce VersionUpdater Uniform version number parsing inside AbstractSettings. Get rid of updateVersion() virtual method and add protected setVersionRegExp() instead. If the user calls version() we wait for version process to finish if it was running. This makes sure that we don't return old / invalid version number while version process is still running. Change-Id: Ie27816534dd52c086acde721f3b49e669a7c30bc Reviewed-by: hjk --- src/plugins/beautifier/abstractsettings.cpp | 87 +++++++++++++++---- src/plugins/beautifier/abstractsettings.h | 16 +++- .../artisticstyle/artisticstyle.cpp | 7 +- .../artisticstyle/artisticstylesettings.cpp | 54 ++---------- .../artisticstyle/artisticstylesettings.h | 15 ---- .../beautifier/uncrustify/uncrustify.cpp | 3 +- .../uncrustify/uncrustifysettings.cpp | 30 +------ .../uncrustify/uncrustifysettings.h | 8 -- 8 files changed, 96 insertions(+), 124 deletions(-) diff --git a/src/plugins/beautifier/abstractsettings.cpp b/src/plugins/beautifier/abstractsettings.cpp index d691984d62f..e9768041452 100644 --- a/src/plugins/beautifier/abstractsettings.cpp +++ b/src/plugins/beautifier/abstractsettings.cpp @@ -34,11 +34,16 @@ #include #include #include +#include #include #include +#include +#include #include +using namespace Utils; + namespace Beautifier { namespace Internal { @@ -47,12 +52,65 @@ const char COMMAND[] = "command"; const char SUPPORTED_MIME[] = "supportedMime"; } +class VersionUpdater +{ +public: + VersionUpdater() + { + QObject::connect(&m_process, &QtcProcess::done, [this] { + if (m_process.result() != ProcessResult::FinishedWithSuccess) + return; + + m_versionNumber = parseVersion(m_process.cleanedStdOut()); + if (m_versionNumber.isNull()) + m_versionNumber = parseVersion(m_process.cleanedStdErr()); + }); + } + + void setVersionRegExp(const QRegularExpression &versionRegExp) + { + m_versionRegExp = versionRegExp; + } + + void update(const FilePath &executable) + { + m_versionNumber = {}; + if (m_versionRegExp.pattern().isEmpty()) + return; + m_process.close(); + m_process.setCommand({executable, {"--version"}}); + m_process.start(); + } + + QVersionNumber version() const + { + if (m_process.state() != QProcess::NotRunning) + m_process.waitForFinished(-1); + return m_versionNumber; + } + +private: + QVersionNumber parseVersion(const QString &text) const + { + const QRegularExpressionMatch match = m_versionRegExp.match(text); + if (!match.hasMatch()) + return {}; + + return {match.captured(1).toInt(), match.captured(2).toInt()}; + } + + QRegularExpression m_versionRegExp; + mutable QtcProcess m_process; + QVersionNumber m_versionNumber; +}; + AbstractSettings::AbstractSettings(const QString &name, const QString &ending) : m_ending(ending) , m_styleDir(Core::ICore::userResourcePath(Beautifier::Constants::SETTINGS_DIRNAME) .pathAppended(name) .toString()) , m_name(name) + , m_versionUpdater(new VersionUpdater) { } @@ -122,29 +180,28 @@ QString AbstractSettings::styleFileName(const QString &key) const return m_styleDir.absoluteFilePath(key + m_ending); } -Utils::FilePath AbstractSettings::command() const +FilePath AbstractSettings::command() const { - return Utils::FilePath::fromString(m_command); + return FilePath::fromString(m_command); } -void AbstractSettings::setCommand(const QString &command) +void AbstractSettings::setCommand(const QString &cmd) { - if (command == m_command) + if (cmd == m_command) return; - m_command = command; - updateVersion(); + m_command = cmd; + m_versionUpdater->update(command()); } -int AbstractSettings::version() const +QVersionNumber AbstractSettings::version() const { - return m_version; + return m_versionUpdater->version(); } -void AbstractSettings::updateVersion() +void AbstractSettings::setVersionRegExp(const QRegularExpression &versionRegExp) { - // If a beautifier needs to know the current tool's version, reimplement and store the version - // in m_version. + m_versionUpdater->setVersionRegExp(versionRegExp); } QString AbstractSettings::supportedMimeTypesAsString() const @@ -157,7 +214,7 @@ void AbstractSettings::setSupportedMimeTypes(const QString &mimes) const QStringList stringTypes = mimes.split(';'); QStringList types; for (const QString &type : stringTypes) { - const Utils::MimeType mime = Utils::mimeTypeForName(type.trimmed()); + const MimeType mime = mimeTypeForName(type.trimmed()); if (!mime.isValid()) continue; const QString canonicalName = mime.name(); @@ -179,8 +236,8 @@ bool AbstractSettings::isApplicable(const Core::IDocument *document) const if (m_supportedMimeTypes.isEmpty()) return true; - const Utils::MimeType documentMimeType = Utils::mimeTypeForName(document->mimeType()); - return Utils::anyOf(m_supportedMimeTypes, [&documentMimeType](const QString &mime) { + const MimeType documentMimeType = mimeTypeForName(document->mimeType()); + return anyOf(m_supportedMimeTypes, [&documentMimeType](const QString &mime) { return documentMimeType.inherits(mime); }); } @@ -246,7 +303,7 @@ void AbstractSettings::save() continue; } - Utils::FileSaver saver(Utils::FilePath::fromUserInput(fi.absoluteFilePath())); + FileSaver saver(FilePath::fromUserInput(fi.absoluteFilePath())); if (saver.hasError()) { BeautifierPlugin::showError(tr("Cannot open file \"%1\": %2.") .arg(saver.filePath().toUserOutput()) diff --git a/src/plugins/beautifier/abstractsettings.h b/src/plugins/beautifier/abstractsettings.h index ae61bde2676..477f5c77187 100644 --- a/src/plugins/beautifier/abstractsettings.h +++ b/src/plugins/beautifier/abstractsettings.h @@ -35,12 +35,19 @@ #include #include +QT_BEGIN_NAMESPACE +class QRegularExpression; +class QVersionNumber; +QT_END_NAMESPACE + namespace Core { class IDocument; } namespace Utils { class FilePath; } namespace Beautifier { namespace Internal { +class VersionUpdater; + class AbstractSettings : public QObject { Q_OBJECT @@ -66,9 +73,8 @@ public: virtual QString styleFileName(const QString &key) const; Utils::FilePath command() const; - void setCommand(const QString &command); - int version() const; - virtual void updateVersion(); + void setCommand(const QString &cmd); + QVersionNumber version() const; QString supportedMimeTypesAsString() const; void setSupportedMimeTypes(const QString &mimes); @@ -81,9 +87,10 @@ signals: void supportedMimeTypesChanged(); protected: + void setVersionRegExp(const QRegularExpression &versionRegExp); + QMap m_styles; QMap m_settings; - int m_version = 0; QString m_ending; QDir m_styleDir; @@ -92,6 +99,7 @@ protected: private: QString m_name; + std::unique_ptr m_versionUpdater; QStringList m_stylesToRemove; QSet m_changedStyles; QString m_command; diff --git a/src/plugins/beautifier/artisticstyle/artisticstyle.cpp b/src/plugins/beautifier/artisticstyle/artisticstyle.cpp index a8851b43cc5..04fda15a819 100644 --- a/src/plugins/beautifier/artisticstyle/artisticstyle.cpp +++ b/src/plugins/beautifier/artisticstyle/artisticstyle.cpp @@ -53,6 +53,7 @@ #include #include +#include using namespace TextEditor; @@ -150,10 +151,10 @@ Command ArtisticStyle::command(const QString &cfgFile) const command.addOption("-q"); command.addOption("--options=" + cfgFile); - const int version = m_settings.version(); - if (version > ArtisticStyleSettings::Version_2_03) { + const QVersionNumber version = m_settings.version(); + if (version > QVersionNumber(2, 3)) { command.setProcessing(Command::PipeProcessing); - if (version == ArtisticStyleSettings::Version_2_04) + if (version == QVersionNumber(2, 4)) command.setPipeAddsNewline(true); command.setReturnsCRLF(Utils::HostOsInfo::isWindowsHost()); command.addOption("-z2"); diff --git a/src/plugins/beautifier/artisticstyle/artisticstylesettings.cpp b/src/plugins/beautifier/artisticstyle/artisticstylesettings.cpp index a47fdda1011..5e7f8425cb9 100644 --- a/src/plugins/beautifier/artisticstyle/artisticstylesettings.cpp +++ b/src/plugins/beautifier/artisticstyle/artisticstylesettings.cpp @@ -57,9 +57,7 @@ const char SETTINGS_NAME[] = "artisticstyle"; ArtisticStyleSettings::ArtisticStyleSettings() : AbstractSettings(SETTINGS_NAME, ".astyle") { - connect(&m_versionWatcher, &QFutureWatcherBase::finished, - this, &ArtisticStyleSettings::helperSetVersion); - + setVersionRegExp(QRegularExpression("([2-9]{1})\\.([0-9]{1,2})(\\.[1-9]{1})?$")); setCommand("astyle"); m_settings.insert(USE_OTHER_FILES, QVariant(true)); m_settings.insert(USE_SPECIFIC_CONFIG_FILE, QVariant(false)); @@ -70,48 +68,6 @@ ArtisticStyleSettings::ArtisticStyleSettings() : read(); } -static int parseVersion(const QString &text) -{ - // The version in Artistic Style is printed like "Artistic Style Version 2.04" - const QRegularExpression rx("([2-9]{1})\\.([0-9]{1,2})(\\.[1-9]{1})?$"); - const QRegularExpressionMatch match = rx.match(text); - if (match.hasMatch()) { - const int major = match.captured(1).toInt() * 100; - const int minor = match.captured(2).toInt(); - return major + minor; - } - return 0; -} - -static int updateVersionHelper(const FilePath &command) -{ - QtcProcess process; - process.setCommand({command, {"--version"}}); - process.runBlocking(); - if (process.result() != ProcessResult::FinishedWithSuccess) - return 0; - - // Astyle prints the version on stdout or stderr, depending on platform - const int version = parseVersion(process.cleanedStdOut().trimmed()); - if (version != 0) - return version; - return parseVersion(process.cleanedStdErr().trimmed()); -} - -void ArtisticStyleSettings::updateVersion() -{ - if (m_versionFuture.isRunning()) - m_versionFuture.cancel(); - - m_versionFuture = runAsync(updateVersionHelper, command()); - m_versionWatcher.setFuture(m_versionFuture); -} - -void ArtisticStyleSettings::helperSetVersion() -{ - m_version = m_versionWatcher.result(); -} - bool ArtisticStyleSettings::useOtherFiles() const { return m_settings.value(USE_OTHER_FILES).toBool(); @@ -132,12 +88,12 @@ void ArtisticStyleSettings::setUseSpecificConfigFile(bool useSpecificConfigFile) m_settings.insert(USE_SPECIFIC_CONFIG_FILE, QVariant(useSpecificConfigFile)); } -Utils::FilePath ArtisticStyleSettings::specificConfigFile() const +FilePath ArtisticStyleSettings::specificConfigFile() const { - return Utils::FilePath::fromString(m_settings.value(SPECIFIC_CONFIG_FILE).toString()); + return FilePath::fromString(m_settings.value(SPECIFIC_CONFIG_FILE).toString()); } -void ArtisticStyleSettings::setSpecificConfigFile(const Utils::FilePath &specificConfigFile) +void ArtisticStyleSettings::setSpecificConfigFile(const FilePath &specificConfigFile) { m_settings.insert(SPECIFIC_CONFIG_FILE, QVariant(specificConfigFile.toString())); } @@ -182,7 +138,7 @@ QString ArtisticStyleSettings::documentationFilePath() const void ArtisticStyleSettings::createDocumentationFile() const { - Utils::QtcProcess process; + QtcProcess process; process.setTimeoutS(2); process.setCommand({command(), {"-h"}}); process.runBlocking(); diff --git a/src/plugins/beautifier/artisticstyle/artisticstylesettings.h b/src/plugins/beautifier/artisticstyle/artisticstylesettings.h index e0fa1b8866e..cf125810317 100644 --- a/src/plugins/beautifier/artisticstyle/artisticstylesettings.h +++ b/src/plugins/beautifier/artisticstyle/artisticstylesettings.h @@ -29,9 +29,6 @@ #include -#include -#include - namespace Beautifier { namespace Internal { @@ -40,15 +37,8 @@ class ArtisticStyleSettings : public AbstractSettings Q_OBJECT public: - enum ArtisticStyleVersion { - Version_2_03 = 203, - Version_2_04 = 204 - }; - ArtisticStyleSettings(); - void updateVersion() override; - bool useOtherFiles() const; void setUseOtherFiles(bool useOtherFiles); @@ -69,11 +59,6 @@ public: QString documentationFilePath() const override; void createDocumentationFile() const override; - -private: - void helperSetVersion(); - QFuture m_versionFuture; - QFutureWatcher m_versionWatcher; }; } // namespace Internal diff --git a/src/plugins/beautifier/uncrustify/uncrustify.cpp b/src/plugins/beautifier/uncrustify/uncrustify.cpp index 689ef3714a0..370a7f37a00 100644 --- a/src/plugins/beautifier/uncrustify/uncrustify.cpp +++ b/src/plugins/beautifier/uncrustify/uncrustify.cpp @@ -49,6 +49,7 @@ #include #include +#include using namespace TextEditor; @@ -180,7 +181,7 @@ Command Uncrustify::command(const QString &cfgFile, bool fragment) const Command command; command.setExecutable(m_settings.command().toString()); command.setProcessing(Command::PipeProcessing); - if (m_settings.version() >= 62) { + if (m_settings.version() >= QVersionNumber(0, 62)) { command.addOption("--assume"); command.addOption("%file"); } else { diff --git a/src/plugins/beautifier/uncrustify/uncrustifysettings.cpp b/src/plugins/beautifier/uncrustify/uncrustifysettings.cpp index 77b775bc9c3..fbc8bfe079b 100644 --- a/src/plugins/beautifier/uncrustify/uncrustifysettings.cpp +++ b/src/plugins/beautifier/uncrustify/uncrustifysettings.cpp @@ -30,7 +30,6 @@ #include "../beautifierconstants.h" #include - #include #include @@ -56,6 +55,7 @@ const char SETTINGS_NAME[] = "uncrustify"; UncrustifySettings::UncrustifySettings() : AbstractSettings(SETTINGS_NAME, ".cfg") { + setVersionRegExp(QRegularExpression("([0-9]{1})\\.([0-9]{2})")); setCommand("uncrustify"); m_settings.insert(USE_OTHER_FILES, QVariant(true)); m_settings.insert(USE_HOME_FILE, QVariant(false)); @@ -210,33 +210,5 @@ void UncrustifySettings::createDocumentationFile() const } } -static bool parseVersion(const QString &text, int &version) -{ - // The version in Uncrustify is printed like "uncrustify 0.62" - const QRegularExpression rx("([0-9]{1})\\.([0-9]{2})"); - const QRegularExpressionMatch match = rx.match(text); - if (!match.hasMatch()) - return false; - - const int major = match.captured(1).toInt() * 100; - const int minor = match.captured(2).toInt(); - version = major + minor; - return true; -} - -void UncrustifySettings::updateVersion() -{ - m_versionProcess.reset(new QtcProcess); - connect(m_versionProcess.get(), &QtcProcess::finished, this, [this] { - if (m_versionProcess->exitStatus() == QProcess::NormalExit) { - if (!parseVersion(QString::fromUtf8(m_versionProcess->readAllStandardOutput()), m_version)) - parseVersion(QString::fromUtf8(m_versionProcess->readAllStandardError()), m_version); - } - m_versionProcess.release()->deleteLater(); - }); - m_versionProcess->setCommand({ command(), { "--version" } }); - m_versionProcess->start(); -} - } // namespace Internal } // namespace Beautifier diff --git a/src/plugins/beautifier/uncrustify/uncrustifysettings.h b/src/plugins/beautifier/uncrustify/uncrustifysettings.h index 8c2db0ef2c4..e2cd9e99ec2 100644 --- a/src/plugins/beautifier/uncrustify/uncrustifysettings.h +++ b/src/plugins/beautifier/uncrustify/uncrustifysettings.h @@ -26,9 +26,6 @@ #pragma once #include "../abstractsettings.h" -#include - -namespace Utils { class QtcProcess; } namespace Beautifier { namespace Internal { @@ -59,16 +56,11 @@ public: QString documentationFilePath() const override; void createDocumentationFile() const override; - void updateVersion() override; - Utils::FilePath specificConfigFile() const; void setSpecificConfigFile(const Utils::FilePath &filePath); bool useSpecificConfigFile() const; void setUseSpecificConfigFile(bool useConfigFile); - -private: - std::unique_ptr m_versionProcess; }; } // namespace Internal