From e3a62efd4380b5e5fdf527fad99d5075b9ae0896 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 11 Jun 2019 15:13:02 +0200 Subject: [PATCH 01/16] Micro optimization in wxStaticTextBase::UpdateLabel() Avoid an unnecessary wxString copy. --- src/common/stattextcmn.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/stattextcmn.cpp b/src/common/stattextcmn.cpp index 811aceb190..d3e01244d3 100644 --- a/src/common/stattextcmn.cpp +++ b/src/common/stattextcmn.cpp @@ -232,7 +232,7 @@ void wxStaticTextBase::UpdateLabel() if (!IsEllipsized()) return; - wxString newlabel = GetEllipsizedLabel(); + const wxString& newlabel = GetEllipsizedLabel(); // we need to touch the "real" label (i.e. the text set inside the control, // using port-specific functions) instead of the string returned by GetLabel(). From b53e9e2006eb96aad4d5d3367a6574a07ad7a3de Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 12 Jun 2019 23:40:18 +0200 Subject: [PATCH 02/16] Rename wxStaticText::Do[SG]etLabel() to WX[SG]etVisibleLabel() The names of these methods were confusing because they implied that they were the actual implementations of the public [SG]etLabel(), while this wasn't at all the case. Give them then ames describing what they really do and also update the comments to hopefully be more clear. No real changes. --- include/wx/generic/stattextg.h | 4 ++-- include/wx/gtk/stattext.h | 4 ++-- include/wx/motif/stattext.h | 4 ++-- include/wx/msw/stattext.h | 4 ++-- include/wx/osx/stattext.h | 4 ++-- include/wx/stattext.h | 28 +++++++++++++++++----------- include/wx/univ/stattext.h | 4 ++-- src/common/stattextcmn.cpp | 4 ++-- src/generic/stattextg.cpp | 4 ++-- src/gtk/stattext.cpp | 4 ++-- src/motif/stattext.cpp | 6 +++--- src/msw/stattext.cpp | 16 +++++++++------- src/osx/stattext_osx.cpp | 8 ++++---- src/univ/stattext.cpp | 6 +++--- 14 files changed, 54 insertions(+), 46 deletions(-) diff --git a/include/wx/generic/stattextg.h b/include/wx/generic/stattextg.h index 2a85d99c17..84210423d4 100644 --- a/include/wx/generic/stattextg.h +++ b/include/wx/generic/stattextg.h @@ -54,8 +54,8 @@ public: protected: virtual wxSize DoGetBestClientSize() const wxOVERRIDE; - virtual wxString DoGetLabel() const wxOVERRIDE { return m_label; } - virtual void DoSetLabel(const wxString& label) wxOVERRIDE; + virtual wxString WXGetVisibleLabel() const wxOVERRIDE { return m_label; } + virtual void WXSetVisibleLabel(const wxString& label) wxOVERRIDE; void DoSetSize(int x, int y, int width, int height, int sizeFlags) wxOVERRIDE; diff --git a/include/wx/gtk/stattext.h b/include/wx/gtk/stattext.h index b94f399a50..37f2939c47 100644 --- a/include/wx/gtk/stattext.h +++ b/include/wx/gtk/stattext.h @@ -49,8 +49,8 @@ protected: virtual wxSize DoGetBestSize() const wxOVERRIDE; - virtual wxString DoGetLabel() const wxOVERRIDE; - virtual void DoSetLabel(const wxString& str) wxOVERRIDE; + virtual wxString WXGetVisibleLabel() const wxOVERRIDE; + virtual void WXSetVisibleLabel(const wxString& str) wxOVERRIDE; #if wxUSE_MARKUP virtual bool DoSetLabelMarkup(const wxString& markup) wxOVERRIDE; #endif // wxUSE_MARKUP diff --git a/include/wx/motif/stattext.h b/include/wx/motif/stattext.h index e92780cd02..2e6ce08699 100644 --- a/include/wx/motif/stattext.h +++ b/include/wx/motif/stattext.h @@ -51,8 +51,8 @@ public: virtual WXWidget GetLabelWidget() const { return m_labelWidget; } - virtual void DoSetLabel(const wxString& str); - virtual wxString DoGetLabel() const; + virtual void WXSetVisibleLabel(const wxString& str); + virtual wxString WXGetVisibleLabel() const; virtual wxSize DoGetBestSize() const; protected: diff --git a/include/wx/msw/stattext.h b/include/wx/msw/stattext.h index e52248fc84..9c9f0fb227 100644 --- a/include/wx/msw/stattext.h +++ b/include/wx/msw/stattext.h @@ -47,8 +47,8 @@ protected: int sizeFlags = wxSIZE_AUTO) wxOVERRIDE; virtual wxSize DoGetBestClientSize() const wxOVERRIDE; - virtual wxString DoGetLabel() const wxOVERRIDE; - virtual void DoSetLabel(const wxString& str) wxOVERRIDE; + virtual wxString WXGetVisibleLabel() const wxOVERRIDE; + virtual void WXSetVisibleLabel(const wxString& str) wxOVERRIDE; wxDECLARE_DYNAMIC_CLASS_NO_COPY(wxStaticText); }; diff --git a/include/wx/osx/stattext.h b/include/wx/osx/stattext.h index 45249da560..87136c1861 100644 --- a/include/wx/osx/stattext.h +++ b/include/wx/osx/stattext.h @@ -41,8 +41,8 @@ public: protected : - virtual wxString DoGetLabel() const wxOVERRIDE; - virtual void DoSetLabel(const wxString& str) wxOVERRIDE; + virtual wxString WXGetVisibleLabel() const wxOVERRIDE; + virtual void WXSetVisibleLabel(const wxString& str) wxOVERRIDE; virtual wxSize DoGetBestSize() const wxOVERRIDE; diff --git a/include/wx/stattext.h b/include/wx/stattext.h index 237f93620e..369ab6dc87 100644 --- a/include/wx/stattext.h +++ b/include/wx/stattext.h @@ -63,21 +63,27 @@ protected: // functions required for wxST_ELLIPSIZE_* support // style. Shouldn't be called if we don't have any. wxString Ellipsize(const wxString& label) const; - // to be called when updating the size of the static text: - // updates the label redoing ellipsization calculations + + // Note that even though ports with native support for ellipsization + // (currently only wxGTK) don't need this stuff, we still need to define it + // as it's used by wxGenericStaticText. + + // Must be called when the size or font changes to redo the ellipsization + // for the new size. Calls WXSetVisibleLabel() to actually update the + // display. void UpdateLabel(); - // These functions are platform-specific and must be overridden in ports - // which do not natively support ellipsization and they must be implemented - // in a way so that the m_labelOrig member of wxControl is not touched: + // These functions are platform-specific and must be implemented in the + // platform-specific code. They must not use or update m_labelOrig. - // returns the real label currently displayed inside the control. - virtual wxString DoGetLabel() const { return wxEmptyString; } + // Returns the label currently displayed inside the control, with mnemonics + // if any. + virtual wxString WXGetVisibleLabel() const = 0; - // sets the real label currently displayed inside the control, - // _without_ invalidating the size. The text passed is always markup-free - // but may contain the mnemonic characters. - virtual void DoSetLabel(const wxString& WXUNUSED(str)) { } + // Sets the real label currently displayed inside the control, _without_ + // invalidating the size. The text passed is always markup-free but may + // contain the mnemonic characters. + virtual void WXSetVisibleLabel(const wxString& str) = 0; // Update the current size to match the best size unless wxST_NO_AUTORESIZE // style is explicitly used. diff --git a/include/wx/univ/stattext.h b/include/wx/univ/stattext.h index 891ad7cf60..54a0284c58 100644 --- a/include/wx/univ/stattext.h +++ b/include/wx/univ/stattext.h @@ -58,8 +58,8 @@ protected: // draw the control virtual void DoDraw(wxControlRenderer *renderer) wxOVERRIDE; - virtual void DoSetLabel(const wxString& str) wxOVERRIDE; - virtual wxString DoGetLabel() const wxOVERRIDE; + virtual void WXSetVisibleLabel(const wxString& str) wxOVERRIDE; + virtual wxString WXGetVisibleLabel() const wxOVERRIDE; wxDECLARE_DYNAMIC_CLASS(wxStaticText); }; diff --git a/src/common/stattextcmn.cpp b/src/common/stattextcmn.cpp index d3e01244d3..553c607647 100644 --- a/src/common/stattextcmn.cpp +++ b/src/common/stattextcmn.cpp @@ -240,9 +240,9 @@ void wxStaticTextBase::UpdateLabel() // In fact, we must be careful not to touch the original label passed to // SetLabel() otherwise GetLabel() will behave in a strange way to the user // (e.g. returning a "Ver...ing" instead of "Very long string") ! - if (newlabel == DoGetLabel()) + if (newlabel == WXGetVisibleLabel()) return; - DoSetLabel(newlabel); + WXSetVisibleLabel(newlabel); } wxString wxStaticTextBase::GetEllipsizedLabel() const diff --git a/src/generic/stattextg.cpp b/src/generic/stattextg.cpp index fe2f13759c..ce32239065 100644 --- a/src/generic/stattextg.cpp +++ b/src/generic/stattextg.cpp @@ -100,7 +100,7 @@ wxSize wxGenericStaticText::DoGetBestClientSize() const void wxGenericStaticText::SetLabel(const wxString& label) { wxControl::SetLabel(label); - DoSetLabel(GetEllipsizedLabel()); + WXSetVisibleLabel(GetEllipsizedLabel()); AutoResizeIfNecessary(); @@ -115,7 +115,7 @@ void wxGenericStaticText::SetLabel(const wxString& label) Refresh(); } -void wxGenericStaticText::DoSetLabel(const wxString& label) +void wxGenericStaticText::WXSetVisibleLabel(const wxString& label) { m_mnemonic = FindAccelIndex(label, &m_label); } diff --git a/src/gtk/stattext.cpp b/src/gtk/stattext.cpp index 36d87b502c..067cf1294e 100644 --- a/src/gtk/stattext.cpp +++ b/src/gtk/stattext.cpp @@ -272,13 +272,13 @@ void wxStaticText::GTKWidgetDoSetMnemonic(GtkWidget* w) // These functions should be used only when GTK+ < 2.6 by wxStaticTextBase::UpdateLabel() -wxString wxStaticText::DoGetLabel() const +wxString wxStaticText::WXGetVisibleLabel() const { GtkLabel *label = GTK_LABEL(m_widget); return wxGTK_CONV_BACK( gtk_label_get_text( label ) ); } -void wxStaticText::DoSetLabel(const wxString& str) +void wxStaticText::WXSetVisibleLabel(const wxString& str) { GTKSetLabelForLabel(GTK_LABEL(m_widget), str); } diff --git a/src/motif/stattext.cpp b/src/motif/stattext.cpp index 44822abbfe..2fdea861ff 100644 --- a/src/motif/stattext.cpp +++ b/src/motif/stattext.cpp @@ -77,12 +77,12 @@ void wxStaticText::SetLabel(const wxString& label) m_labelOrig = label; // save original label // Motif does not support ellipsized labels natively - DoSetLabel(GetEllipsizedLabel()); + WXSetVisibleLabel(GetEllipsizedLabel()); } // for wxST_ELLIPSIZE_* support: -wxString wxStaticText::DoGetLabel() const +wxString wxStaticText::WXGetVisibleLabel() const { XmString label = NULL; XtVaGetValues((Widget)m_labelWidget, XmNlabelString, &label, NULL); @@ -90,7 +90,7 @@ wxString wxStaticText::DoGetLabel() const return wxXmStringToString(label); } -void wxStaticText::DoSetLabel(const wxString& str) +void wxStaticText::WXSetVisibleLabel(const wxString& str) { // build our own cleaned label wxXmString label_str(RemoveMnemonics(str)); diff --git a/src/msw/stattext.cpp b/src/msw/stattext.cpp index f5a29f380e..fd171211a8 100644 --- a/src/msw/stattext.cpp +++ b/src/msw/stattext.cpp @@ -179,10 +179,10 @@ void wxStaticText::SetLabel(const wxString& label) #ifdef SS_ENDELLIPSIS if ( updateStyle.IsOn(SS_ENDELLIPSIS) ) - DoSetLabel(GetLabel()); + WXSetVisibleLabel(GetLabel()); else #endif // SS_ENDELLIPSIS - DoSetLabel(GetEllipsizedLabel()); + WXSetVisibleLabel(GetEllipsizedLabel()); AutoResizeIfNecessary(); } @@ -197,16 +197,18 @@ bool wxStaticText::SetFont(const wxFont& font) return true; } -// for wxST_ELLIPSIZE_* support: +// These functions are used by wxST_ELLIPSIZE_* supporting code in +// wxStaticTextBase which requires us to implement them, but actually the base +// wxWindow methods already do exactly what we need under this platform. -wxString wxStaticText::DoGetLabel() const +wxString wxStaticText::WXGetVisibleLabel() const { - return wxGetWindowText(GetHwnd()); + return wxWindow::GetLabel(); } -void wxStaticText::DoSetLabel(const wxString& str) +void wxStaticText::WXSetVisibleLabel(const wxString& str) { - SetWindowText(GetHwnd(), str.c_str()); + wxWindow::SetLabel(str); } diff --git a/src/osx/stattext_osx.cpp b/src/osx/stattext_osx.cpp index 53e0e0ced9..cea4cf5c19 100644 --- a/src/osx/stattext_osx.cpp +++ b/src/osx/stattext_osx.cpp @@ -71,11 +71,11 @@ void wxStaticText::SetLabel(const wxString& label) ) { // leave ellipsization to the OS - DoSetLabel(GetLabel()); + WXSetVisibleLabel(GetLabel()); } else // not supported natively { - DoSetLabel(GetEllipsizedLabel()); + WXSetVisibleLabel(GetEllipsizedLabel()); } AutoResizeIfNecessary(); @@ -98,7 +98,7 @@ bool wxStaticText::SetFont(const wxFont& font) return ret; } -void wxStaticText::DoSetLabel(const wxString& label) +void wxStaticText::WXSetVisibleLabel(const wxString& label) { m_label = RemoveMnemonics(label); GetPeer()->SetLabel(m_label , GetFont().GetEncoding() ); @@ -118,7 +118,7 @@ bool wxStaticText::DoSetLabelMarkup(const wxString& markup) #endif // wxUSE_MARKUP && wxOSX_USE_COCOA -wxString wxStaticText::DoGetLabel() const +wxString wxStaticText::WXGetVisibleLabel() const { return m_label; } diff --git a/src/univ/stattext.cpp b/src/univ/stattext.cpp index a79338873c..7d25958ad9 100644 --- a/src/univ/stattext.cpp +++ b/src/univ/stattext.cpp @@ -77,15 +77,15 @@ void wxStaticText::SetLabel(const wxString& str) m_labelOrig = str; // draw as real label the abbreviated version of it - DoSetLabel(GetEllipsizedLabel()); + WXSetVisibleLabel(GetEllipsizedLabel()); } -void wxStaticText::DoSetLabel(const wxString& str) +void wxStaticText::WXSetVisibleLabel(const wxString& str) { UnivDoSetLabel(str); } -wxString wxStaticText::DoGetLabel() const +wxString wxStaticText::WXGetVisibleLabel() const { return wxControl::GetLabel(); } From 3f7c3f01906ad7d6b1736a38bce5e58ccc357f68 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 13 Jun 2019 12:51:55 +0200 Subject: [PATCH 03/16] Don't implement wxStaticText::WX[SG]etVisibleLabel() in wxGTK These functions are never used in this port, so make it clear, both in the code and in the comment preceding them. --- src/gtk/stattext.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/gtk/stattext.cpp b/src/gtk/stattext.cpp index 067cf1294e..524b6d97b8 100644 --- a/src/gtk/stattext.cpp +++ b/src/gtk/stattext.cpp @@ -270,17 +270,23 @@ void wxStaticText::GTKWidgetDoSetMnemonic(GtkWidget* w) } -// These functions should be used only when GTK+ < 2.6 by wxStaticTextBase::UpdateLabel() +// These functions are not used as GTK supports ellipsization natively and we +// never call the base class UpdateText() which uses them. +// +// Note that, unfortunately, we still need to define them because they still +// exist, as pure virtuals, in the base class even in wxGTK to allow +// wxGenericStaticText to override them. wxString wxStaticText::WXGetVisibleLabel() const { - GtkLabel *label = GTK_LABEL(m_widget); - return wxGTK_CONV_BACK( gtk_label_get_text( label ) ); + wxFAIL_MSG(wxS("Unreachable")); + + return wxString(); } -void wxStaticText::WXSetVisibleLabel(const wxString& str) +void wxStaticText::WXSetVisibleLabel(const wxString& WXUNUSED(str)) { - GTKSetLabelForLabel(GTK_LABEL(m_widget), str); + wxFAIL_MSG(wxS("Unreachable")); } // static From d22321b14f31017231b50fee63f392a37ce36a16 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 19 Jun 2019 18:49:06 +0200 Subject: [PATCH 04/16] Rewrite wxControl label unit test without CppUnit macros Also remove the macros used in the test to perform the same tests for wxStaticText and wxCheckBox and use a helper function instead, making the code more clear and extensible. No real changes. --- tests/controls/label.cpp | 186 ++++++++++++--------------------------- 1 file changed, 54 insertions(+), 132 deletions(-) diff --git a/tests/controls/label.cpp b/tests/controls/label.cpp index 3ba89c2f7e..5944966c2d 100644 --- a/tests/controls/label.cpp +++ b/tests/controls/label.cpp @@ -20,83 +20,22 @@ #include "wx/app.h" #endif // WX_PRECOMP -#include "wx/control.h" -#include "wx/stattext.h" #include "wx/checkbox.h" +#include "wx/control.h" +#include "wx/scopedptr.h" +#include "wx/stattext.h" -// ---------------------------------------------------------------------------- -// test class -// ---------------------------------------------------------------------------- - -class LabelTestCase : public CppUnit::TestCase +namespace { -public: - LabelTestCase() { } - virtual void setUp() wxOVERRIDE; - virtual void tearDown() wxOVERRIDE; +const char* const ORIGINAL_LABEL = "origin label"; -private: - CPPUNIT_TEST_SUITE( LabelTestCase ); - CPPUNIT_TEST( GetLabel ); - CPPUNIT_TEST( GetLabelText ); - CPPUNIT_TEST( Statics ); - CPPUNIT_TEST_SUITE_END(); - - void GetLabel(); - void GetLabelText(); - void Statics(); - - wxStaticText *m_st; - - // we cannot test wxControl directly (it's abstract) so we rather test wxCheckBox - wxCheckBox *m_cb; - - wxDECLARE_NO_COPY_CLASS(LabelTestCase); -}; - -// register in the unnamed registry so that these tests are run by default -CPPUNIT_TEST_SUITE_REGISTRATION( LabelTestCase ); - -// also include in its own registry so that these tests can be run alone -CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( LabelTestCase, "LabelTestCase" ); - -// ---------------------------------------------------------------------------- -// test initialization -// ---------------------------------------------------------------------------- - -#define ORIGINAL_LABEL "original label" - -void LabelTestCase::setUp() +// The actual testing function. It will change the label of the provided +// control, which is assumed to be ORIGINAL_LABEL initially. +void DoTestLabel(wxControl* c) { - m_st = new wxStaticText(wxTheApp->GetTopWindow(), wxID_ANY, ORIGINAL_LABEL); + CHECK( c->GetLabel() == ORIGINAL_LABEL ); - m_cb = new wxCheckBox(wxTheApp->GetTopWindow(), wxID_ANY, ORIGINAL_LABEL); - - CPPUNIT_ASSERT_EQUAL( ORIGINAL_LABEL, m_st->GetLabel() ); - CPPUNIT_ASSERT_EQUAL( ORIGINAL_LABEL, m_cb->GetLabel() ); -} - -void LabelTestCase::tearDown() -{ - wxDELETE(m_st); - wxDELETE(m_cb); -} - -// ---------------------------------------------------------------------------- -// the tests themselves -// ---------------------------------------------------------------------------- - -#define SET_LABEL(str) \ - m_st->SetLabel(str); \ - m_cb->SetLabel(str); - -#define SET_LABEL_TEXT(str) \ - m_st->SetLabelText(str); \ - m_cb->SetLabelText(str); - -void LabelTestCase::GetLabel() -{ const wxString testLabelArray[] = { "label without mnemonics and markup", "label with &mnemonic", @@ -104,92 +43,75 @@ void LabelTestCase::GetLabel() "label with some markup and &mnemonic", }; - // test calls to SetLabel() and then to GetLabel() - for ( unsigned int s = 0; s < WXSIZEOF(testLabelArray); s++ ) { - SET_LABEL(testLabelArray[s]); + const wxString& l = testLabelArray[s]; // GetLabel() should always return the string passed to SetLabel() - CPPUNIT_ASSERT_EQUAL( testLabelArray[s], m_st->GetLabel() ); - CPPUNIT_ASSERT_EQUAL( testLabelArray[s], m_cb->GetLabel() ); + c->SetLabel(l); + CHECK( c->GetLabel() == l ); + + // GetLabelText() should always return the string passed to SetLabelText() + c->SetLabelText(l); + CHECK( c->GetLabelText() == l ); } // test calls to SetLabelText() and then to GetLabel() - const wxString& testLabel = "label without mnemonics and markup"; - SET_LABEL_TEXT(testLabel); - CPPUNIT_ASSERT_EQUAL( testLabel, m_st->GetLabel() ); - CPPUNIT_ASSERT_EQUAL( testLabel, m_cb->GetLabel() ); + wxString testLabel = "label without mnemonics and markup"; + c->SetLabelText(testLabel); + CHECK( c->GetLabel() == testLabel ); - const wxString& testLabel2 = "label with &mnemonic"; - const wxString& testLabelText2 = "label with &&mnemonic"; - SET_LABEL_TEXT(testLabel2); - CPPUNIT_ASSERT_EQUAL( testLabelText2, m_st->GetLabel() ); - CPPUNIT_ASSERT_EQUAL( testLabelText2, m_cb->GetLabel() ); + c->SetLabelText("label with &mnemonic"); + CHECK( c->GetLabel() == "label with &&mnemonic" ); - const wxString& testLabel3 = "label with some markup"; - SET_LABEL_TEXT(testLabel3); - CPPUNIT_ASSERT_EQUAL( testLabel3, m_st->GetLabel() ); - CPPUNIT_ASSERT_EQUAL( testLabel3, m_cb->GetLabel() ); + testLabel = "label with some markup"; + c->SetLabelText(testLabel); + CHECK( c->GetLabel() == testLabel ); - const wxString& testLabel4 = "label with some markup and &mnemonic"; - const wxString& testLabelText4 = "label with some markup and &&mnemonic"; - SET_LABEL_TEXT(testLabel4); - CPPUNIT_ASSERT_EQUAL( testLabelText4, m_st->GetLabel() ); - CPPUNIT_ASSERT_EQUAL( testLabelText4, m_cb->GetLabel() ); -} + c->SetLabelText("label with some markup and &mnemonic"); + CHECK( c->GetLabel() == "label with some markup and &&mnemonic" ); -void LabelTestCase::GetLabelText() -{ // test calls to SetLabel() and then to GetLabelText() - const wxString& testLabel = "label without mnemonics and markup"; - SET_LABEL(testLabel); - CPPUNIT_ASSERT_EQUAL( testLabel, m_st->GetLabelText() ); - CPPUNIT_ASSERT_EQUAL( testLabel, m_cb->GetLabelText() ); + testLabel = "label without mnemonics and markup"; + c->SetLabel(testLabel); + CHECK( c->GetLabelText() == testLabel ); - const wxString& testLabel2 = "label with &mnemonic"; - const wxString& testLabelText2 = "label with mnemonic"; - SET_LABEL(testLabel2); - CPPUNIT_ASSERT_EQUAL( testLabelText2, m_st->GetLabelText() ); - CPPUNIT_ASSERT_EQUAL( testLabelText2, m_cb->GetLabelText() ); + c->SetLabel("label with &mnemonic"); + CHECK( c->GetLabelText() == "label with mnemonic" ); - const wxString& testLabel3 = "label with some markup"; - SET_LABEL(testLabel3); - CPPUNIT_ASSERT_EQUAL( testLabel3, m_st->GetLabelText() ); - CPPUNIT_ASSERT_EQUAL( testLabel3, m_cb->GetLabelText() ); + testLabel = "label with some markup"; + c->SetLabel(testLabel); + CHECK( c->GetLabelText() == testLabel ); - const wxString& testLabel4 = "label with some markup and &mnemonic"; - const wxString& testLabelText4 = "label with some markup and mnemonic"; - SET_LABEL(testLabel4); - CPPUNIT_ASSERT_EQUAL( testLabelText4, m_st->GetLabelText() ); - CPPUNIT_ASSERT_EQUAL( testLabelText4, m_cb->GetLabelText() ); + c->SetLabel("label with some markup and &mnemonic"); + CHECK( c->GetLabelText() == "label with some markup and mnemonic"); +} +} // anonymous namespace - const wxString testLabelArray[] = { - "label without mnemonics and markup", - "label with &mnemonic", - "label with some markup", - "label with some markup and &mnemonic", - }; - - // test calls to SetLabelText() and then to GetLabelText() - - for ( unsigned int s = 0; s < WXSIZEOF(testLabelArray); s++ ) +TEST_CASE("wxControl::Label", "[wxControl][label]") +{ + SECTION("wxStaticText") { - SET_LABEL_TEXT(testLabelArray[s]); + const wxScopedPtr + st(new wxStaticText(wxTheApp->GetTopWindow(), wxID_ANY, ORIGINAL_LABEL)); + DoTestLabel(st.get()); + } - // GetLabelText() should always return the string passed to SetLabelText() - CPPUNIT_ASSERT_EQUAL( testLabelArray[s], m_st->GetLabelText() ); - CPPUNIT_ASSERT_EQUAL( testLabelArray[s], m_cb->GetLabelText() ); + SECTION("wxCheckBox") + { + const wxScopedPtr + cb(new wxCheckBox(wxTheApp->GetTopWindow(), wxID_ANY, ORIGINAL_LABEL)); + DoTestLabel(cb.get()); } } -void LabelTestCase::Statics() +TEST_CASE("wxControl::RemoveMnemonics", "[wxControl][label][mnemonics]") { - CPPUNIT_ASSERT_EQUAL( "mnemonic", wxControl::RemoveMnemonics("&mnemonic") ); - CPPUNIT_ASSERT_EQUAL( "&mnemonic", wxControl::RemoveMnemonics("&&mnemonic") ); - CPPUNIT_ASSERT_EQUAL( "&mnemonic", wxControl::RemoveMnemonics("&&&mnemonic") ); + CHECK( "mnemonic" == wxControl::RemoveMnemonics("&mnemonic") ); + CHECK( "&mnemonic" == wxControl::RemoveMnemonics("&&mnemonic") ); + CHECK( "&mnemonic" == wxControl::RemoveMnemonics("&&&mnemonic") ); } From 5e98099699c2f89b41ba750bb9429f4adec86395 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 19 Jun 2019 18:55:52 +0200 Subject: [PATCH 05/16] Add unit test for wxGenericStaticText label Run the same tests for it as for the native wxStaticText too. --- tests/controls/label.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/controls/label.cpp b/tests/controls/label.cpp index 5944966c2d..3b52fb02f6 100644 --- a/tests/controls/label.cpp +++ b/tests/controls/label.cpp @@ -25,6 +25,8 @@ #include "wx/scopedptr.h" #include "wx/stattext.h" +#include "wx/generic/stattextg.h" + namespace { @@ -101,6 +103,13 @@ TEST_CASE("wxControl::Label", "[wxControl][label]") DoTestLabel(st.get()); } + SECTION("wxGenericStaticText") + { + const wxScopedPtr + gst(new wxGenericStaticText(wxTheApp->GetTopWindow(), wxID_ANY, ORIGINAL_LABEL)); + DoTestLabel(gst.get()); + } + SECTION("wxCheckBox") { const wxScopedPtr From 0d35c3f36abbbdee9f1d9271da9c3721e5e7bcac Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 19 Jun 2019 19:08:38 +0200 Subject: [PATCH 06/16] Put all wxControl label unit tests inside a loop There doesn't seem to be any reason to write some of them out explicitly. No real changes. --- tests/controls/label.cpp | 39 ++++++--------------------------------- 1 file changed, 6 insertions(+), 33 deletions(-) diff --git a/tests/controls/label.cpp b/tests/controls/label.cpp index 3b52fb02f6..1b8f744cd8 100644 --- a/tests/controls/label.cpp +++ b/tests/controls/label.cpp @@ -53,43 +53,16 @@ void DoTestLabel(wxControl* c) c->SetLabel(l); CHECK( c->GetLabel() == l ); + // GetLabelText() should always return unescaped version of the label + CHECK( c->GetLabelText() == wxControl::RemoveMnemonics(l) ); + // GetLabelText() should always return the string passed to SetLabelText() c->SetLabelText(l); CHECK( c->GetLabelText() == l ); + + // And GetLabel() should be the escaped version of the text + CHECK( l == wxControl::RemoveMnemonics(c->GetLabel()) ); } - - - // test calls to SetLabelText() and then to GetLabel() - - wxString testLabel = "label without mnemonics and markup"; - c->SetLabelText(testLabel); - CHECK( c->GetLabel() == testLabel ); - - c->SetLabelText("label with &mnemonic"); - CHECK( c->GetLabel() == "label with &&mnemonic" ); - - testLabel = "label with some markup"; - c->SetLabelText(testLabel); - CHECK( c->GetLabel() == testLabel ); - - c->SetLabelText("label with some markup and &mnemonic"); - CHECK( c->GetLabel() == "label with some markup and &&mnemonic" ); - - // test calls to SetLabel() and then to GetLabelText() - - testLabel = "label without mnemonics and markup"; - c->SetLabel(testLabel); - CHECK( c->GetLabelText() == testLabel ); - - c->SetLabel("label with &mnemonic"); - CHECK( c->GetLabelText() == "label with mnemonic" ); - - testLabel = "label with some markup"; - c->SetLabel(testLabel); - CHECK( c->GetLabelText() == testLabel ); - - c->SetLabel("label with some markup and &mnemonic"); - CHECK( c->GetLabelText() == "label with some markup and mnemonic"); } } // anonymous namespace From 21babfa2d0b692e16ac676c341a9cacda303c510 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 19 Jun 2019 19:13:00 +0200 Subject: [PATCH 07/16] Test a couple more cases in wxControl label unit test Check that double ampersand also works correctly. --- tests/controls/label.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/controls/label.cpp b/tests/controls/label.cpp index 1b8f744cd8..bb8cf168b8 100644 --- a/tests/controls/label.cpp +++ b/tests/controls/label.cpp @@ -43,6 +43,9 @@ void DoTestLabel(wxControl* c) "label with &mnemonic", "label with some markup", "label with some markup and &mnemonic", + "label with an && (ampersand)", + "label with an && (&ersand)", + "", // empty label should work too }; for ( unsigned int s = 0; s < WXSIZEOF(testLabelArray); s++ ) From a93b1416a7e37fb9543adc50879d498b10d881ac Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 19 Jun 2019 19:18:48 +0200 Subject: [PATCH 08/16] Add unit tests for wxControl::SetLabelMarkup() too Check that "&" is interpreted correctly. --- tests/controls/label.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/controls/label.cpp b/tests/controls/label.cpp index bb8cf168b8..b9153e3f13 100644 --- a/tests/controls/label.cpp +++ b/tests/controls/label.cpp @@ -66,6 +66,16 @@ void DoTestLabel(wxControl* c) // And GetLabel() should be the escaped version of the text CHECK( l == wxControl::RemoveMnemonics(c->GetLabel()) ); } + +#if wxUSE_MARKUP + c->SetLabelMarkup("mnemonic in &markup"); + CHECK( c->GetLabel() == "mnemonic in &markup" ); + CHECK( c->GetLabelText() == "mnemonic in markup" ); + + c->SetLabelMarkup("&& finally"); + CHECK( c->GetLabel() == "&& finally" ); + CHECK( c->GetLabelText() == "& finally" ); +#endif // wxUSE_MARKUP } } // anonymous namespace From ee15a4c9e4b6624f83340aec3e9590a5b55e90f8 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 19 Jun 2019 19:31:11 +0200 Subject: [PATCH 09/16] Avoid assertions when using wxGenericStaticText in widgets sample Clicking on the "Generic wxStaticText" box resulted in several assertions because the markup string contained both a single "&" and a "&" used for the mnemonic. Double the former to avoid misinterpreting it as a mnemonic character too. --- samples/widgets/static.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/widgets/static.cpp b/samples/widgets/static.cpp index 163d500cda..31a24c3837 100644 --- a/samples/widgets/static.cpp +++ b/samples/widgets/static.cpp @@ -346,8 +346,8 @@ void StaticWidgetsPage::CreateContent() #if wxUSE_MARKUP m_textLabelWithMarkup->SetValue("Another label, this time decorated " "with markup; here you need entities " - "for the symbols: < > & ' " " - " but you can still place &mnemonics..."); + "for the symbols: < > && ' " " + " but you can still use &mnemonics too"); #endif // wxUSE_MARKUP // right pane From 672847772d8602ff40c940152b63c18e76396373 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 19 Jun 2019 19:40:05 +0200 Subject: [PATCH 10/16] Fix an off-by-1 bug in wxControl::FindAccelIndex() after "&&" This resulted in wrong letter being underlined in wxGenericStaticText when the mnemonic occurred after "&&" (i.e. an actual ampersand) in the label. Add unit test which passes now, but would fail before on the last check. --- src/common/ctrlcmn.cpp | 8 ++++++-- tests/controls/label.cpp | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/common/ctrlcmn.cpp b/src/common/ctrlcmn.cpp index cb1c0fcf41..3715caf07b 100644 --- a/src/common/ctrlcmn.cpp +++ b/src/common/ctrlcmn.cpp @@ -194,20 +194,24 @@ int wxControlBase::FindAccelIndex(const wxString& label, wxString *labelOnly) labelOnly->Alloc(label.length()); } + // When computing the offset below, we need to ignore the characters that + // are not actually displayed, i.e. the ampersands themselves. + int numSkipped = 0; int indexAccel = -1; for ( wxString::const_iterator pc = label.begin(); pc != label.end(); ++pc ) { if ( *pc == MNEMONIC_PREFIX ) { ++pc; // skip it + ++numSkipped; + if ( pc == label.end() ) break; else if ( *pc != MNEMONIC_PREFIX ) { if ( indexAccel == -1 ) { - // remember it (-1 is for MNEMONIC_PREFIX itself - indexAccel = pc - label.begin() - 1; + indexAccel = pc - label.begin() - numSkipped; } else { diff --git a/tests/controls/label.cpp b/tests/controls/label.cpp index b9153e3f13..0e193196a8 100644 --- a/tests/controls/label.cpp +++ b/tests/controls/label.cpp @@ -67,14 +67,23 @@ void DoTestLabel(wxControl* c) CHECK( l == wxControl::RemoveMnemonics(c->GetLabel()) ); } + // Check that both "&" and "&" work in markup. #if wxUSE_MARKUP c->SetLabelMarkup("mnemonic in &markup"); CHECK( c->GetLabel() == "mnemonic in &markup" ); CHECK( c->GetLabelText() == "mnemonic in markup" ); + c->SetLabelMarkup("mnemonic in &markup"); + CHECK( c->GetLabel() == "mnemonic in &markup" ); + CHECK( c->GetLabelText() == "mnemonic in markup" ); + c->SetLabelMarkup("&& finally"); CHECK( c->GetLabel() == "&& finally" ); CHECK( c->GetLabelText() == "& finally" ); + + c->SetLabelMarkup("&& finally"); + CHECK( c->GetLabel() == "&& finally" ); + CHECK( c->GetLabelText() == "& finally" ); #endif // wxUSE_MARKUP } @@ -110,3 +119,12 @@ TEST_CASE("wxControl::RemoveMnemonics", "[wxControl][label][mnemonics]") CHECK( "&mnemonic" == wxControl::RemoveMnemonics("&&mnemonic") ); CHECK( "&mnemonic" == wxControl::RemoveMnemonics("&&&mnemonic") ); } + +TEST_CASE("wxControl::FindAccelIndex", "[wxControl][label][mnemonics]") +{ + CHECK( wxControl::FindAccelIndex("foo") == wxNOT_FOUND ); + CHECK( wxControl::FindAccelIndex("&foo") == 0 ); + CHECK( wxControl::FindAccelIndex("f&oo") == 1 ); + CHECK( wxControl::FindAccelIndex("foo && bar") == wxNOT_FOUND ); + CHECK( wxControl::FindAccelIndex("foo && &bar") == 6 ); +} From 8fcedbed7b4a3f4f02dd61e83340a91156871349 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 19 Jun 2019 19:49:43 +0200 Subject: [PATCH 11/16] Remove event table from static page in the widgets sample The code was confusing as it used Bind() for some handlers, event table for some others and, for the 3 buttons in the middle column, it actually managed to use both. Get rid of the event table completely to make this more clear. --- samples/widgets/static.cpp | 42 ++++++++------------------------------ 1 file changed, 8 insertions(+), 34 deletions(-) diff --git a/samples/widgets/static.cpp b/samples/widgets/static.cpp index 31a24c3837..73821d9666 100644 --- a/samples/widgets/static.cpp +++ b/samples/widgets/static.cpp @@ -49,15 +49,6 @@ // constants // ---------------------------------------------------------------------------- -// control ids -enum -{ - StaticPage_Reset = wxID_HIGHEST, - StaticPage_BoxText, - StaticPage_LabelText, - StaticPage_LabelTextWithMarkup -}; - // alignment radiobox values enum { @@ -114,7 +105,7 @@ public: protected: // event handlers - void OnCheckOrRadioBox(wxCommandEvent& event); + void OnCheckEllipsize(wxCommandEvent& event); #ifdef wxHAS_WINDOW_LABEL_IN_STATIC_BOX void OnBoxCheckBox(wxCommandEvent& event); #endif // wxHAS_WINDOW_LABEL_IN_STATIC_BOX @@ -176,26 +167,9 @@ protected: #endif // wxUSE_MARKUP private: - wxDECLARE_EVENT_TABLE(); DECLARE_WIDGETS_PAGE(StaticWidgetsPage) }; -// ---------------------------------------------------------------------------- -// event tables -// ---------------------------------------------------------------------------- - -wxBEGIN_EVENT_TABLE(StaticWidgetsPage, WidgetsPage) - EVT_BUTTON(StaticPage_Reset, StaticWidgetsPage::OnButtonReset) - EVT_BUTTON(StaticPage_LabelText, StaticWidgetsPage::OnButtonLabelText) -#if wxUSE_MARKUP - EVT_BUTTON(StaticPage_LabelTextWithMarkup, StaticWidgetsPage::OnButtonLabelWithMarkupText) -#endif // wxUSE_MARKUP - EVT_BUTTON(StaticPage_BoxText, StaticWidgetsPage::OnButtonBoxText) - - EVT_CHECKBOX(wxID_ANY, StaticWidgetsPage::OnCheckOrRadioBox) - EVT_RADIOBOX(wxID_ANY, StaticWidgetsPage::OnCheckOrRadioBox) -wxEND_EVENT_TABLE() - // ============================================================================ // implementation // ============================================================================ @@ -286,6 +260,8 @@ void StaticWidgetsPage::CreateContent() sizerLeft->Add(5, 5, 0, wxGROW | wxALL, 5); // spacer m_chkEllipsize = CreateCheckBoxAndAddToSizer(sizerLeft, "&Ellipsize"); + m_chkEllipsize->Bind(wxEVT_CHECKBOX, + &StaticWidgetsPage::OnCheckEllipsize, this); static const wxString ellipsizeMode[] = { @@ -301,8 +277,9 @@ void StaticWidgetsPage::CreateContent() sizerLeft->Add(m_radioEllipsize, 0, wxGROW | wxALL, 5); - wxButton *btn = new wxButton(this, StaticPage_Reset, "&Reset"); - sizerLeft->Add(btn, 0, wxALIGN_CENTRE_HORIZONTAL | wxALL, 15); + wxButton *b0 = new wxButton(this, wxID_ANY, "&Reset"); + b0->Bind(wxEVT_BUTTON, &StaticWidgetsPage::OnButtonReset, this); + sizerLeft->Add(b0, 0, wxALIGN_CENTRE_HORIZONTAL | wxALL, 15); // middle pane wxSizer *sizerMiddle = new wxStaticBoxSizer(wxVERTICAL, this, @@ -573,12 +550,9 @@ void StaticWidgetsPage::OnButtonReset(wxCommandEvent& WXUNUSED(event)) CreateStatic(); } -void StaticWidgetsPage::OnCheckOrRadioBox(wxCommandEvent& event) +void StaticWidgetsPage::OnCheckEllipsize(wxCommandEvent& event) { - if (event.GetEventObject() == static_cast(m_chkEllipsize)) - { - m_radioEllipsize->Enable(event.IsChecked()); - } + m_radioEllipsize->Enable(event.IsChecked()); CreateStatic(); } From 05627cf54c44178653a41d84b27eaf39795adf6f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 20 Jun 2019 01:44:27 +0200 Subject: [PATCH 12/16] Make wxControlBase::DoEllipsizeSingleLine() private function This function doesn't need to be a method of wxControl, so don't make it one. No real changes, just improve the encapsulation. --- include/wx/control.h | 5 ----- src/common/ctrlcmn.cpp | 12 ++++++------ 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/include/wx/control.h b/include/wx/control.h index 86563645e0..0e646b2faa 100644 --- a/include/wx/control.h +++ b/include/wx/control.h @@ -178,11 +178,6 @@ protected: // initialize the common fields of wxCommandEvent void InitCommandEvent(wxCommandEvent& event) const; - // Ellipsize() helper: - static wxString DoEllipsizeSingleLine(const wxString& label, const wxDC& dc, - wxEllipsizeMode mode, int maxWidth, - int replacementWidth); - #if wxUSE_MARKUP // Remove markup from the given string, returns empty string on error i.e. // if markup was syntactically invalid. diff --git a/src/common/ctrlcmn.cpp b/src/common/ctrlcmn.cpp index 3715caf07b..7768c08d70 100644 --- a/src/common/ctrlcmn.cpp +++ b/src/common/ctrlcmn.cpp @@ -403,12 +403,9 @@ struct EllipsizeCalculator bool m_isOk; }; -} // anonymous namespace - -/* static and protected */ -wxString wxControlBase::DoEllipsizeSingleLine(const wxString& curLine, const wxDC& dc, - wxEllipsizeMode mode, int maxFinalWidthPx, - int replacementWidthPx) +wxString DoEllipsizeSingleLine(const wxString& curLine, const wxDC& dc, + wxEllipsizeMode mode, int maxFinalWidthPx, + int replacementWidthPx) { wxASSERT_MSG(replacementWidthPx > 0, "Invalid parameters"); wxASSERT_LEVEL_2_MSG(!curLine.Contains('\n'), @@ -523,6 +520,9 @@ wxString wxControlBase::DoEllipsizeSingleLine(const wxString& curLine, const wxD return calc.GetEllipsizedText(); } +} // anonymous namespace + + /* static */ wxString wxControlBase::Ellipsize(const wxString& label, const wxDC& dc, wxEllipsizeMode mode, int maxFinalWidth, From a6b87746362451bd842ae95ca5d3a82ce76a3e4f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 20 Jun 2019 01:45:26 +0200 Subject: [PATCH 13/16] Run label unit tests for ellipsized wxStaticText too Check that the public methods still behave as expected even if the displayed value is different because it is ellipsized. --- tests/controls/label.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/controls/label.cpp b/tests/controls/label.cpp index 0e193196a8..94806a8cd7 100644 --- a/tests/controls/label.cpp +++ b/tests/controls/label.cpp @@ -98,6 +98,15 @@ TEST_CASE("wxControl::Label", "[wxControl][label]") DoTestLabel(st.get()); } + SECTION("wxStaticText/ellipsized") + { + const wxScopedPtr + st(new wxStaticText(wxTheApp->GetTopWindow(), wxID_ANY, ORIGINAL_LABEL, + wxDefaultPosition, wxDefaultSize, + wxST_ELLIPSIZE_START)); + DoTestLabel(st.get()); + } + SECTION("wxGenericStaticText") { const wxScopedPtr From 30726437c0935218805ad5042d47854d610a4b44 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 20 Jun 2019 01:46:28 +0200 Subject: [PATCH 14/16] Fix the result width check in the ellipsization unit test When processing mnemonics, the resulting string should still contain them and they need to be stripped before measuring its width, but the code didn't do it. This didn't prevent the tests from passing, but only due to another bug in ellipsization code itself, which lost the mnemonics completely. As this bug is about to be fixed, the test needs to take mnemonics into account properly now. --- tests/graphics/ellipsization.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/graphics/ellipsization.cpp b/tests/graphics/ellipsization.cpp index 2a584762f2..d7c90242b7 100644 --- a/tests/graphics/ellipsization.cpp +++ b/tests/graphics/ellipsization.cpp @@ -108,6 +108,16 @@ void EllipsizationTestCase::NormalCase() flagsToTest[f] ); + // Note that we must measure the width of the text that + // will be rendered, and when mnemonics are used, this + // means we have to remove them first. + const wxString + displayed = flagsToTest[f] & wxELLIPSIZE_FLAGS_PROCESS_MNEMONICS + ? wxControl::RemoveMnemonics(ret) + : ret; + const int + width = dc.GetMultiLineTextExtent(displayed).GetWidth(); + WX_ASSERT_MESSAGE ( ( @@ -115,10 +125,10 @@ void EllipsizationTestCase::NormalCase() s, f, m, str, ret, - dc.GetMultiLineTextExtent(ret).GetWidth(), + width, widthsToTest[w] ), - dc.GetMultiLineTextExtent(ret).GetWidth() <= widthsToTest[w] + width <= widthsToTest[w] ); } } From 63a40a09b298e0bc7b702b131c4ceac6e72a35a7 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 20 Jun 2019 01:48:33 +0200 Subject: [PATCH 15/16] Preserve mnemonics in ellipsized labels Ellipsization code was completely broken when used with the usual control label strings containing mnemonics: it simply stripped the mnemonics completely, losing them even if the label wasn't actually ellipsized, and turned "&&" into a mnemonic. I.e. "&Plug && play" appeared without underlined "P" but with underlined space before "play" before. Fix this by pretending that all ampersands in the string to be ellipsized have zero width. This is not precise, as the result of GetPartialTextExtents() for a string with the ampersands is not exactly the same as the sum of its result for the string without the ampersands and the width of the ampersands themselves, but it should be pretty close and unlikely to result in any problems in practice for the controls labels. At the very least this fixes the completely wrong behaviour of the controls on the "Static" page of the widgets sample, where ellipsization is enabled by default and setting the label text with mnemonics didn't work at all. --- src/common/ctrlcmn.cpp | 46 +++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/common/ctrlcmn.cpp b/src/common/ctrlcmn.cpp index 7768c08d70..8aabe0bfa7 100644 --- a/src/common/ctrlcmn.cpp +++ b/src/common/ctrlcmn.cpp @@ -282,7 +282,8 @@ namespace struct EllipsizeCalculator { EllipsizeCalculator(const wxString& s, const wxDC& dc, - int maxFinalWidthPx, int replacementWidthPx) + int maxFinalWidthPx, int replacementWidthPx, + int flags) : m_initialCharToRemove(0), m_nCharsToRemove(0), @@ -294,6 +295,31 @@ struct EllipsizeCalculator { m_isOk = dc.GetPartialTextExtents(s, m_charOffsetsPx); wxASSERT( m_charOffsetsPx.GetCount() == s.length() ); + + if ( flags & wxELLIPSIZE_FLAGS_PROCESS_MNEMONICS ) + { + // The ampersand itself shouldn't count for the width calculation + // as it won't be displayed: either it's an ampersand before some + // other character in which case it indicates a mnemonic, or it's + // an escaped ampersand, in which case only the second one of the + // pair of ampersands will be displayed. But we can't just remove + // the ampersand as we still need it in the final label, in order + // not to lose the mnemonics. Hence we use this hack, and pretend + // that ampersands simply have zero width. Of course, it could be + // not completely precise, but this is the best we can do without + // completely changing this code structure. + size_t n = 0; + int delta = 0; + for ( wxString::const_iterator it = s.begin(); + it != s.end(); + ++it, ++n ) + { + if ( *it == '&' ) + delta += dc.GetTextExtent(wxS('&')).GetWidth(); + + m_charOffsetsPx[n] -= delta; + } + } } bool IsOk() const { return m_isOk; } @@ -405,7 +431,7 @@ struct EllipsizeCalculator wxString DoEllipsizeSingleLine(const wxString& curLine, const wxDC& dc, wxEllipsizeMode mode, int maxFinalWidthPx, - int replacementWidthPx) + int replacementWidthPx, int flags) { wxASSERT_MSG(replacementWidthPx > 0, "Invalid parameters"); wxASSERT_LEVEL_2_MSG(!curLine.Contains('\n'), @@ -413,9 +439,6 @@ wxString DoEllipsizeSingleLine(const wxString& curLine, const wxDC& dc, wxASSERT_MSG( mode != wxELLIPSIZE_NONE, "shouldn't be called at all then" ); - // NOTE: this function assumes that any mnemonic/tab character has already - // been handled if it was necessary to handle them (see Ellipsize()) - if (maxFinalWidthPx <= 0) return wxEmptyString; @@ -423,7 +446,7 @@ wxString DoEllipsizeSingleLine(const wxString& curLine, const wxDC& dc, if (len <= 1 ) return curLine; - EllipsizeCalculator calc(curLine, dc, maxFinalWidthPx, replacementWidthPx); + EllipsizeCalculator calc(curLine, dc, maxFinalWidthPx, replacementWidthPx, flags); if ( !calc.IsOk() ) return curLine; @@ -545,7 +568,7 @@ wxString wxControlBase::Ellipsize(const wxString& label, const wxDC& dc, if ( pc == label.end() || *pc == wxS('\n') ) { curLine = DoEllipsizeSingleLine(curLine, dc, mode, maxFinalWidth, - replacementWidth); + replacementWidth, flags); // add this (ellipsized) row to the rest of the label ret << curLine; @@ -555,15 +578,6 @@ wxString wxControlBase::Ellipsize(const wxString& label, const wxDC& dc, ret << *pc; curLine.clear(); } - // we need to remove mnemonics from the label for correct calculations - else if ( *pc == wxS('&') && (flags & wxELLIPSIZE_FLAGS_PROCESS_MNEMONICS) ) - { - // pc+1 is safe: at worst we'll be at end() - wxString::const_iterator next = pc + 1; - if ( next != label.end() && *next == wxS('&') ) - curLine += wxS('&'); // && becomes & - //else: remove this ampersand - } // we need also to expand tabs to properly calc their size else if ( *pc == wxS('\t') && (flags & wxELLIPSIZE_FLAGS_EXPAND_TABS) ) { From 958521183a2547123dfe7fb53e9a726002b08593 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 7 Jul 2019 15:54:14 +0200 Subject: [PATCH 16/16] Add minimal support for ellipsization to wxQt wxStaticText Set the visible label to the ellipsized value, if necessary. Also call AutoResizeIfNecessary() for consistency with the other ports. --- include/wx/qt/stattext.h | 5 ++++- src/qt/stattext.cpp | 17 ++++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/include/wx/qt/stattext.h b/include/wx/qt/stattext.h index f427f16490..5a19698191 100644 --- a/include/wx/qt/stattext.h +++ b/include/wx/qt/stattext.h @@ -31,10 +31,13 @@ public: const wxString &name = wxStaticTextNameStr ); virtual void SetLabel(const wxString& label) wxOVERRIDE; - virtual wxString GetLabel() const wxOVERRIDE; virtual QWidget *GetHandle() const wxOVERRIDE; +protected: + virtual wxString WXGetVisibleLabel() const wxOVERRIDE; + virtual void WXSetVisibleLabel(const wxString& str) wxOVERRIDE; + private: QLabel *m_qtLabel; diff --git a/src/qt/stattext.cpp b/src/qt/stattext.cpp index 2eb50b5f18..28e68b08be 100644 --- a/src/qt/stattext.cpp +++ b/src/qt/stattext.cpp @@ -59,11 +59,26 @@ bool wxStaticText::Create(wxWindow *parent, } void wxStaticText::SetLabel(const wxString& label) +{ + // If the label doesn't really change, avoid flicker by not doing anything. + if ( label == m_labelOrig ) + return; + + // save the label in m_labelOrig with both the markup (if any) and + // the mnemonics characters (if any) + m_labelOrig = label; + + WXSetVisibleLabel(GetEllipsizedLabel()); + + AutoResizeIfNecessary(); +} + +void wxStaticText::WXSetVisibleLabel(const wxString& label) { m_qtLabel->setText( wxQtConvertString( label ) ); } -wxString wxStaticText::GetLabel() const +wxString wxStaticText::WXGetVisibleLabel() const { return wxQtConvertString( m_qtLabel->text() ); }