Merge branch 'colour-as-string'

Return empty string from wxColour::GetAsString() if it's invalid.

See https://github.com/wxWidgets/wxWidgets/pull/1947

interface/wx/colour.h

@@ -113,10 +113,11 @@ public:
         @c wxC2S_HTML_SYNTAX, to obtain the colour as "#" followed by 6
         hexadecimal digits (e.g. wxColour(255,0,0) == "#FF0000").
 
-        This function never fails and always returns a non-empty string but
+        This function returns empty string if the colour is not initialized
-        asserts if the colour has alpha channel (i.e. is non opaque) but
+        (see IsOk()). Otherwise, the returned string is always non-empty, but
-        @c wxC2S_CSS_SYNTAX (which is the only one supporting alpha) is not
+        the function asserts if the colour has alpha channel (i.e. is non
-        specified in flags.
+        opaque) but @c wxC2S_CSS_SYNTAX (which is the only one supporting
+        alpha) is not specified in @a flags.
 
         @note For non-solid (i.e. non-RGB) colour this function returns
         "rgb(??, ?? ??)" or "#??????".

src/common/colourcmn.cpp

@@ -218,6 +218,9 @@ bool wxColourBase::FromString(const wxString& str)
 
 wxString wxColourBase::GetAsString(long flags) const
 {
+    if ( !IsOk() )
+        return wxString();
+
     wxString colName;
 
     if ( IsSolid() )

tests/graphics/colour.cpp

@@ -20,99 +20,139 @@
 #include "asserthelper.h"
 
 // ----------------------------------------------------------------------------
-// helpers
+// helpers for checking wxColour RGB[A] values
 // ----------------------------------------------------------------------------
 
-// Check the colour components, with and without alpha.
+typedef wxColour::ChannelType ChannelType;
-//
-// NB: These are macros and not functions to have correct line numbers in case
-//     of failure.
-#define ASSERT_EQUAL_RGB(c, r, g, b) \
-    CPPUNIT_ASSERT_EQUAL( r, (int)c.Red() ); \
-    CPPUNIT_ASSERT_EQUAL( g, (int)c.Green() ); \
-    CPPUNIT_ASSERT_EQUAL( b, (int)c.Blue() )
 
-#define ASSERT_EQUAL_RGBA(c, r, g, b, a) \
+class ColourRGBMatcher : public Catch::MatcherBase<wxColour>
-    ASSERT_EQUAL_RGB(c, r, g, b); \
-    CPPUNIT_ASSERT_EQUAL( a, (int)c.Alpha() )
-
-// ----------------------------------------------------------------------------
-// test class
-// ----------------------------------------------------------------------------
-
-class ColourTestCase : public CppUnit::TestCase
 {
 public:
-    ColourTestCase() { }
+    ColourRGBMatcher(ChannelType red, ChannelType green, ChannelType blue)
+        : m_red(red),
+          m_green(green),
+          m_blue(blue)
+    {
+    }
 
-private:
+    bool match(const wxColour& c) const wxOVERRIDE
-    CPPUNIT_TEST_SUITE( ColourTestCase );
+    {
-        CPPUNIT_TEST( GetSetRGB );
+        return c.Red() == m_red && c.Green() == m_green && c.Blue() == m_blue;
-        CPPUNIT_TEST( FromString );
+    }
-    CPPUNIT_TEST_SUITE_END();
 
-    void GetSetRGB();
+    std::string describe() const wxOVERRIDE
-    void FromString();
+    {
+        return wxString::Format("!= RGB(%#02x, %#02x, %#02x)",
+                                m_red, m_green, m_blue).ToStdString();
+    }
 
-    wxDECLARE_NO_COPY_CLASS(ColourTestCase);
+protected:
+    const ChannelType m_red, m_green, m_blue;
 };
 
-// register in the unnamed registry so that these tests are run by default
+class ColourRGBAMatcher : public ColourRGBMatcher
-CPPUNIT_TEST_SUITE_REGISTRATION( ColourTestCase );
+{
+public:
+    ColourRGBAMatcher(ChannelType red, ChannelType green, ChannelType blue,
+                      ChannelType alpha)
+        : ColourRGBMatcher(red, green, blue),
+          m_alpha(alpha)
+    {
+    }
 
-// also include in its own registry so that these tests can be run alone
+    bool match(const wxColour& c) const wxOVERRIDE
-CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( ColourTestCase, "ColourTestCase" );
+    {
+        return ColourRGBMatcher::match(c) && c.Alpha() == m_alpha;
+    }
 
-void ColourTestCase::GetSetRGB()
+    std::string describe() const wxOVERRIDE
+    {
+        return wxString::Format("!= RGBA(%#02x, %#02x, %#02x, %#02x)",
+                                m_red, m_green, m_blue, m_alpha).ToStdString();
+    }
+
+private:
+    const ChannelType m_alpha;
+};
+
+inline
+ColourRGBMatcher
+RGBSameAs(ChannelType red, ChannelType green, ChannelType blue)
+{
+    return ColourRGBMatcher(red, green, blue);
+}
+
+inline
+ColourRGBAMatcher
+RGBASameAs(ChannelType red, ChannelType green, ChannelType blue, ChannelType alpha)
+{
+    return ColourRGBAMatcher(red, green, blue, alpha);
+}
+
+// ----------------------------------------------------------------------------
+// tests
+// ----------------------------------------------------------------------------
+
+TEST_CASE("wxColour::GetSetRGB", "[colour][rgb]")
 {
     wxColour c;
     c.SetRGB(0x123456);
 
-    CPPUNIT_ASSERT_EQUAL( 0x56, (int)c.Red() );
+    CHECK( c.Red()   == 0x56 );
-    CPPUNIT_ASSERT_EQUAL( 0x34, (int)c.Green() );
+    CHECK( c.Green() == 0x34 );
-    CPPUNIT_ASSERT_EQUAL( 0x12, (int)c.Blue() );
+    CHECK( c.Blue()  == 0x12 );
-    CPPUNIT_ASSERT_EQUAL( wxALPHA_OPAQUE, c.Alpha() );
+    CHECK( c.Alpha() == wxALPHA_OPAQUE );
 
-    CPPUNIT_ASSERT_EQUAL( wxColour(0x123456), c );
+    CHECK( c == wxColour(0x123456) );
-    CPPUNIT_ASSERT_EQUAL( 0x123456, c.GetRGB() );
+    CHECK( c.GetRGB() == 0x123456 );
 
     c.SetRGBA(0xaabbccdd);
 
-    CPPUNIT_ASSERT_EQUAL( 0xdd, (int)c.Red() );
+    CHECK( c.Red()   == 0xdd);
-    CPPUNIT_ASSERT_EQUAL( 0xcc, (int)c.Green() );
+    CHECK( c.Green() == 0xcc);
-    CPPUNIT_ASSERT_EQUAL( 0xbb, (int)c.Blue() );
+    CHECK( c.Blue()  == 0xbb);
 
     // wxX11 doesn't support alpha at all currently.
 #ifndef __WXX11__
-    CPPUNIT_ASSERT_EQUAL( 0xaa, (int)c.Alpha() );
+    CHECK( c.Alpha() == 0xaa );
 #endif // __WXX11__
 
     // FIXME: at least under wxGTK wxColour ctor doesn't take alpha channel
     //        into account: bug or feature?
-    //CPPUNIT_ASSERT_EQUAL( wxColour(0xaabbccdd), c );
+    //CHECK( c == wxColour(0xaabbccdd) );
-    CPPUNIT_ASSERT_EQUAL( 0xbbccdd, c.GetRGB() );
+    CHECK( c.GetRGB() == 0xbbccdd );
 #ifndef __WXX11__
-    CPPUNIT_ASSERT_EQUAL( 0xaabbccdd, c.GetRGBA() );
+    CHECK( c.GetRGBA() == 0xaabbccdd );
 #endif // __WXX11__
 }
 
-void ColourTestCase::FromString()
+TEST_CASE("wxColour::FromString", "[colour][string]")
 {
-    ASSERT_EQUAL_RGB( wxColour("rgb(11, 22, 33)"), 11, 22, 33 );
+    CHECK_THAT( wxColour("rgb(11, 22, 33)"), RGBSameAs(11, 22, 33) );
     // wxX11 doesn't support alpha at all currently.
 #ifndef __WXX11__
-    ASSERT_EQUAL_RGBA( wxColour("rgba(11, 22, 33, 0.5)"), 11, 22, 33, 128 );
+    CHECK_THAT( wxColour("rgba(11, 22, 33, 0.5)"), RGBASameAs(11, 22, 33, 128) );
-    ASSERT_EQUAL_RGBA( wxColour("rgba( 11, 22, 33, 0.5 )"), 11, 22, 33, 128 );
+    CHECK_THAT( wxColour("rgba( 11, 22, 33, 0.5 )"), RGBASameAs(11, 22, 33, 128) );
 #endif // __WXX11__
 
-    ASSERT_EQUAL_RGB( wxColour("#aabbcc"), 0xaa, 0xbb, 0xcc );
+    CHECK_THAT( wxColour("#aabbcc"), RGBSameAs(0xaa, 0xbb, 0xcc) );
 
-    ASSERT_EQUAL_RGB( wxColour("red"), 0xff, 0, 0 );
+    CHECK_THAT( wxColour("red"), RGBSameAs(0xff, 0, 0) );
 
     wxColour col;
-    CPPUNIT_ASSERT( !wxFromString("rgb(1, 2)", &col) );
+    CHECK( !wxFromString("rgb(1, 2)", &col) );
-    CPPUNIT_ASSERT( !wxFromString("rgba(1, 2, 3.456)", &col) );
+    CHECK( !wxFromString("rgba(1, 2, 3.456)", &col) );
-    CPPUNIT_ASSERT( !wxFromString("rgba(1, 2, 3.456, foo)", &col) );
+    CHECK( !wxFromString("rgba(1, 2, 3.456, foo)", &col) );
+}
+
+TEST_CASE("wxColour::GetAsString", "[colour][string]")
+{
+    CHECK( wxColour().GetAsString() == "" );
+
+    wxColour red("red");
+    CHECK( red.GetAsString() == "red" );
+    CHECK( red.GetAsString(wxC2S_CSS_SYNTAX) == "rgb(255, 0, 0)" );
+    CHECK( red.GetAsString(wxC2S_HTML_SYNTAX) == "#FF0000" );
 }
 
 TEST_CASE("wxColour::GetLuminance", "[colour][luminance]")