lxqt / libqtxdg

Qt implementation of freedesktop.org xdg specs
https://lxqt.github.io
GNU Lesser General Public License v2.1
72 stars 35 forks source link

QStringView is not supported by Qt5 5.12. #250

Closed darkshram closed 3 years ago

darkshram commented 3 years ago

Trying to build LXQt 0.17.0. Everything built perfectly in my Qt5 5.15 tree. But in my Qt5 5.12 tree fails. QStringView is not supported by Qt5 5.12.

Expected Behavior

Should build with Qt5 5.12? CMakeLists.txt says Qt5 5.12. should be supported.

Current Behavior

Does not build with Qt5 5.12.10.

Possible Solution

a) Add conditionals for QStringView and let QStringRef for older Qt5?

#if (QT_VERSION >= QT_VERSION_CHECK(5,13,0))
     QStringView code
# else
     QStringRef code
#endif

b) Officially drop support for Qt5 5.12 LTS?

System Information
tsujan commented 3 years ago

QStringView was introduced in Qt 5.10 (→ https://doc.qt.io/qt-5/qstringview.html) but, yes, some of its methods need 5.14 or 5.15.

Since we can't change the min. required version in a point release, version checks may be needed. But they could be ugly :(

QStringView was first used in https://github.com/lxqt/libqtxdg/commit/a7de9b2dc7d7b9c37443263884eb4d83d47f134a. @luis-pereira, what should we do? I don't have Qt < 5.15 anywhere.

darkshram commented 3 years ago

Made a patch to test, it built without warnings nor errors. Will try build the whole LXQt with Qt5 5.12 and then test if something broke because of this patch.

diff -Naur libqtxdg-3.7.0.orig/src/xdgiconloader/xdgiconloader.cpp libqtxdg-3.7.0/src/xdgiconloader/xdgiconloader.cpp
--- libqtxdg-3.7.0.orig/src/xdgiconloader/xdgiconloader.cpp 2021-04-05 02:19:04.000000000 -0500
+++ libqtxdg-3.7.0/src/xdgiconloader/xdgiconloader.cpp  2021-04-15 09:59:58.939803666 -0500
@@ -48,7 +48,9 @@
 #include <QtCore/QList>
 #include <QtCore/QDir>
 #include <QtCore/QSettings>
+#if (QT_VERSION >= QT_VERSION_CHECK(5,13,0))
 #include <QtCore/QStringView>
+#endif
 #include <QtGui/QPainter>
 #include <QImageReader>
 #include <QXmlStreamReader>
@@ -109,7 +111,11 @@
 {
 public:
     explicit QIconCacheGtkReader(const QString &themeDir);
+#if (QT_VERSION >= QT_VERSION_CHECK(5,13,0))
     QVector<const char *> lookup(QStringView);
+#else
+    QVector<const char *> lookup(const QStringRef &);
+#endif
     bool isValid() const { return m_isValid; }
     bool reValid(bool infoRefresh);
 private:
@@ -219,7 +225,11 @@
     with this name is present. The char* are pointers to the mapped data.
     For example, this would return { "32x32/apps", "24x24/apps" , ... }
  */
+#if (QT_VERSION >= QT_VERSION_CHECK(5,13,0))
 QVector<const char *> QIconCacheGtkReader::lookup(QStringView name)
+#else
+QVector<const char *> QIconCacheGtkReader::lookup(const QStringRef &name)
+#endif
 {
     QVector<const char *> ret;
     if (!isValid() || name.isEmpty())
@@ -400,8 +410,11 @@
     const QString pngext(QLatin1String(".png"));
     const QString xpmext(QLatin1String(".xpm"));

-
+#if (QT_VERSION >= QT_VERSION_CHECK(5,13,0))
     QStringView iconNameFallback(iconName);
+#else
+    QStringRef iconNameFallback(&iconName);
+#endif

     // Iterate through all icon's fallbacks in current theme
     if (info.entries.isEmpty()) {
tsujan commented 3 years ago

and then test if something broke because of this patch.

I don't think it'll break anything.

EDIT: I think the check should be done against Qt 5.14, not 5.13, but that doesn't make a difference in your case.

If you also find a compilation problem with Qt 5.12 in other LXQt components, please tell us!

darkshram commented 3 years ago

@tsujan Count on it. Libfm-qt has a build issue too. Will post it in libfm-qt issues.

tsujan commented 3 years ago

@darkshram Thanks! We needed your tests for point releases; I think none of us has Qt 5.12.

luis-pereira commented 3 years ago

@tsujan I still have 5.12.9 in one machine.

luis-pereira commented 3 years ago

@tsujan You are right, QStringView::lastIndexOf() was introduced in 5.14. grrrrrrrrrrrrr

luis-pereira commented 3 years ago

@darkshram Thanks.