Improve Version Parsing to handle more separators

launcher/Version.cpp

@@ -1,6 +1,6 @@
 #include "Version.h"
 
-#include <QStringList>
+#include <QDebug>
 #include <QUrl>
 #include <QRegularExpression>
 #include <QRegularExpressionMatch>
@@ -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;
@@ -74,12 +74,43 @@ bool Version::operator!=(const Version &other) const
 void Version::parse()
 {
     m_sections.clear();
+    QString currentSection;
 
-    // FIXME: this is bad. versions can contain a lot more separators...
-    QStringList parts = m_string.split('.');
+    auto classChange = [](QChar lastChar, QChar currentChar) {
+        return !lastChar.isNull() && ((!lastChar.isDigit() && currentChar.isDigit()) || (lastChar.isDigit() && !currentChar.isDigit()));
+    };
 
-    for (const auto& part : parts)
-    {
-        m_sections.append(Section(part));
+    for (int i = 0; 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()) {
+                m_sections.append(Section(currentSection));
+            }
+            currentSection = "";
+        }
+        currentSection += current_char;
+    }
+    if (!currentSection.isEmpty()) {
+        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: [ ";
+
+    bool first = true;
+    for (auto s : v.m_sections) {
+        if (!first) debug.nospace() << ", ";
+        debug.nospace() << s.m_fullString;
+        first = false;
+    }
+
+    debug.nospace() << " ]" << " }";
+
+    return debug;
+}

launcher/Version.h

@@ -35,6 +35,7 @@
 
 #pragma once
 
+#include <QDebug>
 #include <QString>
 #include <QStringView>
 #include <QList>
@@ -59,13 +60,16 @@ class Version
         return m_string;
     }
 
+    friend QDebug operator<<(QDebug debug, const Version& v);
+
 private:
     QString m_string;
     struct Section
     {
         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++)
             {
@@ -83,6 +87,7 @@ class Version
             if(numPart.size())
             {
                 numValid = true;
+                m_isNull = false;
                 m_numPart = numPart.toInt();
             }
 #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
@@ -92,6 +97,7 @@ class Version
 #endif
             if(stringPart.size())
             {
+                m_isNull = false;
                 m_stringPart = stringPart.toString();
             }
         }
@@ -100,9 +106,18 @@ class Version
         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) {
+                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;
@@ -113,7 +128,14 @@ class Version
             }
         }
         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)
@@ -129,6 +151,13 @@ class Version
         }
         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)
@@ -143,7 +172,11 @@ class Version
             }
         }
     };
+
+
     QList<Section> m_sections;
 
     void parse();
 };
+
+

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)

tests/Version_test.cpp

@@ -15,7 +15,6 @@
 
 #include <QTest>
 
-#include <TestUtil.h>
 #include <Version.h>
 
 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);