From c0c3892064a775b13fd5cae00f58b43bee062003 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgars=20C=C4=ABrulis?= Date: Sun, 15 Jan 2023 09:47:31 +0200 Subject: [PATCH 01/15] Version.cpp: Improve version parsing to handle mixed numeric and alphabetic characters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Edgars Cīrulis --- launcher/Version.cpp | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/launcher/Version.cpp b/launcher/Version.cpp index b9090e299..5d814a25d 100644 --- a/launcher/Version.cpp +++ b/launcher/Version.cpp @@ -74,12 +74,36 @@ bool Version::operator!=(const Version &other) const void Version::parse() { m_sections.clear(); - - // FIXME: this is bad. versions can contain a lot more separators... - QStringList parts = m_string.split('.'); - - for (const auto& part : parts) - { - m_sections.append(Section(part)); + QString currentSection; + bool lastCharWasDigit = false; + for (int i = 0; i < m_string.size(); ++i) { + if(m_string[i].isDigit()){ + if(!lastCharWasDigit){ + if(!currentSection.isEmpty()){ + m_sections.append(Section(currentSection)); + } + currentSection = ""; + } + currentSection += m_string[i]; + lastCharWasDigit = true; + }else if(m_string[i].isLetter()){ + if(lastCharWasDigit){ + if(!currentSection.isEmpty()){ + m_sections.append(Section(currentSection)); + } + currentSection = ""; + } + currentSection += m_string[i]; + lastCharWasDigit = false; + } + else if(m_string[i] == '-' || m_string[i] == '_'){ + if(!currentSection.isEmpty()){ + m_sections.append(Section(currentSection)); + } + currentSection = ""; + } + } + if (!currentSection.isEmpty()) { + m_sections.append(Section(currentSection)); } } From 6fb837c529ce838efabd1899a5803c124013fbf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgars=20C=C4=ABrulis?= Date: Sun, 15 Jan 2023 13:18:13 +0200 Subject: [PATCH 02/15] Version.cpp: Add version string parser to split on '.' character MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Edgars Cīrulis --- launcher/Version.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launcher/Version.cpp b/launcher/Version.cpp index 5d814a25d..9481716d2 100644 --- a/launcher/Version.cpp +++ b/launcher/Version.cpp @@ -96,7 +96,7 @@ void Version::parse() currentSection += m_string[i]; lastCharWasDigit = false; } - else if(m_string[i] == '-' || m_string[i] == '_'){ + else if(m_string[i] == '.' || m_string[i] == '-' || m_string[i] == '_'){ if(!currentSection.isEmpty()){ m_sections.append(Section(currentSection)); } From de11017552a5e3e06c436051b2218c4411a0fb24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgars=20C=C4=ABrulis?= Date: Sun, 15 Jan 2023 14:30:18 +0200 Subject: [PATCH 03/15] Version.cpp: Use anonymous function to eliminate code duplication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Edgars Cīrulis --- launcher/Version.cpp | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/launcher/Version.cpp b/launcher/Version.cpp index 9481716d2..f61d53e80 100644 --- a/launcher/Version.cpp +++ b/launcher/Version.cpp @@ -75,26 +75,18 @@ void Version::parse() { m_sections.clear(); QString currentSection; - bool lastCharWasDigit = false; + auto classChange = [] (QChar lastChar, QChar currentChar) { + return (( lastChar.isLetter() && currentChar.isDigit() ) || (lastChar.isDigit() && currentChar.isLetter()) ); + }; for (int i = 0; i < m_string.size(); ++i) { - if(m_string[i].isDigit()){ - if(!lastCharWasDigit){ + if(m_string[i].isDigit() || m_string[i].isLetter()){ + if(i>0 && classChange(m_string[i-1], m_string[i])){ if(!currentSection.isEmpty()){ m_sections.append(Section(currentSection)); } currentSection = ""; } currentSection += m_string[i]; - lastCharWasDigit = true; - }else if(m_string[i].isLetter()){ - if(lastCharWasDigit){ - if(!currentSection.isEmpty()){ - m_sections.append(Section(currentSection)); - } - currentSection = ""; - } - currentSection += m_string[i]; - lastCharWasDigit = false; } else if(m_string[i] == '.' || m_string[i] == '-' || m_string[i] == '_'){ if(!currentSection.isEmpty()){ From 198139feb4546fbe819b7076c1689582ea67caa7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgars=20C=C4=ABrulis?= Date: Sun, 15 Jan 2023 17:07:44 +0200 Subject: [PATCH 04/15] Version.cpp: Simplify Version::parse by using const auto& current_char MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Edgars Cīrulis --- launcher/Version.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/launcher/Version.cpp b/launcher/Version.cpp index f61d53e80..73be60587 100644 --- a/launcher/Version.cpp +++ b/launcher/Version.cpp @@ -79,16 +79,17 @@ void Version::parse() return (( lastChar.isLetter() && currentChar.isDigit() ) || (lastChar.isDigit() && currentChar.isLetter()) ); }; for (int i = 0; i < m_string.size(); ++i) { - if(m_string[i].isDigit() || m_string[i].isLetter()){ - if(i>0 && classChange(m_string[i-1], m_string[i])){ + const auto& current_char = m_string.at(i); + if(current_char.isDigit() || current_char.isLetter()){ + if(i>0 && classChange(m_string.at(i-1), current_char)){ if(!currentSection.isEmpty()){ m_sections.append(Section(currentSection)); } currentSection = ""; } - currentSection += m_string[i]; + currentSection += current_char; } - else if(m_string[i] == '.' || m_string[i] == '-' || m_string[i] == '_'){ + else if(current_char == '.' || current_char == '-' || current_char == '_'){ if(!currentSection.isEmpty()){ m_sections.append(Section(currentSection)); } From a84e4b0e07dbcb736d92e98a3beca9025c981686 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgars=20C=C4=ABrulis?= Date: Sun, 15 Jan 2023 17:44:17 +0200 Subject: [PATCH 05/15] Version.cpp: Format parse function code using clang-format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Edgars Cīrulis --- launcher/Version.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/launcher/Version.cpp b/launcher/Version.cpp index 73be60587..0640e6d3c 100644 --- a/launcher/Version.cpp +++ b/launcher/Version.cpp @@ -75,22 +75,21 @@ void Version::parse() { m_sections.clear(); QString currentSection; - auto classChange = [] (QChar lastChar, QChar currentChar) { - return (( lastChar.isLetter() && currentChar.isDigit() ) || (lastChar.isDigit() && currentChar.isLetter()) ); + auto classChange = [](QChar lastChar, QChar currentChar) { + return ((lastChar.isLetter() && currentChar.isDigit()) || (lastChar.isDigit() && currentChar.isLetter())); }; for (int i = 0; i < m_string.size(); ++i) { const auto& current_char = m_string.at(i); - if(current_char.isDigit() || current_char.isLetter()){ - if(i>0 && classChange(m_string.at(i-1), current_char)){ - if(!currentSection.isEmpty()){ + if (current_char.isDigit() || current_char.isLetter()) { + if (i > 0 && classChange(m_string.at(i - 1), current_char)) { + if (!currentSection.isEmpty()) { m_sections.append(Section(currentSection)); } currentSection = ""; } currentSection += current_char; - } - else if(current_char == '.' || current_char == '-' || current_char == '_'){ - if(!currentSection.isEmpty()){ + } else if (current_char == '.' || current_char == '-' || current_char == '_') { + if (!currentSection.isEmpty()) { m_sections.append(Section(currentSection)); } currentSection = ""; From 3bec4a80b3de58d31992eda8497a3d099190b92d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgars=20C=C4=ABrulis?= Date: Tue, 17 Jan 2023 06:53:01 +0200 Subject: [PATCH 06/15] Version.cpp: Decompose version strings according to flexver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Rachel Powers <508861+Ryex@users.noreply.github.com> Signed-off-by: Edgars Cīrulis --- launcher/Version.cpp | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/launcher/Version.cpp b/launcher/Version.cpp index 0640e6d3c..01f513e34 100644 --- a/launcher/Version.cpp +++ b/launcher/Version.cpp @@ -75,25 +75,20 @@ void Version::parse() { m_sections.clear(); QString currentSection; + auto classChange = [](QChar lastChar, QChar currentChar) { - return ((lastChar.isLetter() && currentChar.isDigit()) || (lastChar.isDigit() && currentChar.isLetter())); + return !lastChar.isNull() && ((!lastChar.isDigit() && currentChar.isDigit()) || (lastChar.isDigit() && !currentChar.isDigit())); }; + for (int i = 0; i < m_string.size(); ++i) { const auto& current_char = m_string.at(i); - if (current_char.isDigit() || current_char.isLetter()) { - if (i > 0 && classChange(m_string.at(i - 1), current_char)) { - if (!currentSection.isEmpty()) { - m_sections.append(Section(currentSection)); - } - currentSection = ""; - } - currentSection += current_char; - } else if (current_char == '.' || current_char == '-' || current_char == '_') { + if ((i > 0 && classChange(m_string.at(i - 1), current_char)) || current_char == '.' || current_char == '-' || current_char == '+') { if (!currentSection.isEmpty()) { m_sections.append(Section(currentSection)); } currentSection = ""; } + currentSection += current_char; } if (!currentSection.isEmpty()) { m_sections.append(Section(currentSection)); From 730f714e973eadf76d2f834a9e062ce5bb44e41f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgars=20C=C4=ABrulis?= Date: Tue, 17 Jan 2023 07:33:36 +0200 Subject: [PATCH 07/15] Version.cpp: Remove unnecessary QStringList include MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Edgars Cīrulis --- launcher/Version.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/launcher/Version.cpp b/launcher/Version.cpp index 01f513e34..9fdd955b7 100644 --- a/launcher/Version.cpp +++ b/launcher/Version.cpp @@ -1,6 +1,5 @@ #include "Version.h" -#include #include #include #include From 9934537e19c7ce6f9bf926cc8abba023297b0a40 Mon Sep 17 00:00:00 2001 From: Rachel Powers <508861+Ryex@users.noreply.github.com> Date: Thu, 19 Jan 2023 09:46:35 +0200 Subject: [PATCH 08/15] feat: add debug printing for Version Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com> --- launcher/Version.cpp | 18 ++++++++++++++++++ launcher/Version.h | 7 +++++++ tests/CMakeLists.txt | 3 +++ tests/Version_test.cpp | 3 ++- 4 files changed, 30 insertions(+), 1 deletion(-) diff --git a/launcher/Version.cpp b/launcher/Version.cpp index 9fdd955b7..2129ebfd7 100644 --- a/launcher/Version.cpp +++ b/launcher/Version.cpp @@ -1,5 +1,6 @@ #include "Version.h" +#include #include #include #include @@ -93,3 +94,20 @@ void Version::parse() m_sections.append(Section(currentSection)); } } + + +/// qDebug print support for the BlockedMod struct +QDebug operator<<(QDebug debug, const Version& v) +{ + QDebugStateSaver saver(debug); + + debug.nospace() << "Version{ string: " << v.toString() << ", sections: [ "; + + for (auto s : v.m_sections) { + debug.nospace() << s.m_fullString << ", "; + } + + debug.nospace() << " ]" << " }"; + + return debug; +} \ No newline at end of file diff --git a/launcher/Version.h b/launcher/Version.h index aceb7a073..c09273744 100644 --- a/launcher/Version.h +++ b/launcher/Version.h @@ -35,6 +35,7 @@ #pragma once +#include #include #include #include @@ -59,6 +60,8 @@ public: return m_string; } + friend QDebug operator<<(QDebug debug, const Version& v); + private: QString m_string; struct Section @@ -143,7 +146,11 @@ private: } } }; + + QList
m_sections; void parse(); }; + + diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9f84a9a7b..0f716a751 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -53,3 +53,6 @@ ecm_add_test(Packwiz_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR ecm_add_test(Index_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test TEST_NAME Index) + +ecm_add_test(Version_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test + TEST_NAME Version) diff --git a/tests/Version_test.cpp b/tests/Version_test.cpp index 734528b7e..6836b6fa0 100644 --- a/tests/Version_test.cpp +++ b/tests/Version_test.cpp @@ -15,7 +15,6 @@ #include -#include #include class ModUtilsTest : public QObject @@ -74,6 +73,8 @@ private slots: const auto v1 = Version(first); const auto v2 = Version(second); + qDebug() << v1 << "vs" << v2; + QCOMPARE(v1 < v2, lessThan); QCOMPARE(v1 > v2, !lessThan && !equal); QCOMPARE(v1 == v2, equal); From 7ed993b54e20d74c000a29720bc9317ad4849ed0 Mon Sep 17 00:00:00 2001 From: Rachel Powers <508861+Ryex@users.noreply.github.com> Date: Wed, 18 Jan 2023 10:11:53 -0700 Subject: [PATCH 09/15] fix: proper null padded version comparison Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com> --- launcher/Version.cpp | 17 ++++++++++------- launcher/Version.h | 27 ++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/launcher/Version.cpp b/launcher/Version.cpp index 2129ebfd7..d59339e7b 100644 --- a/launcher/Version.cpp +++ b/launcher/Version.cpp @@ -15,9 +15,9 @@ bool Version::operator<(const Version &other) const const int size = qMax(m_sections.size(), other.m_sections.size()); for (int i = 0; i < size; ++i) { - const Section sec1 = (i >= m_sections.size()) ? Section("0") : m_sections.at(i); + const Section sec1 = (i >= m_sections.size()) ? Section("") : m_sections.at(i); const Section sec2 = - (i >= other.m_sections.size()) ? Section("0") : other.m_sections.at(i); + (i >= other.m_sections.size()) ? Section("") : other.m_sections.at(i); if (sec1 != sec2) { return sec1 < sec2; @@ -35,9 +35,9 @@ bool Version::operator>(const Version &other) const const int size = qMax(m_sections.size(), other.m_sections.size()); for (int i = 0; i < size; ++i) { - const Section sec1 = (i >= m_sections.size()) ? Section("0") : m_sections.at(i); + const Section sec1 = (i >= m_sections.size()) ? Section("") : m_sections.at(i); const Section sec2 = - (i >= other.m_sections.size()) ? Section("0") : other.m_sections.at(i); + (i >= other.m_sections.size()) ? Section("") : other.m_sections.at(i); if (sec1 != sec2) { return sec1 > sec2; @@ -55,9 +55,9 @@ bool Version::operator==(const Version &other) const const int size = qMax(m_sections.size(), other.m_sections.size()); for (int i = 0; i < size; ++i) { - const Section sec1 = (i >= m_sections.size()) ? Section("0") : m_sections.at(i); + const Section sec1 = (i >= m_sections.size()) ? Section("") : m_sections.at(i); const Section sec2 = - (i >= other.m_sections.size()) ? Section("0") : other.m_sections.at(i); + (i >= other.m_sections.size()) ? Section("") : other.m_sections.at(i); if (sec1 != sec2) { return false; @@ -103,8 +103,11 @@ QDebug operator<<(QDebug debug, const Version& v) debug.nospace() << "Version{ string: " << v.toString() << ", sections: [ "; + bool first = true; for (auto s : v.m_sections) { - debug.nospace() << s.m_fullString << ", "; + if (!first) debug.nospace() << ", "; + debug.nospace() << s.m_fullString; + first = false; } debug.nospace() << " ]" << " }"; diff --git a/launcher/Version.h b/launcher/Version.h index c09273744..1f1bea833 100644 --- a/launcher/Version.h +++ b/launcher/Version.h @@ -69,6 +69,7 @@ private: explicit Section(const QString &fullString) { m_fullString = fullString; + m_isNull = true; int cutoff = m_fullString.size(); for(int i = 0; i < m_fullString.size(); i++) { @@ -86,6 +87,7 @@ private: if(numPart.size()) { numValid = true; + m_isNull = false; m_numPart = numPart.toInt(); } #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) @@ -95,6 +97,7 @@ private: #endif if(stringPart.size()) { + m_isNull = false; m_stringPart = stringPart.toString(); } } @@ -103,9 +106,17 @@ private: int m_numPart = 0; QString m_stringPart; QString m_fullString; + bool m_isNull; inline bool operator!=(const Section &other) const { + if (m_isNull && other.numValid) { + return 0 != other.m_numPart; + } else if (numValid && other.m_isNull) { + return m_numPart != 0; + } else if (m_isNull || other.m_isNull) { + return false; + } if(numValid && other.numValid) { return m_numPart != other.m_numPart || m_stringPart != other.m_stringPart; @@ -116,7 +127,14 @@ private: } } inline bool operator<(const Section &other) const - { + { + if (m_isNull && other.numValid) { + return 0 < other.m_numPart; + } else if (numValid && other.m_isNull) { + return m_numPart < 0; + } else if (m_isNull || other.m_isNull) { + return true; + } if(numValid && other.numValid) { if(m_numPart < other.m_numPart) @@ -132,6 +150,13 @@ private: } inline bool operator>(const Section &other) const { + if (m_isNull && other.numValid) { + return 0 > other.m_numPart; + } else if (numValid && other.m_isNull) { + return m_numPart > 0; + } else if (m_isNull || other.m_isNull) { + return false; + } if(numValid && other.numValid) { if(m_numPart > other.m_numPart) From f49ad2ee03974c9fe94882d99d1a2bee67b87285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgars=20C=C4=ABrulis?= Date: Thu, 19 Jan 2023 10:39:57 +0200 Subject: [PATCH 10/15] Version.h: Fix comparison of null version in Version class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Rachel Powers <508861+Ryex@users.noreply.github.com> Signed-off-by: Edgars Cīrulis --- launcher/Version.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/launcher/Version.h b/launcher/Version.h index 1f1bea833..9db03521f 100644 --- a/launcher/Version.h +++ b/launcher/Version.h @@ -115,7 +115,8 @@ private: } else if (numValid && other.m_isNull) { return m_numPart != 0; } else if (m_isNull || other.m_isNull) { - return false; + if ((m_stringPart == ".") || (other.m_stringPart == ".")) return false; + return true; } if(numValid && other.numValid) { From 0199d8a74fbd76f3f37c02e4702dd5bed09fad93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgars=20C=C4=ABrulis?= Date: Thu, 19 Jan 2023 14:11:45 +0200 Subject: [PATCH 11/15] Version.cpp: Add new line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Edgars Cīrulis --- launcher/Version.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launcher/Version.cpp b/launcher/Version.cpp index d59339e7b..9b96f68e5 100644 --- a/launcher/Version.cpp +++ b/launcher/Version.cpp @@ -113,4 +113,4 @@ QDebug operator<<(QDebug debug, const Version& v) debug.nospace() << " ]" << " }"; return debug; -} \ No newline at end of file +} From 5ae69c079a15fa16945b306e29925e800cb28c87 Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 19 Jan 2023 21:18:39 -0300 Subject: [PATCH 12/15] feat(tests): add FlexVer test vector to the Version tests Signed-off-by: flow --- tests/Version_test.cpp | 103 ++++++++++++++++++++---- tests/testdata/Version/test_vectors.txt | 63 +++++++++++++++ 2 files changed, 152 insertions(+), 14 deletions(-) create mode 100644 tests/testdata/Version/test_vectors.txt diff --git a/tests/Version_test.cpp b/tests/Version_test.cpp index 6836b6fa0..bb0a7f5a6 100644 --- a/tests/Version_test.cpp +++ b/tests/Version_test.cpp @@ -17,15 +17,20 @@ #include -class ModUtilsTest : public QObject -{ +class VersionTest : public QObject { Q_OBJECT - void setupVersions() + + void addDataColumns() { QTest::addColumn("first"); QTest::addColumn("second"); QTest::addColumn("lessThan"); QTest::addColumn("equal"); + } + + void setupVersions() + { + addDataColumns(); QTest::newRow("equal, explicit") << "1.2.0" << "1.2.0" << false << true; QTest::newRow("equal, implicit 1") << "1.2" << "1.2.0" << false << true; @@ -49,20 +54,12 @@ class ModUtilsTest : public QObject QTest::newRow("greaterThan, two-digit") << "1.42" << "1.41" << false << false; } -private slots: - void initTestCase() - { - - } - void cleanupTestCase() - { - - } - + private slots: void test_versionCompare_data() { setupVersions(); } + void test_versionCompare() { QFETCH(QString, first); @@ -79,8 +76,86 @@ private slots: QCOMPARE(v1 > v2, !lessThan && !equal); QCOMPARE(v1 == v2, equal); } + + void test_flexVerTestVector_data() + { + addDataColumns(); + + QDir test_vector_dir(QFINDTESTDATA("testdata/Version")); + + QFile vector_file{test_vector_dir.absoluteFilePath("test_vectors.txt")}; + + vector_file.open(QFile::OpenModeFlag::ReadOnly); + + int test_number = 0; + const QString test_name_template { "FlexVer test #%1 (%2)" }; + for (auto line = vector_file.readLine(); !vector_file.atEnd(); line = vector_file.readLine()) { + line = line.simplified(); + if (line.startsWith('#') || line.isEmpty()) + continue; + + test_number += 1; + + auto split_line = line.split('<'); + if (split_line.size() == 2) { + QString first{split_line.first().simplified()}; + QString second{split_line.last().simplified()}; + + auto new_test_name = test_name_template.arg(QString::number(test_number), "lessThan").toLatin1().data(); + QTest::newRow(new_test_name) << first << second << true << false; + + continue; + } + + split_line = line.split('='); + if (split_line.size() == 2) { + QString first{split_line.first().simplified()}; + QString second{split_line.last().simplified()}; + + auto new_test_name = test_name_template.arg(QString::number(test_number), "equals").toLatin1().data(); + QTest::newRow(new_test_name) << first << second << false << true; + + continue; + } + + split_line = line.split('>'); + if (split_line.size() == 2) { + QString first{split_line.first().simplified()}; + QString second{split_line.last().simplified()}; + + auto new_test_name = test_name_template.arg(QString::number(test_number), "greaterThan").toLatin1().data(); + QTest::newRow(new_test_name) << first << second << false << false; + + continue; + } + + qCritical() << "Unexpected separator in the test vector: "; + qCritical() << line; + + QVERIFY(0 != 0); + } + + vector_file.close(); + } + + void test_flexVerTestVector() + { + QFETCH(QString, first); + QFETCH(QString, second); + QFETCH(bool, lessThan); + QFETCH(bool, equal); + + const auto v1 = Version(first); + const auto v2 = Version(second); + + qDebug() << v1 << "vs" << v2; + + QCOMPARE(v1 < v2, lessThan); + QCOMPARE(v1 > v2, !lessThan && !equal); + QCOMPARE(v1 == v2, equal); + } }; -QTEST_GUILESS_MAIN(ModUtilsTest) +QTEST_GUILESS_MAIN(VersionTest) #include "Version_test.moc" diff --git a/tests/testdata/Version/test_vectors.txt b/tests/testdata/Version/test_vectors.txt new file mode 100644 index 000000000..e6c6507cf --- /dev/null +++ b/tests/testdata/Version/test_vectors.txt @@ -0,0 +1,63 @@ +# Test vector from: +# https://github.com/unascribed/FlexVer/blob/704e12759b6e59220ff888f8bf2ec15b8f8fd969/test/test_vectors.txt +# +# This test file is formatted as " ", seperated by the space character +# Implementations should ignore lines starting with "#" and lines that have a length of 0 + +# Basic numeric ordering (lexical string sort fails these) +10 > 2 +100 > 10 + +# Trivial common numerics +1.0 < 1.1 +1.0 < 1.0.1 +1.1 > 1.0.1 + +# SemVer compatibility +1.5 > 1.5-pre1 +1.5 = 1.5+foobar + +# SemVer incompatibility +1.5 < 1.5-2 +1.5-pre10 > 1.5-pre2 + +# Empty strings + = +1 > + < 1 + +# Check boundary between textual and prerelease +a-a < a + +# Check boundary between textual and appendix +a+a = a + +# Dash is included in prerelease comparison (if stripped it will be a smaller component) +# Note that a-a < a=a regardless since the prerelease splits the component creating a smaller first component; 0 is added to force splitting regardless +a0-a < a0=a + +# Pre-releases must contain only non-digit +1.16.5-10 > 1.16.5 + +# Pre-releases can have multiple dashes (should not be split) +# Reasoning for test data: "p-a!" > "p-a-" (correct); "p-a!" < "p-a t-" (what happens if every dash creates a new component) +-a- > -a! + +# Misc +b1.7.3 > a1.2.6 +b1.2.6 > a1.7.3 +a1.1.2 < a1.1.2_01 +1.16.5-0.00.5 > 1.14.2-1.3.7 +1.0.0 < 1.0.0_01 +1.0.1 > 1.0.0_01 +1.0.0_01 < 1.0.1 +0.17.1-beta.1 < 0.17.1 +0.17.1-beta.1 < 0.17.1-beta.2 +1.4.5_01 = 1.4.5_01+fabric-1.17 +1.4.5_01 = 1.4.5_01+fabric-1.17+ohgod +14w16a < 18w40b +18w40a < 18w40b +1.4.5_01+fabric-1.17 < 18w40b +13w02a < c0.3.0_01 +0.6.0-1.18.x < 0.9.beta-1.18.x + From 81848e05f100a135ad1d307ccabb796be0540daa Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 19 Jan 2023 21:31:55 -0300 Subject: [PATCH 13/15] refactor: simplify Version operators Signed-off-by: flow --- launcher/Version.cpp | 66 +++++++++++++++++--------------------------- launcher/Version.h | 4 +-- 2 files changed, 28 insertions(+), 42 deletions(-) diff --git a/launcher/Version.cpp b/launcher/Version.cpp index 9b96f68e5..9307aab36 100644 --- a/launcher/Version.cpp +++ b/launcher/Version.cpp @@ -1,67 +1,41 @@ #include "Version.h" #include -#include #include #include +#include -Version::Version(const QString &str) : m_string(str) +Version::Version(QString str) : m_string(std::move(str)) { parse(); } -bool Version::operator<(const Version &other) const +bool Version::operator<(const Version& other) const { - const int size = qMax(m_sections.size(), other.m_sections.size()); - for (int i = 0; i < size; ++i) - { - const Section sec1 = (i >= m_sections.size()) ? Section("") : m_sections.at(i); + const auto size = qMax(m_sections.size(), other.m_sections.size()); + for (int i = 0; i < size; ++i) { + const Section sec1 = + (i >= m_sections.size()) ? Section("") : m_sections.at(i); const Section sec2 = (i >= other.m_sections.size()) ? Section("") : other.m_sections.at(i); + if (sec1 != sec2) - { return sec1 < sec2; - } } return false; } -bool Version::operator<=(const Version &other) const +bool Version::operator==(const Version& other) const { - return *this < other || *this == other; -} -bool Version::operator>(const Version &other) const -{ - const int size = qMax(m_sections.size(), other.m_sections.size()); - for (int i = 0; i < size; ++i) - { - const Section sec1 = (i >= m_sections.size()) ? Section("") : m_sections.at(i); + const auto size = qMax(m_sections.size(), other.m_sections.size()); + for (int i = 0; i < size; ++i) { + const Section sec1 = + (i >= m_sections.size()) ? Section("") : m_sections.at(i); const Section sec2 = (i >= other.m_sections.size()) ? Section("") : other.m_sections.at(i); - if (sec1 != sec2) - { - return sec1 > sec2; - } - } - return false; -} -bool Version::operator>=(const Version &other) const -{ - return *this > other || *this == other; -} -bool Version::operator==(const Version &other) const -{ - const int size = qMax(m_sections.size(), other.m_sections.size()); - for (int i = 0; i < size; ++i) - { - const Section sec1 = (i >= m_sections.size()) ? Section("") : m_sections.at(i); - const Section sec2 = - (i >= other.m_sections.size()) ? Section("") : other.m_sections.at(i); if (sec1 != sec2) - { return false; - } } return true; @@ -70,6 +44,18 @@ bool Version::operator!=(const Version &other) const { return !operator==(other); } +bool Version::operator<=(const Version &other) const +{ + return *this < other || *this == other; +} +bool Version::operator>(const Version &other) const +{ + return !(*this <= other); +} +bool Version::operator>=(const Version &other) const +{ + return !(*this < other); +} void Version::parse() { @@ -96,7 +82,7 @@ void Version::parse() } -/// qDebug print support for the BlockedMod struct +/// qDebug print support for the Version class QDebug operator<<(QDebug debug, const Version& v) { QDebugStateSaver saver(debug); diff --git a/launcher/Version.h b/launcher/Version.h index 9db03521f..b587319ab 100644 --- a/launcher/Version.h +++ b/launcher/Version.h @@ -45,8 +45,8 @@ class QUrl; class Version { public: - Version(const QString &str); - Version() {} + Version(QString str); + Version() = default; bool operator<(const Version &other) const; bool operator<=(const Version &other) const; From bcebb1920ff5df4f2a311984b296bfd8d5969997 Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 19 Jan 2023 21:59:33 -0300 Subject: [PATCH 14/15] refactor: clean up Section struct Signed-off-by: flow --- launcher/Version.h | 128 +++++++++++++++++++-------------------------- 1 file changed, 53 insertions(+), 75 deletions(-) diff --git a/launcher/Version.h b/launcher/Version.h index b587319ab..23481c29c 100644 --- a/launcher/Version.h +++ b/launcher/Version.h @@ -36,15 +36,14 @@ #pragma once #include +#include #include #include -#include class QUrl; -class Version -{ -public: +class Version { + public: Version(QString str); Version() = default; @@ -55,125 +54,104 @@ public: bool operator==(const Version &other) const; bool operator!=(const Version &other) const; - QString toString() const - { - return m_string; - } + QString toString() const { return m_string; } friend QDebug operator<<(QDebug debug, const Version& v); -private: - QString m_string; - struct Section - { - explicit Section(const QString &fullString) + private: + struct Section { + explicit Section(QString fullString) : m_isNull(true), m_fullString(std::move(fullString)) { - m_fullString = fullString; - m_isNull = true; int cutoff = m_fullString.size(); - for(int i = 0; i < m_fullString.size(); i++) - { - if(!m_fullString[i].isDigit()) - { + for (int i = 0; i < m_fullString.size(); i++) { + if (!m_fullString[i].isDigit()) { cutoff = i; break; } } + #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) auto numPart = QStringView{m_fullString}.left(cutoff); #else auto numPart = m_fullString.leftRef(cutoff); #endif - if(numPart.size()) - { - numValid = true; + + if (!numPart.isEmpty()) { m_isNull = false; m_numPart = numPart.toInt(); } + #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) auto stringPart = QStringView{m_fullString}.mid(cutoff); #else auto stringPart = m_fullString.midRef(cutoff); #endif - if(stringPart.size()) - { + + if (!stringPart.isEmpty()) { m_isNull = false; m_stringPart = stringPart.toString(); } } - explicit Section() {} - bool numValid = false; + + explicit Section() = default; + + bool m_isNull = false; int m_numPart = 0; + QString m_stringPart; QString m_fullString; - bool m_isNull; - inline bool operator!=(const Section &other) const + inline bool operator==(const Section& other) const { - if (m_isNull && other.numValid) { - return 0 != other.m_numPart; - } else if (numValid && other.m_isNull) { - return m_numPart != 0; - } else if (m_isNull || other.m_isNull) { - if ((m_stringPart == ".") || (other.m_stringPart == ".")) return false; - return true; - } - if(numValid && other.numValid) - { - return m_numPart != other.m_numPart || m_stringPart != other.m_stringPart; - } - else - { - return m_fullString != other.m_fullString; - } + if (m_isNull && !other.m_isNull) + return other.m_numPart == 0; + + if (!m_isNull && other.m_isNull) + return m_numPart == 0; + + if (m_isNull || other.m_isNull) + return (m_stringPart == ".") || (other.m_stringPart == "."); + + if (!m_isNull && !other.m_isNull) + return (m_numPart == other.m_numPart) && (m_stringPart == other.m_stringPart); + + return m_fullString == other.m_fullString; } + inline bool operator<(const Section &other) const { - if (m_isNull && other.numValid) { - return 0 < other.m_numPart; - } else if (numValid && other.m_isNull) { + if (m_isNull && !other.m_isNull) + return other.m_numPart > 0; + + if (!m_isNull && other.m_isNull) return m_numPart < 0; - } else if (m_isNull || other.m_isNull) { + + if (m_isNull || other.m_isNull) return true; - } - if(numValid && other.numValid) - { + + if (!m_isNull && !other.m_isNull) { if(m_numPart < other.m_numPart) return true; if(m_numPart == other.m_numPart && m_stringPart < other.m_stringPart) return true; return false; } - else - { - return m_fullString < other.m_fullString; - } + + return m_fullString < other.m_fullString; + } + + inline bool operator!=(const Section& other) const + { + return !(*this == other); } inline bool operator>(const Section &other) const { - if (m_isNull && other.numValid) { - return 0 > other.m_numPart; - } else if (numValid && other.m_isNull) { - return m_numPart > 0; - } else if (m_isNull || other.m_isNull) { - return false; - } - if(numValid && other.numValid) - { - if(m_numPart > other.m_numPart) - return true; - if(m_numPart == other.m_numPart && m_stringPart > other.m_stringPart) - return true; - return false; - } - else - { - return m_fullString > other.m_fullString; - } + return !(*this < other || *this == other); } }; - + private: + QString m_string; QList
m_sections; void parse(); From 445f9e5f717bf1ad9b764704b320bbec237a7682 Mon Sep 17 00:00:00 2001 From: flow Date: Fri, 20 Jan 2023 11:11:35 -0300 Subject: [PATCH 15/15] feat+fix(Version): make comparsion FlexVer-compatible ... and fixes a minor issue in the parsing. This changes the expected behavior of Versions in one significant way: Now, Versions like 1.2 or 1.5 evaluate to LESS THAN 1.2.0 and 1.5.0 respectively. This makes sense for sorting versions, since one expects the versions without patch release to 'contain' the ones with, so the ones without should be evaluated uniformily with the ones with the patch. Signed-off-by: flow --- launcher/Version.cpp | 94 +++++++++++++++++++++++++++--------------- launcher/Version.h | 66 ++++++++++++++++------------- tests/Version_test.cpp | 16 +++---- 3 files changed, 106 insertions(+), 70 deletions(-) diff --git a/launcher/Version.cpp b/launcher/Version.cpp index 9307aab36..e4311f314 100644 --- a/launcher/Version.cpp +++ b/launcher/Version.cpp @@ -10,49 +10,63 @@ Version::Version(QString str) : m_string(std::move(str)) parse(); } +#define VERSION_OPERATOR(return_on_different) \ + bool exclude_our_sections = false; \ + bool exclude_their_sections = false; \ + \ + const auto size = qMax(m_sections.size(), other.m_sections.size()); \ + for (int i = 0; i < size; ++i) { \ + Section sec1 = (i >= m_sections.size()) ? Section() : m_sections.at(i); \ + Section sec2 = (i >= other.m_sections.size()) ? Section() : other.m_sections.at(i); \ + \ + { /* Don't include appendixes in the comparison */ \ + if (sec1.isAppendix()) \ + exclude_our_sections = true; \ + if (sec2.isAppendix()) \ + exclude_their_sections = true; \ + \ + if (exclude_our_sections) { \ + sec1 = Section(); \ + if (sec2.m_isNull) \ + break; \ + } \ + \ + if (exclude_their_sections) { \ + sec2 = Section(); \ + if (sec1.m_isNull) \ + break; \ + } \ + } \ + \ + if (sec1 != sec2) \ + return return_on_different; \ + } + bool Version::operator<(const Version& other) const { - const auto size = qMax(m_sections.size(), other.m_sections.size()); - for (int i = 0; i < size; ++i) { - const Section sec1 = - (i >= m_sections.size()) ? Section("") : m_sections.at(i); - const Section sec2 = - (i >= other.m_sections.size()) ? Section("") : other.m_sections.at(i); - - if (sec1 != sec2) - return sec1 < sec2; - } + VERSION_OPERATOR(sec1 < sec2) return false; } bool Version::operator==(const Version& other) const { - const auto size = qMax(m_sections.size(), other.m_sections.size()); - for (int i = 0; i < size; ++i) { - const Section sec1 = - (i >= m_sections.size()) ? Section("") : m_sections.at(i); - const Section sec2 = - (i >= other.m_sections.size()) ? Section("") : other.m_sections.at(i); - - if (sec1 != sec2) - return false; - } + VERSION_OPERATOR(false) return true; } -bool Version::operator!=(const Version &other) const +bool Version::operator!=(const Version& other) const { return !operator==(other); } -bool Version::operator<=(const Version &other) const +bool Version::operator<=(const Version& other) const { return *this < other || *this == other; } -bool Version::operator>(const Version &other) const +bool Version::operator>(const Version& other) const { return !(*this <= other); } -bool Version::operator>=(const Version &other) const +bool Version::operator>=(const Version& other) const { return !(*this < other); } @@ -62,25 +76,37 @@ void Version::parse() m_sections.clear(); QString currentSection; - auto classChange = [](QChar lastChar, QChar currentChar) { - return !lastChar.isNull() && ((!lastChar.isDigit() && currentChar.isDigit()) || (lastChar.isDigit() && !currentChar.isDigit())); + if (m_string.isEmpty()) + return; + + auto classChange = [&](QChar lastChar, QChar currentChar) { + if (lastChar.isNull()) + return false; + if (lastChar.isDigit() != currentChar.isDigit()) + return true; + + const QList s_separators{ '.', '-', '+' }; + if (s_separators.contains(currentChar) && currentSection.at(0) != currentChar) + return true; + + return false; }; - for (int i = 0; i < m_string.size(); ++i) { + currentSection += m_string.at(0); + for (int i = 1; i < m_string.size(); ++i) { const auto& current_char = m_string.at(i); - if ((i > 0 && classChange(m_string.at(i - 1), current_char)) || current_char == '.' || current_char == '-' || current_char == '+') { - if (!currentSection.isEmpty()) { + if (classChange(m_string.at(i - 1), current_char)) { + if (!currentSection.isEmpty()) m_sections.append(Section(currentSection)); - } currentSection = ""; } + currentSection += current_char; } - if (!currentSection.isEmpty()) { - m_sections.append(Section(currentSection)); - } -} + if (!currentSection.isEmpty()) + m_sections.append(Section(currentSection)); +} /// qDebug print support for the Version class QDebug operator<<(QDebug debug, const Version& v) diff --git a/launcher/Version.h b/launcher/Version.h index 23481c29c..659f8e54e 100644 --- a/launcher/Version.h +++ b/launcher/Version.h @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-3.0-only /* * PolyMC - Minecraft Launcher + * Copyright (C) 2023 flowln * Copyright (C) 2022 Sefa Eyeoglu * * This program is free software: you can redistribute it and/or modify @@ -60,7 +61,7 @@ class Version { private: struct Section { - explicit Section(QString fullString) : m_isNull(true), m_fullString(std::move(fullString)) + explicit Section(QString fullString) : m_fullString(std::move(fullString)) { int cutoff = m_fullString.size(); for (int i = 0; i < m_fullString.size(); i++) { @@ -95,45 +96,54 @@ class Version { explicit Section() = default; - bool m_isNull = false; - int m_numPart = 0; + bool m_isNull = true; + int m_numPart = 0; QString m_stringPart; + QString m_fullString; + [[nodiscard]] inline bool isAppendix() const { return m_stringPart.startsWith('+'); } + [[nodiscard]] inline bool isPreRelease() const { return m_stringPart.startsWith('-') && m_stringPart.length() > 1; } + inline bool operator==(const Section& other) const { if (m_isNull && !other.m_isNull) - return other.m_numPart == 0; - + return false; if (!m_isNull && other.m_isNull) - return m_numPart == 0; - - if (m_isNull || other.m_isNull) - return (m_stringPart == ".") || (other.m_stringPart == "."); - - if (!m_isNull && !other.m_isNull) - return (m_numPart == other.m_numPart) && (m_stringPart == other.m_stringPart); - - return m_fullString == other.m_fullString; - } - - inline bool operator<(const Section &other) const - { - if (m_isNull && !other.m_isNull) - return other.m_numPart > 0; - - if (!m_isNull && other.m_isNull) - return m_numPart < 0; - - if (m_isNull || other.m_isNull) - return true; + return false; if (!m_isNull && !other.m_isNull) { - if(m_numPart < other.m_numPart) + return (m_numPart == other.m_numPart) && (m_stringPart == other.m_stringPart); + } + + return true; + } + + inline bool operator<(const Section& other) const + { + static auto unequal_is_less = [](Section const& non_null) -> bool { + if (non_null.m_stringPart.isEmpty()) + return non_null.m_numPart == 0; + return (non_null.m_stringPart != QLatin1Char('.')) && non_null.isPreRelease(); + }; + + if (!m_isNull && other.m_isNull) + return unequal_is_less(*this); + if (m_isNull && !other.m_isNull) + return !unequal_is_less(other); + + if (!m_isNull && !other.m_isNull) { + if (m_numPart < other.m_numPart) return true; - if(m_numPart == other.m_numPart && m_stringPart < other.m_stringPart) + if (m_numPart == other.m_numPart && m_stringPart < other.m_stringPart) return true; + + if (!m_stringPart.isEmpty() && other.m_stringPart.isEmpty()) + return false; + if (m_stringPart.isEmpty() && !other.m_stringPart.isEmpty()) + return true; + return false; } diff --git a/tests/Version_test.cpp b/tests/Version_test.cpp index bb0a7f5a6..afb4c6102 100644 --- a/tests/Version_test.cpp +++ b/tests/Version_test.cpp @@ -33,24 +33,24 @@ class VersionTest : public QObject { addDataColumns(); QTest::newRow("equal, explicit") << "1.2.0" << "1.2.0" << false << true; - QTest::newRow("equal, implicit 1") << "1.2" << "1.2.0" << false << true; - QTest::newRow("equal, implicit 2") << "1.2.0" << "1.2" << false << true; QTest::newRow("equal, two-digit") << "1.42" << "1.42" << false << true; QTest::newRow("lessThan, explicit 1") << "1.2.0" << "1.2.1" << true << false; QTest::newRow("lessThan, explicit 2") << "1.2.0" << "1.3.0" << true << false; QTest::newRow("lessThan, explicit 3") << "1.2.0" << "2.2.0" << true << false; - QTest::newRow("lessThan, implicit 1") << "1.2" << "1.2.1" << true << false; - QTest::newRow("lessThan, implicit 2") << "1.2" << "1.3.0" << true << false; - QTest::newRow("lessThan, implicit 3") << "1.2" << "2.2.0" << true << false; + QTest::newRow("lessThan, implicit 1") << "1.2" << "1.2.0" << true << false; + QTest::newRow("lessThan, implicit 2") << "1.2" << "1.2.1" << true << false; + QTest::newRow("lessThan, implicit 3") << "1.2" << "1.3.0" << true << false; + QTest::newRow("lessThan, implicit 4") << "1.2" << "2.2.0" << true << false; QTest::newRow("lessThan, two-digit") << "1.41" << "1.42" << true << false; QTest::newRow("greaterThan, explicit 1") << "1.2.1" << "1.2.0" << false << false; QTest::newRow("greaterThan, explicit 2") << "1.3.0" << "1.2.0" << false << false; QTest::newRow("greaterThan, explicit 3") << "2.2.0" << "1.2.0" << false << false; - QTest::newRow("greaterThan, implicit 1") << "1.2.1" << "1.2" << false << false; - QTest::newRow("greaterThan, implicit 2") << "1.3.0" << "1.2" << false << false; - QTest::newRow("greaterThan, implicit 3") << "2.2.0" << "1.2" << false << false; + QTest::newRow("greaterThan, implicit 1") << "1.2.0" << "1.2" << false << false; + QTest::newRow("greaterThan, implicit 2") << "1.2.1" << "1.2" << false << false; + QTest::newRow("greaterThan, implicit 3") << "1.3.0" << "1.2" << false << false; + QTest::newRow("greaterThan, implicit 4") << "2.2.0" << "1.2" << false << false; QTest::newRow("greaterThan, two-digit") << "1.42" << "1.41" << false << false; }