Don't require leading TAB in wxAcceleratorEntry::FromString().

FromString() should parse string returned by ToString() successfully but this
wasn't the case because the accelerator parsing functions always insisted on
having a TAB in the string.

Fix this, document the string format and add a unit test checking for the
correct behaviour.

Closes #12745.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@66308 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

interface/wx/accel.h

@@ -103,10 +103,11 @@ public:
     bool IsOk() const;
 
     /**
-        Returns a wxString for this accelerator.
+        Returns a textual representation of this accelerator.
 
-        This function formats it using the @c "flags-keycode" format
+        The returned string is of the form <code>[Alt+][Ctrl+][Shift+]Key</code>
-        where @c flags maybe a hyphen-separed list of @c "shift|alt|ctrl".
+        where the modifier keys are present only if the corresponding flag is
+        set.
     */
     wxString ToString() const;
 
@@ -114,7 +115,11 @@ public:
         Parses the given string and sets the accelerator accordingly.
 
         @param str
-            Should be a string in the form "flags-keycode"
+            This string may be either in the same format as returned by
+            ToString(), i.e. contain the accelerator itself only, or have the
+            format of a full menu item text with i.e. <code>Label TAB
+            Accelerator</code>. In the latter case, the part of the string
+            before the TAB is ignored.
 
         @return @true if the given string correctly initialized this object
                 (i.e. if IsOk() returns true after this call)

src/common/accelcmn.cpp

@@ -160,17 +160,19 @@ wxAcceleratorEntry::ParseAccel(const wxString& text, int *flagsOut, int *keyOut)
     wxString label = text;
     label.Trim(true);  // the initial \t must be preserved so don't strip leading whitespaces
 
+    // If we're passed the entire menu item label instead of just the
+    // accelerator, skip the label part and only look after the TAB.
     // check for accelerators: they are given after '\t'
     int posTab = label.Find(wxT('\t'));
     if ( posTab == wxNOT_FOUND )
-    {
+        posTab = 0;
-        return false;
+    else
-    }
+        posTab++;
 
     // parse the accelerator string
     int accelFlags = wxACCEL_NORMAL;
     wxString current;
-    for ( size_t n = (size_t)posTab + 1; n < label.length(); n++ )
+    for ( size_t n = (size_t)posTab; n < label.length(); n++ )
     {
         if ( (label[n] == '+') || (label[n] == '-') )
         {

tests/Makefile.in

@@ -202,6 +202,7 @@ TEST_GUI_OBJECTS =  \
 	test_gui_image.o \
 	test_gui_rawbmp.o \
 	test_gui_htmlwindow.o \
+	test_gui_accelentry.o \
 	test_gui_menu.o \
 	test_gui_guifuncs.o \
 	test_gui_selstoretest.o \
@@ -829,6 +830,9 @@ test_gui_rawbmp.o: $(srcdir)/image/rawbmp.cpp $(TEST_GUI_ODEP)
 test_gui_htmlwindow.o: $(srcdir)/html/htmlwindow.cpp $(TEST_GUI_ODEP)
 	$(CXXC) -c -o $@ $(TEST_GUI_CXXFLAGS) $(srcdir)/html/htmlwindow.cpp
 
+test_gui_accelentry.o: $(srcdir)/menu/accelentry.cpp $(TEST_GUI_ODEP)
+	$(CXXC) -c -o $@ $(TEST_GUI_CXXFLAGS) $(srcdir)/menu/accelentry.cpp
+
 test_gui_menu.o: $(srcdir)/menu/menu.cpp $(TEST_GUI_ODEP)
 	$(CXXC) -c -o $@ $(TEST_GUI_CXXFLAGS) $(srcdir)/menu/menu.cpp
 

tests/makefile.bcc

@@ -153,7 +153,7 @@ TEST_GUI_OBJECTS =  \
 	$(OBJS)\test_gui_gaugetest.obj \
 	$(OBJS)\test_gui_gridtest.obj \
 	$(OBJS)\test_gui_headerctrltest.obj \
-	$(OBJS)\test_gui_htmllboxtest.obj \
+	$(OBJS)\test_gui_htmllboxtest.obj \
 	$(OBJS)\test_gui_hyperlinkctrltest.obj \
 	$(OBJS)\test_gui_itemcontainertest.obj \
 	$(OBJS)\test_gui_label.obj \
@@ -187,6 +187,7 @@ TEST_GUI_OBJECTS =  \
 	$(OBJS)\test_gui_image.obj \
 	$(OBJS)\test_gui_rawbmp.obj \
 	$(OBJS)\test_gui_htmlwindow.obj \
+	$(OBJS)\test_gui_accelentry.obj \
 	$(OBJS)\test_gui_menu.obj \
 	$(OBJS)\test_gui_guifuncs.obj \
 	$(OBJS)\test_gui_selstoretest.obj \
@@ -773,9 +774,9 @@ $(OBJS)\test_gui_gridtest.obj: .\controls\gridtest.cpp
 $(OBJS)\test_gui_headerctrltest.obj: .\controls\headerctrltest.cpp
 	$(CXX) -q -c -P -o$@ $(TEST_GUI_CXXFLAGS) .\controls\headerctrltest.cpp
 
-$(OBJS)\test_gui_htmllboxtest.obj: .\controls\htmllboxtest.cpp
+$(OBJS)\test_gui_htmllboxtest.obj: .\controls\htmllboxtest.cpp
-	$(CXX) -q -c -P -o$@ $(TEST_GUI_CXXFLAGS) .\controls\htmllboxtest.cpp
+	$(CXX) -q -c -P -o$@ $(TEST_GUI_CXXFLAGS) .\controls\htmllboxtest.cpp
-
+
 $(OBJS)\test_gui_hyperlinkctrltest.obj: .\controls\hyperlinkctrltest.cpp
 	$(CXX) -q -c -P -o$@ $(TEST_GUI_CXXFLAGS) .\controls\hyperlinkctrltest.cpp
 
@@ -875,6 +876,9 @@ $(OBJS)\test_gui_rawbmp.obj: .\image\rawbmp.cpp
 $(OBJS)\test_gui_htmlwindow.obj: .\html\htmlwindow.cpp
 	$(CXX) -q -c -P -o$@ $(TEST_GUI_CXXFLAGS) .\html\htmlwindow.cpp
 
+$(OBJS)\test_gui_accelentry.obj: .\menu\accelentry.cpp
+	$(CXX) -q -c -P -o$@ $(TEST_GUI_CXXFLAGS) .\menu\accelentry.cpp
+
 $(OBJS)\test_gui_menu.obj: .\menu\menu.cpp
 	$(CXX) -q -c -P -o$@ $(TEST_GUI_CXXFLAGS) .\menu\menu.cpp
 

tests/makefile.gcc

@@ -146,7 +146,7 @@ TEST_GUI_OBJECTS =  \
 	$(OBJS)\test_gui_gaugetest.o \
 	$(OBJS)\test_gui_gridtest.o \
 	$(OBJS)\test_gui_headerctrltest.o \
-	$(OBJS)\test_gui_htmllboxtest.o \
+	$(OBJS)\test_gui_htmllboxtest.o \
 	$(OBJS)\test_gui_hyperlinkctrltest.o \
 	$(OBJS)\test_gui_itemcontainertest.o \
 	$(OBJS)\test_gui_label.o \
@@ -180,6 +180,7 @@ TEST_GUI_OBJECTS =  \
 	$(OBJS)\test_gui_image.o \
 	$(OBJS)\test_gui_rawbmp.o \
 	$(OBJS)\test_gui_htmlwindow.o \
+	$(OBJS)\test_gui_accelentry.o \
 	$(OBJS)\test_gui_menu.o \
 	$(OBJS)\test_gui_guifuncs.o \
 	$(OBJS)\test_gui_selstoretest.o \
@@ -754,9 +755,9 @@ $(OBJS)\test_gui_gridtest.o: ./controls/gridtest.cpp
 $(OBJS)\test_gui_headerctrltest.o: ./controls/headerctrltest.cpp
 	$(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $<
 
-$(OBJS)\test_gui_htmllboxtest.o: ./controls/htmllboxtest.cpp
+$(OBJS)\test_gui_htmllboxtest.o: ./controls/htmllboxtest.cpp
-	$(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $<
+	$(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $<
-
+
 $(OBJS)\test_gui_hyperlinkctrltest.o: ./controls/hyperlinkctrltest.cpp
 	$(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $<
 
@@ -856,6 +857,9 @@ $(OBJS)\test_gui_rawbmp.o: ./image/rawbmp.cpp
 $(OBJS)\test_gui_htmlwindow.o: ./html/htmlwindow.cpp
 	$(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $<
 
+$(OBJS)\test_gui_accelentry.o: ./menu/accelentry.cpp
+	$(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $<
+
 $(OBJS)\test_gui_menu.o: ./menu/menu.cpp
 	$(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $<
 

tests/makefile.vc

@@ -148,7 +148,7 @@ TEST_GUI_OBJECTS =  \
 	$(OBJS)\test_gui_gaugetest.obj \
 	$(OBJS)\test_gui_gridtest.obj \
 	$(OBJS)\test_gui_headerctrltest.obj \
-	$(OBJS)\test_gui_htmllboxtest.obj \
+	$(OBJS)\test_gui_htmllboxtest.obj \
 	$(OBJS)\test_gui_hyperlinkctrltest.obj \
 	$(OBJS)\test_gui_itemcontainertest.obj \
 	$(OBJS)\test_gui_label.obj \
@@ -182,6 +182,7 @@ TEST_GUI_OBJECTS =  \
 	$(OBJS)\test_gui_image.obj \
 	$(OBJS)\test_gui_rawbmp.obj \
 	$(OBJS)\test_gui_htmlwindow.obj \
+	$(OBJS)\test_gui_accelentry.obj \
 	$(OBJS)\test_gui_menu.obj \
 	$(OBJS)\test_gui_guifuncs.obj \
 	$(OBJS)\test_gui_selstoretest.obj \
@@ -899,9 +900,9 @@ $(OBJS)\test_gui_gridtest.obj: .\controls\gridtest.cpp
 $(OBJS)\test_gui_headerctrltest.obj: .\controls\headerctrltest.cpp
 	$(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\controls\headerctrltest.cpp
 
-$(OBJS)\test_gui_htmllboxtest.obj: .\controls\htmllboxtest.cpp
+$(OBJS)\test_gui_htmllboxtest.obj: .\controls\htmllboxtest.cpp
-	$(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\controls\htmllboxtest.cpp
+	$(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\controls\htmllboxtest.cpp
-
+
 $(OBJS)\test_gui_hyperlinkctrltest.obj: .\controls\hyperlinkctrltest.cpp
 	$(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\controls\hyperlinkctrltest.cpp
 
@@ -1001,6 +1002,9 @@ $(OBJS)\test_gui_rawbmp.obj: .\image\rawbmp.cpp
 $(OBJS)\test_gui_htmlwindow.obj: .\html\htmlwindow.cpp
 	$(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\html\htmlwindow.cpp
 
+$(OBJS)\test_gui_accelentry.obj: .\menu\accelentry.cpp
+	$(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\menu\accelentry.cpp
+
 $(OBJS)\test_gui_menu.obj: .\menu\menu.cpp
 	$(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\menu\menu.cpp
 

tests/makefile.wat

@@ -388,7 +388,7 @@ TEST_GUI_OBJECTS =  &
 	$(OBJS)\test_gui_gaugetest.obj &
 	$(OBJS)\test_gui_gridtest.obj &
 	$(OBJS)\test_gui_headerctrltest.obj &
-	$(OBJS)\test_gui_htmllboxtest.obj &
+	$(OBJS)\test_gui_htmllboxtest.obj &
 	$(OBJS)\test_gui_hyperlinkctrltest.obj &
 	$(OBJS)\test_gui_itemcontainertest.obj &
 	$(OBJS)\test_gui_label.obj &
@@ -422,6 +422,7 @@ TEST_GUI_OBJECTS =  &
 	$(OBJS)\test_gui_image.obj &
 	$(OBJS)\test_gui_rawbmp.obj &
 	$(OBJS)\test_gui_htmlwindow.obj &
+	$(OBJS)\test_gui_accelentry.obj &
 	$(OBJS)\test_gui_menu.obj &
 	$(OBJS)\test_gui_guifuncs.obj &
 	$(OBJS)\test_gui_selstoretest.obj &
@@ -812,9 +813,9 @@ $(OBJS)\test_gui_gridtest.obj :  .AUTODEPEND .\controls\gridtest.cpp
 $(OBJS)\test_gui_headerctrltest.obj :  .AUTODEPEND .\controls\headerctrltest.cpp
 	$(CXX) -bt=nt -zq -fo=$^@ $(TEST_GUI_CXXFLAGS) $<
 
-$(OBJS)\test_gui_htmllboxtest.obj :  .AUTODEPEND .\controls\htmllboxtest.cpp
+$(OBJS)\test_gui_htmllboxtest.obj :  .AUTODEPEND .\controls\htmllboxtest.cpp
-	$(CXX) -bt=nt -zq -fo=$^@ $(TEST_GUI_CXXFLAGS) $<
+	$(CXX) -bt=nt -zq -fo=$^@ $(TEST_GUI_CXXFLAGS) $<
-
+
 $(OBJS)\test_gui_hyperlinkctrltest.obj :  .AUTODEPEND .\controls\hyperlinkctrltest.cpp
 	$(CXX) -bt=nt -zq -fo=$^@ $(TEST_GUI_CXXFLAGS) $<
 
@@ -914,6 +915,9 @@ $(OBJS)\test_gui_rawbmp.obj :  .AUTODEPEND .\image\rawbmp.cpp
 $(OBJS)\test_gui_htmlwindow.obj :  .AUTODEPEND .\html\htmlwindow.cpp
 	$(CXX) -bt=nt -zq -fo=$^@ $(TEST_GUI_CXXFLAGS) $<
 
+$(OBJS)\test_gui_accelentry.obj :  .AUTODEPEND .\menu\accelentry.cpp
+	$(CXX) -bt=nt -zq -fo=$^@ $(TEST_GUI_CXXFLAGS) $<
+
 $(OBJS)\test_gui_menu.obj :  .AUTODEPEND .\menu\menu.cpp
 	$(CXX) -bt=nt -zq -fo=$^@ $(TEST_GUI_CXXFLAGS) $<
 

tests/menu/accelentry.cpp

@@ -0,0 +1,90 @@
+///////////////////////////////////////////////////////////////////////////////
+// Name:        tests/menu/accelentry.cpp
+// Purpose:     wxAcceleratorEntry unit test
+// Author:      Vadim Zeitlin
+// Created:     2010-12-03
+// RCS-ID:      $Id$
+// Copyright:   (c) 2010 Vadim Zeitlin
+///////////////////////////////////////////////////////////////////////////////
+
+// ----------------------------------------------------------------------------
+// headers
+// ----------------------------------------------------------------------------
+
+#include "testprec.h"
+
+#ifdef __BORLANDC__
+    #pragma hdrstop
+#endif
+
+#ifndef WX_PRECOMP
+#endif // WX_PRECOMP
+
+#include "wx/accel.h"
+#include "wx/scopedptr.h"
+
+class AccelEntryTestCase : public CppUnit::TestCase
+{
+public:
+    AccelEntryTestCase() {}
+
+private:
+    CPPUNIT_TEST_SUITE( AccelEntryTestCase );
+        CPPUNIT_TEST( Create );
+        CPPUNIT_TEST( ToFromString );
+    CPPUNIT_TEST_SUITE_END();
+
+    void Create();
+    void ToFromString();
+
+    wxDECLARE_NO_COPY_CLASS(AccelEntryTestCase);
+};
+
+// register in the unnamed registry so that these tests are run by default
+CPPUNIT_TEST_SUITE_REGISTRATION( AccelEntryTestCase );
+
+// also include in it's own registry so that these tests can be run alone
+CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( AccelEntryTestCase, "AccelEntryTestCase" );
+
+namespace
+{
+
+void CheckAccelEntry(const wxAcceleratorEntry& accel, int keycode, int flags)
+{
+    CPPUNIT_ASSERT_EQUAL( keycode, accel.GetKeyCode() );
+    CPPUNIT_ASSERT_EQUAL( flags, accel.GetFlags() );
+}
+
+} // anonymous namespace
+
+void AccelEntryTestCase::Create()
+{
+    wxScopedPtr<wxAcceleratorEntry>
+        pa(wxAcceleratorEntry::Create("Foo\tCtrl+Z"));
+    CPPUNIT_ASSERT( pa );
+    CPPUNIT_ASSERT( pa->IsOk() );
+
+    CheckAccelEntry(*pa, 'Z', wxACCEL_CTRL);
+
+
+    pa.reset(wxAcceleratorEntry::Create("Shift-Q"));
+    CPPUNIT_ASSERT( pa );
+    CPPUNIT_ASSERT( pa->IsOk() );
+
+    CheckAccelEntry(*pa, 'Q', wxACCEL_SHIFT);
+
+
+    pa.reset(wxAcceleratorEntry::Create("bloordyblop"));
+    CPPUNIT_ASSERT( !pa );
+}
+
+void AccelEntryTestCase::ToFromString()
+{
+    wxAcceleratorEntry a(wxACCEL_ALT, 'X');
+    CPPUNIT_ASSERT_EQUAL( "Alt+X", a.ToString() );
+
+    CPPUNIT_ASSERT( a.FromString("Alt+Shift+F1") );
+    CheckAccelEntry(a, WXK_F1, wxACCEL_ALT | wxACCEL_SHIFT);
+
+    CPPUNIT_ASSERT( !a.FromString("bloordyblop") );
+}

tests/test.bkl

@@ -183,6 +183,7 @@
             image/image.cpp
             image/rawbmp.cpp
             html/htmlwindow.cpp
+            menu/accelentry.cpp
             menu/menu.cpp
             misc/guifuncs.cpp
             misc/selstoretest.cpp

tests/test_test_gui.dsp

@@ -235,6 +235,10 @@ LINK32=link.exe
 # PROP Default_Filter ""
 # Begin Source File
 
+SOURCE=.\menu\accelentry.cpp
+# End Source File
+# Begin Source File
+
 SOURCE=.\asserthelper.cpp
 # End Source File
 # Begin Source File
@@ -337,10 +341,10 @@ SOURCE=.\controls\headerctrltest.cpp
 # End Source File
 # Begin Source File
 
-SOURCE=.\controls\htmllboxtest.cpp
+SOURCE=.\controls\htmllboxtest.cpp
-# End Source File
+# End Source File
-# Begin Source File
+# Begin Source File
-
+
 SOURCE=.\html\htmlwindow.cpp
 # End Source File
 # Begin Source File

tests/test_vc7_test_gui.vcproj

@@ -565,6 +565,9 @@
 			Name="Source Files"
 			Filter="cpp;c;cc;cxx;def;odl;idl;hpj;bat;asm;asmx"
 			UniqueIdentifier="{4FC737F1-C7A5-4376-A066-2A32D752A2FF}">
+			<File
+				RelativePath=".\menu\accelentry.cpp">
+			</File>
 			<File
 				RelativePath=".\asserthelper.cpp">
 			</File>
@@ -689,9 +692,9 @@
 				RelativePath=".\controls\headerctrltest.cpp">
 			</File>
 			<File
-				RelativePath=".\controls\htmllboxtest.cpp">
+				RelativePath=".\controls\htmllboxtest.cpp">
-			</File>
+			</File>
-			<File
+			<File
 				RelativePath=".\html\htmlwindow.cpp">
 			</File>
 			<File

tests/test_vc8_test_gui.vcproj

@@ -827,6 +827,10 @@
 			Filter="cpp;c;cc;cxx;def;odl;idl;hpj;bat;asm;asmx"
 			UniqueIdentifier="{4FC737F1-C7A5-4376-A066-2A32D752A2FF}"
 			>
+			<File
+				RelativePath=".\menu\accelentry.cpp"
+				>
+			</File>
 			<File
 				RelativePath=".\asserthelper.cpp"
 				>
@@ -992,10 +996,10 @@
 				>
 			</File>
 			<File
-				RelativePath=".\controls\htmllboxtest.cpp"
+				RelativePath=".\controls\htmllboxtest.cpp"
-				>
+				>
-			</File>
+			</File>
-			<File
+			<File
 				RelativePath=".\html\htmlwindow.cpp"
 				>
 			</File>

tests/test_vc9_test_gui.vcproj

@@ -799,6 +799,10 @@
 			Filter="cpp;c;cc;cxx;def;odl;idl;hpj;bat;asm;asmx"
 			UniqueIdentifier="{4FC737F1-C7A5-4376-A066-2A32D752A2FF}"
 			>
+			<File
+				RelativePath=".\menu\accelentry.cpp"
+				>
+			</File>
 			<File
 				RelativePath=".\asserthelper.cpp"
 				>
@@ -964,10 +968,10 @@
 				>
 			</File>
 			<File
-				RelativePath=".\controls\htmllboxtest.cpp"
+				RelativePath=".\controls\htmllboxtest.cpp"
-				>
+				>
-			</File>
+			</File>
-			<File
+			<File
 				RelativePath=".\html\htmlwindow.cpp"
 				>
 			</File>