From 78b6876974d2cea087cb229257097052dad5fcf7 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 15 Feb 2022 11:05:05 +0100 Subject: [PATCH] Long live QColor::fromString()! It is customary for Qt types that can be constructed from string-ish to provide a fromString() named constructor. QColor didn't, relying instead on a set of overloaded implicit and explicit constructors. Add the named constructor, with the intent to deprecate the string-ish QColor constructors after a grace period. To prevent new users from using known-to-become-deprecated API, mark the old functions as \obsolete. Also rename isValidColor() to isValidColorName(). The only reason why these are lumped together in single commit is so that their docs can refer to each other instead of having to temporarily refer to obsolete API. [ChangeLog][QtGui][QColor] Added fromString() and isValidColorName(), both taking QAnyStringView. Task-number: QTBUG-101389 Change-Id: I2857c728257ad2f14c7c968b45547bdf07c44b63 Reviewed-by: Allan Sandfeld Jensen Reviewed-by: Qt CI Bot --- src/gui/painting/qcolor.cpp | 74 +++++++++++++++++-- src/gui/painting/qcolor.h | 14 +++- tests/auto/gui/painting/qcolor/tst_qcolor.cpp | 52 +++++++------ 3 files changed, 109 insertions(+), 31 deletions(-) diff --git a/src/gui/painting/qcolor.cpp b/src/gui/painting/qcolor.cpp index 9c32aff330..653e56a3c8 100644 --- a/src/gui/painting/qcolor.cpp +++ b/src/gui/painting/qcolor.cpp @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2020 The Qt Company Ltd. +** Copyright (C) 2022 The Qt Company Ltd. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of the QtGui module of the Qt Toolkit. @@ -829,6 +829,8 @@ QColor::QColor(Spec spec) noexcept /*! \fn QColor::QColor(const QString &name) + \obsolete + Constructs a named color in the same way as setNamedColor() using the given \a name. @@ -840,6 +842,8 @@ QColor::QColor(Spec spec) noexcept /*! \fn QColor::QColor(const char *name) + \obsolete + Constructs a named color in the same way as setNamedColor() using the given \a name. @@ -850,6 +854,8 @@ QColor::QColor(Spec spec) noexcept /*! \fn QColor::QColor(QLatin1String name) + \obsolete + Constructs a named color in the same way as setNamedColor() using the given \a name. @@ -885,6 +891,8 @@ QString QColor::name(NameFormat format) const } /*! + \obsolete + Sets the RGB value of this QColor to \a name, which may be in one of these formats: @@ -910,32 +918,36 @@ QString QColor::name(NameFormat format) const void QColor::setNamedColor(const QString &name) { - setColorFromString(qToStringViewIgnoringNull(name)); + *this = fromString(qToAnyStringViewIgnoringNull(name)); } /*! \overload \since 5.10 + \obsolete */ void QColor::setNamedColor(QStringView name) { - setColorFromString(name); + *this = fromString(name); } /*! \overload \since 5.8 + \obsolete */ void QColor::setNamedColor(QLatin1String name) { - setColorFromString(name); + *this = fromString(name); } /*! \since 4.7 + \obsolete + Returns \c true if the \a name is a valid color name and can be used to construct a valid QColor object, otherwise returns false. @@ -946,27 +958,77 @@ void QColor::setNamedColor(QLatin1String name) */ bool QColor::isValidColor(const QString &name) { - return isValidColor(qToStringViewIgnoringNull(name)); + return isValidColorName(qToAnyStringViewIgnoringNull(name)); } /*! \overload \since 5.10 + \obsolete */ bool QColor::isValidColor(QStringView name) noexcept { - return name.size() && QColor().setColorFromString(name); + return isValidColorName(name); } /*! \overload \since 5.8 + \obsolete */ bool QColor::isValidColor(QLatin1String name) noexcept +{ + return isValidColorName(name); +} + +/*! + \since 6.4 + + Returns \c true if the \a name is a valid color name and can + be used to construct a valid QColor object, otherwise returns + false. + + It uses the same algorithm used in fromString(). + + \sa fromString() +*/ +bool QColor::isValidColorName(QAnyStringView name) noexcept { return name.size() && QColor().setColorFromString(name); } +/*! + \since 6.4 + + Returns an RGB QColor parsed from \a name, which may be in one + of these formats: + + \list + \li #RGB (each of R, G, and B is a single hex digit) + \li #RRGGBB + \li #AARRGGBB (Since 5.2) + \li #RRRGGGBBB + \li #RRRRGGGGBBBB + \li A name from the list of colors defined in the list of + \l{https://www.w3.org/TR/SVG11/types.html#ColorKeywords}{SVG color keyword names} + provided by the World Wide Web Consortium; for example, "steelblue" or "gainsboro". + These color names work on all platforms. Note that these color names are \e not the + same as defined by the Qt::GlobalColor enums, e.g. "green" and Qt::green does not + refer to the same color. + \li \c transparent - representing the absence of a color. + \endlist + + Returns an invalid color if \a name cannot be parsed. + + \sa isValidColorName() +*/ +QColor QColor::fromString(QAnyStringView name) noexcept +{ + QColor c; + c.setColorFromString(name); + return c; +} + bool QColor::setColorFromString(QAnyStringView name) noexcept { if (!name.size()) { diff --git a/src/gui/painting/qcolor.h b/src/gui/painting/qcolor.h index 5898f79f24..a2ff932460 100644 --- a/src/gui/painting/qcolor.h +++ b/src/gui/painting/qcolor.h @@ -85,10 +85,12 @@ public: QColor(QRgba64 rgba64) noexcept; inline QColor(const QString& name); explicit inline QColor(QStringView name); - inline QColor(const char *aname) : QColor(QLatin1String(aname)) {} + inline QColor(const char *aname); inline QColor(QLatin1String name); QColor(Spec spec) noexcept; + static QColor fromString(QAnyStringView name) noexcept; + QColor &operator=(Qt::GlobalColor color) noexcept; bool isValid() const noexcept; @@ -224,6 +226,7 @@ public: static bool isValidColor(const QString &name); static bool isValidColor(QStringView) noexcept; static bool isValidColor(QLatin1String) noexcept; + static bool isValidColorName(QAnyStringView) noexcept; private: @@ -295,13 +298,16 @@ public: // can't give friendship to a namespace, so it needs to be public Q_DECLARE_TYPEINFO(QColor, Q_RELOCATABLE_TYPE); inline QColor::QColor(QLatin1String aname) -{ setNamedColor(aname); } + : QColor(fromString(aname)) {} inline QColor::QColor(QStringView aname) -{ setNamedColor(aname); } + : QColor(fromString(aname)) {} inline QColor::QColor(const QString& aname) -{ setNamedColor(aname); } + : QColor(fromString(aname)) {} + +inline QColor::QColor(const char *aname) + : QColor(fromString(aname)) {} inline bool QColor::isValid() const noexcept { return cspec != Invalid; } diff --git a/tests/auto/gui/painting/qcolor/tst_qcolor.cpp b/tests/auto/gui/painting/qcolor/tst_qcolor.cpp index 5c93a41b71..47e684b55c 100644 --- a/tests/auto/gui/painting/qcolor/tst_qcolor.cpp +++ b/tests/auto/gui/painting/qcolor/tst_qcolor.cpp @@ -54,8 +54,8 @@ private slots: void name(); void namehex_data(); void namehex(); - void setNamedColor_data(); - void setNamedColor(); + void fromString_data(); + void fromString(); void constructNamedColorWithSpace(); @@ -243,13 +243,13 @@ void tst_QColor::isValid_data() QTest::newRow("defaultConstructor") << QColor() << false; QTest::newRow("rgbConstructor-valid") << QColor(2,5,7) << true; QTest::newRow("rgbConstructor-invalid") << QColor(2,5,999) << false; - QTest::newRow("nameQStringConstructor-valid") << QColor(QString("#ffffff")) << true; - QTest::newRow("nameQStringConstructor-invalid") << QColor(QString("#ffffgg")) << false; - QTest::newRow("nameQStringConstructor-empty") << QColor(QString("")) << false; - QTest::newRow("nameQStringConstructor-named") << QColor(QString("red")) << true; - QTest::newRow("nameCharConstructor-valid") << QColor("#ffffff") << true; - QTest::newRow("nameCharConstructor-invalid") << QColor("#ffffgg") << false; - QTest::newRow("nameCharConstructor-invalid-2") << QColor("#fffffg") << false; + QTest::newRow("nameQStringConstructor-valid") << QColor::fromString("#ffffff") << true; + QTest::newRow("nameQStringConstructor-invalid") << QColor::fromString("#ffffgg") << false; + QTest::newRow("nameQStringConstructor-empty") << QColor::fromString("") << false; + QTest::newRow("nameQStringConstructor-named") << QColor::fromString("red") << true; + QTest::newRow("nameCharConstructor-valid") << QColor::fromString("#ffffff") << true; + QTest::newRow("nameCharConstructor-invalid") << QColor::fromString("#ffffgg") << false; + QTest::newRow("nameCharConstructor-invalid-2") << QColor::fromString("#fffffg") << false; } void tst_QColor::isValid() @@ -336,6 +336,7 @@ void tst_QColor::namehex() QFETCH(QString, hexcolor); QFETCH(QColor, color); QCOMPARE(QColor(hexcolor), color); + QCOMPARE(QColor::fromString(hexcolor), color); } void tst_QColor::globalColors_data() @@ -721,25 +722,27 @@ static const int rgbTblSize = sizeof(rgbTbl) / sizeof(RGBData); #undef rgb -void tst_QColor::setNamedColor_data() +void tst_QColor::fromString_data() { QTest::addColumn("byCtor"); QTest::addColumn("bySetNamedColor"); + QTest::addColumn("byFromString"); QTest::addColumn("expected"); for (const auto e : rgbTbl) { QColor expected; expected.setRgba(e.value); -#define ROW(expr) \ - do { \ - QColor bySetNamedColor; \ - bySetNamedColor.setNamedColor(expr); \ - auto byCtor = QColor(expr); \ - QTest::addRow("%s: %s", e.name, #expr) \ - << byCtor << bySetNamedColor << expected; \ - } while (0) \ - /*end*/ +#define ROW(expr) row(expr, #expr) + auto row = [&] (auto expr, const char *exprS) { + QColor bySetNamedColor; + bySetNamedColor.setNamedColor(expr); + auto byCtor = QColor(expr); + QTest::addRow("%s: %s", e.name, exprS) + << byCtor << bySetNamedColor + << QColor::fromString(expr) + << expected; + }; const auto l1 = QLatin1String(e.name); const auto l1UpperBA = QByteArray(e.name).toUpper(); @@ -766,14 +769,16 @@ void tst_QColor::setNamedColor_data() } } -void tst_QColor::setNamedColor() +void tst_QColor::fromString() { QFETCH(QColor, byCtor); QFETCH(QColor, bySetNamedColor); + QFETCH(QColor, byFromString); QFETCH(QColor, expected); QCOMPARE(byCtor, expected); QCOMPARE(bySetNamedColor, expected); + QCOMPARE(byFromString, expected); } @@ -781,14 +786,19 @@ void tst_QColor::constructNamedColorWithSpace() { QColor whiteSmoke("white smoke"); QCOMPARE(whiteSmoke, QColor(245, 245, 245)); + QCOMPARE(QColor::fromString("white smoke"), QColorConstants::Svg::whitesmoke); } void tst_QColor::colorNames() { - QStringList all = QColor::colorNames(); + const QStringList all = QColor::colorNames(); QCOMPARE(all.size(), rgbTblSize); for (int i = 0; i < all.size(); ++i) QCOMPARE(all.at(i), QLatin1String(rgbTbl[i].name)); + for (const QString &name : all) + QVERIFY(QColor::isValidColorName(name)); + for (const auto &e : rgbTbl) + QVERIFY(QColor::isValidColorName(e.name)); } void tst_QColor::spec()