fixes to menu stock items support (patch 1547639)

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

docs/latex/wx/menuitem.tex

@@ -42,11 +42,36 @@ only implemented for Windows and GTK+.
 
 Constructs a wxMenuItem object.
 
-The preferred way to create standard menu items is to use default value of
+Menu items can be standard, or ``stock menu items'', or custom. For the
-\arg{text}. If no text is supplied and \arg{id} is one of standard IDs from
+standard menu items (such as commands to open a file, exit the program and so
-\helpref{this list}{stockitems}, a standard label and a standard accelerator
+on, see \helpref{stock items}{stockitems} for the full list) it is enough to
-will be used. In addition to that, the button will be decorated with stock
+specify just the stock ID and leave \arg{text} and \arg{helpString} empty. In
-icons under GTK+ 2.
+fact, leaving at least \arg{text} empty for the stock menu items is strongly
+recommended as they will have appearance and keyboard interface (including
+standard accelerators) familiar to the user.
+
+For the custom (non-stock) menu items, \arg{text} must be specified and while 
+\arg{helpString} may be left empty, it's recommended to pass the item
+description (which is automatically shown by the library in the status bar when
+the menu item is selected) in this parameter.
+
+Finally note that you can e.g. use a stock menu label without using its stock
+help string:
+
+\begin{verbatim}
+// use all stock properties:
+helpMenu->Append(wxID_ABOUT);
+
+// use the stock label and the stock accelerator but not the stock help string:
+helpMenu->Append(wxID_ABOUT, wxEmptyString, wxT("My custom help string"));
+
+// use all stock properties except for the bitmap:
+wxMenuItem *mymenu = new wxMenuItem(helpMenu, wxID_ABOUT);
+mymenu->SetBitmap(wxArtProvider::GetBitmap(wxART_WARNING));
+helpMenu->Append(mymenu);
+\end{verbatim}
+
+that is, stock properties are set independently one from the other.
 
 \wxheading{Parameters}
 

docs/latex/wx/stockitems.tex

@@ -1,9 +1,11 @@
 \section{Stock items}\label{stockitems}
 
-Window IDs for which stock buttons are created
+Window IDs for which stock buttons and menu items are created
-(see \helpref{wxButton constructor}{wxbuttonctor}):
+(see \helpref{wxButton constructor}{wxbuttonctor} and 
+\helpref{wxMenuItem constructor}{wxmenuitemctor}):
 
 \begin{twocollist}\itemsep=0pt
+\twocolitem{{\bf Stock ID}}{{\bf Stock label}}
 \twocolitem{wxID\_ABOUT}{"\&About"}
 \twocolitem{wxID\_ADD}{"Add"}
 \twocolitem{wxID\_APPLY}{"\&Apply"}
@@ -58,4 +60,8 @@ Window IDs for which stock buttons are created
 \twocolitem{wxID\_ZOOM\_FIT}{"Zoom to \&Fit"}
 \twocolitem{wxID\_ZOOM\_IN}{"Zoom \&In"}
 \twocolitem{wxID\_ZOOM\_OUT}{"Zoom \&Out"}
-\end{twocollist}\itemsep=0pt
+\end{twocollist}
+
+
+Note that some of the IDs listed above have also a stock accelerator
+and an help string associated.

include/wx/menuitem.h

@@ -64,7 +64,7 @@ public:
     //     any), i.e. it may contain '&' or '_' or "\t..." and thus is
     //     different from the item's label which only contains the text shown
     //     in the menu
-    virtual void SetText(const wxString& str) { m_text = str; }
+    virtual void SetText(const wxString& str);
     wxString GetLabel() const { return GetLabelFromText(m_text); }
     const wxString& GetText() const { return m_text; }
 
@@ -92,7 +92,7 @@ public:
     void Toggle() { Check(!m_isChecked); }
 
     // help string (displayed in the status bar by default)
-    void SetHelp(const wxString& str) { m_help = str; }
+    void SetHelp(const wxString& str);
     const wxString& GetHelp() const { return m_help; }
 
 #if wxUSE_ACCEL

include/wx/stockitem.h

@@ -51,6 +51,18 @@ WXDLLEXPORT wxString wxGetStockLabel(wxWindowID id,
 
 #endif
 
+// wxStockHelpStringClient conceptually works like wxArtClient: it gives a hint to
+// wxGetStockHelpString() about the context where the help string is to be used
+enum wxStockHelpStringClient
+{
+    wxSTOCK_MENU        // help string to use for menu items
+};
+
+// Returns an help string for the given stock UI element and for the given "context".
+WXDLLEXPORT wxString wxGetStockHelpString(wxWindowID id,
+                                          wxStockHelpStringClient client = wxSTOCK_MENU);
+
+
 #ifdef __WXGTK20__
 
 #include <gdk/gdktypes.h>

src/common/menucmn.cpp

@@ -34,6 +34,8 @@
     #include "wx/menu.h"
 #endif
 
+#include "wx/stockitem.h"
+
 // ----------------------------------------------------------------------------
 // template lists
 // ----------------------------------------------------------------------------
@@ -174,13 +176,17 @@ static int
 bool
 wxAcceleratorEntry::ParseAccel(const wxString& text, int *flagsOut, int *keyOut)
 {
-    // the parser won't like leading/trailing spaces
+    // the parser won't like trailing spaces
-    wxString label = text.Strip(wxString::both);
+    wxString label = text;
+    label.Trim(true);  // the initial \t must be preserved so don't strip leading whitespaces
 
     // check for accelerators: they are given after '\t'
     int posTab = label.Find(wxT('\t'));
     if ( posTab == wxNOT_FOUND )
+    {
+        wxLogDebug(wxT("Invalid menu label: no accelerators"));
         return false;
+    }
 
     // parse the accelerator string
     int accelFlags = wxACCEL_NORMAL;
@@ -360,8 +366,6 @@ wxMenuItemBase::wxMenuItemBase(wxMenu *parentMenu,
                                const wxString& help,
                                wxItemKind kind,
                                wxMenu *subMenu)
-              : m_text(text),
-                m_help(help)
 {
     wxASSERT_MSG( parentMenu != NULL, wxT("menuitem should have a menu") );
 
@@ -375,6 +379,9 @@ wxMenuItemBase::wxMenuItemBase(wxMenu *parentMenu,
         m_id = wxNewId();
     if (m_id == wxID_SEPARATOR)
         m_kind = wxITEM_SEPARATOR;
+
+    SetText(text);
+    SetHelp(help);
 }
 
 wxMenuItemBase::~wxMenuItemBase()
@@ -403,6 +410,30 @@ void wxMenuItemBase::SetAccel(wxAcceleratorEntry *accel)
 
 #endif // wxUSE_ACCEL
 
+void wxMenuItemBase::SetText(const wxString& str)
+{
+    m_text = str;
+
+    if ( m_text.empty() && !IsSeparator() )
+    {
+        wxASSERT_MSG( wxIsStockID(GetId()),
+                      wxT("A non-stock menu item with an empty label?") );
+        m_text = wxGetStockLabel(GetId(), wxSTOCK_WITH_ACCELERATOR |
+                                          wxSTOCK_WITH_MNEMONIC);
+    }
+}
+
+void wxMenuItemBase::SetHelp(const wxString& str)
+{
+    m_help = str;
+
+    if ( m_help.empty() && !IsSeparator() && wxIsStockID(GetId()) )
+    {
+        // get a stock help string
+        m_help = wxGetStockHelpString(GetId());
+    }
+}
+
 bool wxMenuBase::ms_locked = true;
 
 // ----------------------------------------------------------------------------

src/common/stockitem.cpp

@@ -190,6 +190,42 @@ wxString wxGetStockLabel(wxWindowID id, long flags)
     return stockLabel;
 }
 
+wxString wxGetStockHelpString(wxWindowID id, wxStockHelpStringClient client)
+{
+    wxString stockHelp;
+
+    #define STOCKITEM(stockid, ctx, helpstr)             \
+        case stockid:                                    \
+            if (client==ctx) stockHelp = helpstr;        \
+            break;
+
+    switch (id)
+    {
+        // NB: these help string should be not too specific as they could be used
+        //     in completely different programs!
+        STOCKITEM(wxID_ABOUT,    wxSTOCK_MENU, _("Show about dialog"))
+        STOCKITEM(wxID_COPY,     wxSTOCK_MENU, _("Copy selection"))
+        STOCKITEM(wxID_CUT,      wxSTOCK_MENU, _("Cut selection"))
+        STOCKITEM(wxID_DELETE,   wxSTOCK_MENU, _("Delete selection"))
+        STOCKITEM(wxID_REPLACE,  wxSTOCK_MENU, _("Replace selection"))
+        STOCKITEM(wxID_PASTE,    wxSTOCK_MENU, _("Paste selection"))
+        STOCKITEM(wxID_EXIT,     wxSTOCK_MENU, _("Quit this program"))
+        STOCKITEM(wxID_REDO,     wxSTOCK_MENU, _("Redo last action"))
+        STOCKITEM(wxID_UNDO,     wxSTOCK_MENU, _("Undo last action"))
+        STOCKITEM(wxID_CLOSE,    wxSTOCK_MENU, _("Close current document"))
+        STOCKITEM(wxID_SAVE,     wxSTOCK_MENU, _("Save current document"))
+        STOCKITEM(wxID_SAVEAS,   wxSTOCK_MENU, _("Save current document with a different filename"))
+
+        default:
+            // there's no stock help string for this ID / client
+            return wxEmptyString;
+    }
+
+    #undef STOCKITEM
+
+    return stockHelp;
+}
+
 #if wxUSE_ACCEL
 
 wxAcceleratorEntry wxGetStockAccelerator(wxWindowID id)

src/gtk1/menu.cpp

@@ -769,10 +769,11 @@ wxString wxMenuItemBase::GetLabelFromText(const wxString& text)
 void wxMenuItem::SetText( const wxString& string )
 {
     wxString str = string;
-    if (str.IsEmpty())
+    if ( str.empty() && !IsSeparator() )
     {
         wxASSERT_MSG(wxIsStockID(GetId()), wxT("A non-stock menu item with an empty label?"));
-        str = wxGetStockLabel(GetId(), wxSTOCK_WITH_ACCELERATOR|wxSTOCK_WITH_MNEMONIC);
+        str = wxGetStockLabel(GetId(), wxSTOCK_WITH_ACCELERATOR |
+                                       wxSTOCK_WITH_MNEMONIC);
     }
 
     // Some optimization to avoid flicker

src/msw/menuitem.cpp

@@ -154,12 +154,6 @@ wxMenuItem::wxMenuItem(wxMenu *parentMenu,
 
 void wxMenuItem::Init()
 {
-    if (m_text.IsEmpty())
-    {
-        wxASSERT_MSG(wxIsStockID(GetId()), wxT("A non-stock menu item with an empty label?"));
-        m_text = wxGetStockLabel(GetId(), wxSTOCK_WITH_ACCELERATOR|wxSTOCK_WITH_MNEMONIC);
-    }
-
     m_radioGroup.start = -1;
     m_isRadioGroupStart = false;
 
@@ -351,17 +345,16 @@ void wxMenuItem::SetText(const wxString& txt)
     if ( m_text == txt )
         return;
 
-    if (text.IsEmpty())
+    // wxMenuItemBase will do stock ID checks
-    {
-        wxASSERT_MSG(wxIsStockID(GetId()), wxT("A non-stock menu item with an empty label?"));
-        text = wxGetStockLabel(GetId(), wxSTOCK_WITH_ACCELERATOR|wxSTOCK_WITH_MNEMONIC);
-    }
-
     wxMenuItemBase::SetText(text);
-    OWNER_DRAWN_ONLY( wxOwnerDrawn::SetName(text) );
+
+    // m_text could now be different from 'text' if we are a stock menu item,
+    // so use only m_text below
+
+    OWNER_DRAWN_ONLY( wxOwnerDrawn::SetName(m_text) );
 #if wxUSE_OWNER_DRAWN
     // tell the owner drawing code to to show the accel string as well
-    SetAccelString(text.AfterFirst(_T('\t')));
+    SetAccelString(m_text.AfterFirst(_T('\t')));
 #endif
 
     HMENU hMenu = GetHMenuOf(m_parentMenu);
@@ -399,7 +392,7 @@ void wxMenuItem::SetText(const wxString& txt)
 #endif  //owner drawn
         {
             flagsOld |= MF_STRING;
-            data = (wxChar*) text.c_str();
+            data = (wxChar*) m_text.c_str();
         }
 
 #ifdef __WXWINCE__
@@ -413,7 +406,7 @@ void wxMenuItem::SetText(const wxString& txt)
         info.cbSize = sizeof(info);
         info.fMask = MIIM_TYPE;
         info.fType = MFT_STRING;
-        info.cch = text.length();
+        info.cch = m_text.length();
         info.dwTypeData = (LPTSTR) data ;
         if ( !::SetMenuItemInfo(hMenu, id, FALSE, & info) )
         {

src/os2/menuitem.cpp

@@ -373,17 +373,16 @@ void wxMenuItem::SetText( const wxString& rText )
     if (m_text == sText)
         return;
 
-    if (sText.IsEmpty())
+    // wxMenuItemBase will do stock ID checks
-    {
-        wxASSERT_MSG(wxIsStockID(GetId()), wxT("A non-stock menu item with an empty label?"));
-        sText = wxGetStockLabel(GetId(), wxSTOCK_WITH_ACCELERATOR|wxSTOCK_WITH_MNEMONIC);
-    }
-
     wxMenuItemBase::SetText(sText);
-    OWNER_DRAWN_ONLY(wxOwnerDrawn::SetName(sText));
+
+    // m_text could now be different from 'text' if we are a stock menu item,
+    // so use only m_text below
+
+    OWNER_DRAWN_ONLY(wxOwnerDrawn::SetName(m_text));
 #if  wxUSE_OWNER_DRAWN
     if (rText.IsEmpty())
-        SetAccelString(sText.AfterFirst(_T('\t')));
+        SetAccelString(m_text.AfterFirst(_T('\t')));
     else
         SetAccelString(rText.AfterFirst(_T('\t')));
 #endif // wxUSE_OWNER_DRAWN
@@ -428,7 +427,7 @@ void wxMenuItem::SetText( const wxString& rText )
 #endif  //owner drawn
         {
             uFlagsOld |= MIS_TEXT;
-            pData = (BYTE*)sText.c_str();
+            pData = (BYTE*)m_text.c_str();
         }
 
         //

src/univ/menu.cpp

@@ -1538,18 +1538,12 @@ void wxMenuItem::UpdateAccelInfo()
     m_strAccel = m_text.AfterFirst(_T('\t'));
 }
 
-void wxMenuItem::SetText(const wxString& txt)
+void wxMenuItem::SetText(const wxString& text)
 {
-    if ( txt != m_text )
+    if ( text != m_text )
     {
-        wxString text = txt;
-        if (text.IsEmpty())
-        {
-            wxASSERT_MSG(wxIsStockID(GetId()), wxT("A non-stock menu item with an empty label?"));
-            text = wxGetStockLabel(GetId(), wxSTOCK_WITH_ACCELERATOR|wxSTOCK_WITH_MNEMONIC);
-        }
-
         // first call the base class version to change m_text
+        // (and also check if we don't have a stock menu item)
         wxMenuItemBase::SetText(text);
 
         UpdateAccelInfo();