wxMenu code clean up

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

docs/latex/wx/menu.tex

@@ -201,13 +201,14 @@ before matching.
 
 \membersection{wxMenu::FindItemForId}\label{wxmenufinditemforid}
 
-\constfunc{wxMenuItem*}{FindItemForId}{\param{int}{ id}}
+\constfunc{wxMenuItem*}{FindItemForId}{\param{int}{ id}, \param{wxMenu **}{ menuForItem = NULL}}
 
 Finds the menu item object associated with the given menu item identifier.
 
 \wxheading{Parameters}
 
 \docparam{id}{Menu item identifier.}
+\docparam{menuForItem}{will be filled with the menu for this item if not NULL.}
 
 \wxheading{Return value}
 

include/wx/gtk/menu.h

@@ -44,20 +44,28 @@ class wxMenuBar: public wxWindow
     DECLARE_DYNAMIC_CLASS(wxMenuBar)
 
 public:
-  wxMenuBar( long style );
+    // ctors
     wxMenuBar();
+    wxMenuBar(long style);
+    wxMenuBar(int n, wxMenu *menus[], const wxString titles[]);
+
+    // menubar construction
     void Append( wxMenu *menu, const wxString &title );
 
-  int FindMenuItem( const wxString &menuString, const wxString &itemString ) const;
+    // item search
-  wxMenuItem* FindMenuItemById( int id ) const;
+        // by menu and item names, returns wxNOT_FOUND if not found
-  inline wxMenuItem* FindItemForId( int id ) const { return FindMenuItemById( id ); }
+    virtual int FindMenuItem(const wxString& menuString,
+                             const wxString& itemString) const;
+        // returns NULL if not found
+    wxMenuItem* FindItem( int id ) const;
+        // returns NULL if not found, fills menuForItem if !NULL
+    wxMenuItem *FindItemForId(int itemId, wxMenu **menuForItem = NULL) const;
 
+    // state control
     void Check( int id, bool check );
-  bool Checked( int id ) const;
+    bool IsChecked( int id ) const;
     void Enable( int id, bool enable );
-  bool Enabled( int id ) const;
+    bool IsEnabled( int id ) const;
-  inline bool IsEnabled( int id ) const { return Enabled(id); }
-  inline bool IsChecked( int id ) const { return Checked(id); }
 
     wxString GetLabel( int id ) const;
     void SetLabel( int id, const wxString &label );
@@ -69,9 +77,21 @@ public:
     virtual void SetHelpString( int id, const wxString& helpString );
     virtual wxString GetHelpString( int id ) const;
 
-  inline int GetMenuCount() const { return m_menus.Number(); }
+    int GetMenuCount() const { return m_menus.Number(); }
-  inline wxMenu *GetMenu( int n ) const { return (wxMenu *)m_menus.Nth(n)->Data(); }
+    wxMenu *GetMenu( int n ) const { return (wxMenu *)m_menus.Nth(n)->Data(); }
 
+#ifdef WXWIN_COMPATIBILITY
+    // compatibility: these functions are deprecated
+    bool Enabled(int id) const { return IsEnabled(id); }
+    bool Checked(int id) const { return IsChecked(id); }
+
+    wxMenuItem* FindMenuItemById( int id ) const { return FindItem(id); }
+#endif // WXWIN_COMPATIBILITY
+
+    // implementation
+    wxList& GetMenus() { return m_menus; }
+
+protected:
     wxList       m_menus;
     GtkWidget   *m_menubar;
 };
@@ -86,7 +106,8 @@ DECLARE_DYNAMIC_CLASS(wxMenu)
 
 public:
     // construction
-  wxMenu( const wxString& title = wxEmptyString, const wxFunction func = (wxFunction) NULL );
+    wxMenu( const wxString& title = wxEmptyString,
+            const wxFunction func = (wxFunction) NULL );
 
     // operations
         // title
@@ -99,12 +120,11 @@ public:
     void Append(int id, const wxString &item,
                 wxMenu *subMenu, const wxString &helpStr = "" );
     void Append(wxMenuItem *pItem);
-  void Break() {};
+    void Break() { }
 
     // find item by name/id
     int FindItem( const wxString itemString ) const;
     wxMenuItem *FindItem( int id ) const;
-  wxMenuItem *FindItemForId( int id ) const { return FindItem( id ); }
 
     // get/set item's state
     void Enable( int id, bool enable );
@@ -122,13 +142,11 @@ public:
         // accessors
     wxList& GetItems() { return m_items; }
 
-  inline void Callback(const wxFunction func) { m_callback = func; }
+    void SetEventHandler(wxEvtHandler *handler) { m_eventHandler = handler; }
+    wxEvtHandler *GetEventHandler() { return m_eventHandler; }
 
-  inline void SetEventHandler(wxEvtHandler *handler) { m_eventHandler = handler; }
+    void SetClientData( void* clientData ) { m_clientData = clientData; }
-  inline wxEvtHandler *GetEventHandler() { return m_eventHandler; }
+    void* GetClientData() const { return m_clientData; }
-  
-  inline void SetClientData( void* clientData ) { m_clientData = clientData; }
-  inline void* GetClientData() const { return m_clientData; }
 
     // Updates the UI for a menu and all submenus recursively.
     // source is the object that has the update event handlers
@@ -136,21 +154,33 @@ public:
     // will be used.
     void UpdateUI(wxEvtHandler* source = (wxEvtHandler*) NULL);
 
-// implementation
+    wxMenuItem *FindItemForId( int id ) const { return FindItem( id ); }
 
+#ifdef WXWIN_COMPATIBILITY
+    wxFunction GetCallback() const { return m_callback; }
+    void Callback(const wxFunction func) { m_callback = func; }
+
+    // compatibility: these functions are deprecated
+    bool Enabled(int id) const { return IsEnabled(id); }
+    bool Checked(int id) const { return IsChecked(id); }
+#endif // WXWIN_COMPATIBILITY
+
+    // implementation
     int FindMenuIdByMenuItem( GtkWidget *menuItem ) const;
     void SetInvokingWindow( wxWindow *win );
     wxWindow *GetInvokingWindow();
 
+    // implementation only
+    GtkWidget     *m_menu;  // GtkMenu
+    GtkWidget     *m_owner;
+
+private:
     wxString       m_title;
     wxList         m_items;
     wxWindow      *m_invokingWindow;
     wxFunction     m_callback;
     wxEvtHandler  *m_eventHandler;
     void          *m_clientData;
-
-  GtkWidget     *m_menu;  // GtkMenu
-  GtkWidget     *m_owner;
 };
 
 #endif // __GTKMENUH__

include/wx/gtk/menuitem.h

@@ -28,7 +28,6 @@
 //-----------------------------------------------------------------------------
 
 class wxMenuItem;
-
 class wxMenu;
 
 //-----------------------------------------------------------------------------
@@ -72,10 +71,13 @@ public:
     const wxString& GetHelp() const { return m_helpStr; }
 
     // implementation
-  
     void SetMenuItem(GtkWidget *menuItem) { m_menuItem = menuItem; }
     GtkWidget *GetMenuItem() const { return m_menuItem; }
 
+    void SetCheckedFlag(bool checked) { m_isChecked = checked; }
+    bool GetCheckedFlag() const { return m_isChecked; }
+
+private:
     int           m_id;
     wxString      m_text;
     bool          m_isCheckMenu;

include/wx/gtk1/menu.h

@@ -44,20 +44,28 @@ class wxMenuBar: public wxWindow
     DECLARE_DYNAMIC_CLASS(wxMenuBar)
 
 public:
-  wxMenuBar( long style );
+    // ctors
     wxMenuBar();
+    wxMenuBar(long style);
+    wxMenuBar(int n, wxMenu *menus[], const wxString titles[]);
+
+    // menubar construction
     void Append( wxMenu *menu, const wxString &title );
 
-  int FindMenuItem( const wxString &menuString, const wxString &itemString ) const;
+    // item search
-  wxMenuItem* FindMenuItemById( int id ) const;
+        // by menu and item names, returns wxNOT_FOUND if not found
-  inline wxMenuItem* FindItemForId( int id ) const { return FindMenuItemById( id ); }
+    virtual int FindMenuItem(const wxString& menuString,
+                             const wxString& itemString) const;
+        // returns NULL if not found
+    wxMenuItem* FindItem( int id ) const;
+        // returns NULL if not found, fills menuForItem if !NULL
+    wxMenuItem *FindItemForId(int itemId, wxMenu **menuForItem = NULL) const;
 
+    // state control
     void Check( int id, bool check );
-  bool Checked( int id ) const;
+    bool IsChecked( int id ) const;
     void Enable( int id, bool enable );
-  bool Enabled( int id ) const;
+    bool IsEnabled( int id ) const;
-  inline bool IsEnabled( int id ) const { return Enabled(id); }
-  inline bool IsChecked( int id ) const { return Checked(id); }
 
     wxString GetLabel( int id ) const;
     void SetLabel( int id, const wxString &label );
@@ -69,9 +77,21 @@ public:
     virtual void SetHelpString( int id, const wxString& helpString );
     virtual wxString GetHelpString( int id ) const;
 
-  inline int GetMenuCount() const { return m_menus.Number(); }
+    int GetMenuCount() const { return m_menus.Number(); }
-  inline wxMenu *GetMenu( int n ) const { return (wxMenu *)m_menus.Nth(n)->Data(); }
+    wxMenu *GetMenu( int n ) const { return (wxMenu *)m_menus.Nth(n)->Data(); }
 
+#ifdef WXWIN_COMPATIBILITY
+    // compatibility: these functions are deprecated
+    bool Enabled(int id) const { return IsEnabled(id); }
+    bool Checked(int id) const { return IsChecked(id); }
+
+    wxMenuItem* FindMenuItemById( int id ) const { return FindItem(id); }
+#endif // WXWIN_COMPATIBILITY
+
+    // implementation
+    wxList& GetMenus() { return m_menus; }
+
+protected:
     wxList       m_menus;
     GtkWidget   *m_menubar;
 };
@@ -86,7 +106,8 @@ DECLARE_DYNAMIC_CLASS(wxMenu)
 
 public:
     // construction
-  wxMenu( const wxString& title = wxEmptyString, const wxFunction func = (wxFunction) NULL );
+    wxMenu( const wxString& title = wxEmptyString,
+            const wxFunction func = (wxFunction) NULL );
 
     // operations
         // title
@@ -99,12 +120,11 @@ public:
     void Append(int id, const wxString &item,
                 wxMenu *subMenu, const wxString &helpStr = "" );
     void Append(wxMenuItem *pItem);
-  void Break() {};
+    void Break() { }
 
     // find item by name/id
     int FindItem( const wxString itemString ) const;
     wxMenuItem *FindItem( int id ) const;
-  wxMenuItem *FindItemForId( int id ) const { return FindItem( id ); }
 
     // get/set item's state
     void Enable( int id, bool enable );
@@ -122,13 +142,11 @@ public:
         // accessors
     wxList& GetItems() { return m_items; }
 
-  inline void Callback(const wxFunction func) { m_callback = func; }
+    void SetEventHandler(wxEvtHandler *handler) { m_eventHandler = handler; }
+    wxEvtHandler *GetEventHandler() { return m_eventHandler; }
 
-  inline void SetEventHandler(wxEvtHandler *handler) { m_eventHandler = handler; }
+    void SetClientData( void* clientData ) { m_clientData = clientData; }
-  inline wxEvtHandler *GetEventHandler() { return m_eventHandler; }
+    void* GetClientData() const { return m_clientData; }
-  
-  inline void SetClientData( void* clientData ) { m_clientData = clientData; }
-  inline void* GetClientData() const { return m_clientData; }
 
     // Updates the UI for a menu and all submenus recursively.
     // source is the object that has the update event handlers
@@ -136,21 +154,33 @@ public:
     // will be used.
     void UpdateUI(wxEvtHandler* source = (wxEvtHandler*) NULL);
 
-// implementation
+    wxMenuItem *FindItemForId( int id ) const { return FindItem( id ); }
 
+#ifdef WXWIN_COMPATIBILITY
+    wxFunction GetCallback() const { return m_callback; }
+    void Callback(const wxFunction func) { m_callback = func; }
+
+    // compatibility: these functions are deprecated
+    bool Enabled(int id) const { return IsEnabled(id); }
+    bool Checked(int id) const { return IsChecked(id); }
+#endif // WXWIN_COMPATIBILITY
+
+    // implementation
     int FindMenuIdByMenuItem( GtkWidget *menuItem ) const;
     void SetInvokingWindow( wxWindow *win );
     wxWindow *GetInvokingWindow();
 
+    // implementation only
+    GtkWidget     *m_menu;  // GtkMenu
+    GtkWidget     *m_owner;
+
+private:
     wxString       m_title;
     wxList         m_items;
     wxWindow      *m_invokingWindow;
     wxFunction     m_callback;
     wxEvtHandler  *m_eventHandler;
     void          *m_clientData;
-
-  GtkWidget     *m_menu;  // GtkMenu
-  GtkWidget     *m_owner;
 };
 
 #endif // __GTKMENUH__

include/wx/gtk1/menuitem.h

@@ -28,7 +28,6 @@
 //-----------------------------------------------------------------------------
 
 class wxMenuItem;
-
 class wxMenu;
 
 //-----------------------------------------------------------------------------
@@ -72,10 +71,13 @@ public:
     const wxString& GetHelp() const { return m_helpStr; }
 
     // implementation
-  
     void SetMenuItem(GtkWidget *menuItem) { m_menuItem = menuItem; }
     GtkWidget *GetMenuItem() const { return m_menuItem; }
 
+    void SetCheckedFlag(bool checked) { m_isChecked = checked; }
+    bool GetCheckedFlag() const { return m_isChecked; }
+
+private:
     int           m_id;
     wxString      m_text;
     bool          m_isCheckMenu;

include/wx/msw/menu.h

@@ -22,12 +22,14 @@
 class WXDLLEXPORT wxMenuItem;
 class WXDLLEXPORT wxMenuBar;
 class WXDLLEXPORT wxMenu;
+class WXDLLEXPORT wxFrame;
 
 WXDLLEXPORT_DATA(extern const char*) wxEmptyString;
 
 // ----------------------------------------------------------------------------
 // Menu
 // ----------------------------------------------------------------------------
+
 class WXDLLEXPORT wxMenu : public wxEvtHandler
 {
     DECLARE_DYNAMIC_CLASS(wxMenu)
@@ -35,68 +37,85 @@ class WXDLLEXPORT wxMenu: public wxEvtHandler
 public:
     // ctor & dtor
     wxMenu(const wxString& title = wxEmptyString, const wxFunction func = NULL);
-  ~wxMenu();
+    virtual ~wxMenu();
 
     // construct menu
-    // append items to the menu
+        // append a separator to the menu
-      // separator line
     void AppendSeparator();
-      // normal item
+        // append a normal item to the menu
-  void Append(int id, const wxString& Label, const wxString& helpString = wxEmptyString,
+    void Append(int id, const wxString& label,
+                const wxString& helpString = wxEmptyString,
                 bool checkable = FALSE);
-      // a submenu
+        // append a submenu
-  void Append(int id, const wxString& Label, wxMenu *SubMenu,
+    void Append(int id, const wxString& label,
+                wxMenu *submenu,
                 const wxString& helpString = wxEmptyString);
-      // the most generic form (create wxMenuItem first and use it's functions)
+        // append anything (create wxMenuItem first)
     void Append(wxMenuItem *pItem);
+
         // insert a break in the menu
     void Break();
-    // delete an item
-  void Delete(int id); /* If it's a submenu, menu is not destroyed. VZ: why? */
 
-  // Client data
+        // delete an item
-  inline void SetClientData(void* clientData) { m_clientData = clientData; }
+        // If it's a submenu, menu is not destroyed.
-  inline void* GetClientData() const { return m_clientData; }
+        // VZ: why? shouldn't it return "wxMenu *" then?
+    void Delete(int id);
+
+    // client data
+    void SetClientData(void* clientData) { m_clientData = clientData; }
+    void* GetClientData() const { return m_clientData; }
 
     // menu item control
-  void Enable(int id, bool Flag);
+        // enable/disable item
-  bool Enabled(int id) const;
+    void Enable(int id, bool enable);
-  inline bool IsEnabled(int id) const { return Enabled(id); };
+        // TRUE if enabled
-  void Check(int id, bool Flag);
+    bool IsEnabled(int id) const;
-  bool Checked(int id) const;
-  inline bool IsChecked(int id) const { return Checked(id); };
 
-  // item properties
+        // check/uncheck item - only for checkable items, of course
-    // title
+    void Check(int id, bool check);
+        // TRUE if checked
+    bool IsChecked(int id) const;
+
+    // other properties
+        // the menu title
     void SetTitle(const wxString& label);
     const wxString GetTitle() const;
-    // label
+        // the item label
     void SetLabel(int id, const wxString& label);
     wxString GetLabel(int id) const;
         // help string
     virtual void SetHelpString(int id, const wxString& helpString);
     virtual wxString GetHelpString(int id) const;
 
+        // get the list of items
+    wxList& GetItems() const { return (wxList &)m_menuItems; }
+
     // find item
-    // Finds the item id matching the given string, wxNOT_FOUND if not found.
+        // returns id of the item matching the given string or wxNOT_FOUND
     virtual int FindItem(const wxString& itemString) const;
-    // Find wxMenuItem by ID, and item's menu too if itemMenu is !NULL.
+        // returns NULL if not found
+    wxMenuItem* FindItem(int id) const { return FindItemForId(id); }
+        // find wxMenuItem by ID, and item's menu too if itemMenu is !NULL
     wxMenuItem *FindItemForId(int itemId, wxMenu **itemMenu = NULL) const;
 
-  // Updates the UI for a menu and all submenus recursively.
+    // Updates the UI for a menu and all submenus recursively. source is the
-  // source is the object that has the update event handlers
+    // object that has the update event handlers defined for it. If NULL, the
-  // defined for it. If NULL, the menu or associated window
+    // menu or associated window will be used.
-  // will be used.
     void UpdateUI(wxEvtHandler* source = (wxEvtHandler*)NULL);
 
     void ProcessCommand(wxCommandEvent& event);
-  inline void Callback(const wxFunction func) { m_callback = func; }
 
     virtual void SetParent(wxEvtHandler *parent) { m_parent = parent; }
-  inline void SetEventHandler(wxEvtHandler *handler) { m_eventHandler = handler; }
+    void SetEventHandler(wxEvtHandler *handler) { m_eventHandler = handler; }
-  inline wxEvtHandler *GetEventHandler() { return m_eventHandler; }
+    wxEvtHandler *GetEventHandler() const { return m_eventHandler; }
 
-  inline wxList& GetItems() const { return (wxList&) m_menuItems; }
+#ifdef WXWIN_COMPATIBILITY
+    void Callback(const wxFunction func) { m_callback = func; }
+
+    // compatibility: these functions are deprecated
+    bool Enabled(int id) const { return IsEnabled(id); }
+    bool Checked(int id) const { return IsChecked(id); }
+#endif // WXWIN_COMPATIBILITY
 
     // IMPLEMENTATION
     bool MSWCommand(WXUINT param, WXWORD id);
@@ -136,49 +155,71 @@ public:
 // ----------------------------------------------------------------------------
 // Menu Bar (a la Windows)
 // ----------------------------------------------------------------------------
-class WXDLLEXPORT wxFrame;
+
 class WXDLLEXPORT wxMenuBar : public wxEvtHandler
 {
     DECLARE_DYNAMIC_CLASS(wxMenuBar)
 
 public:
+    // ctors & dtor
     wxMenuBar();
     wxMenuBar(long style);
     wxMenuBar(int n, wxMenu *menus[], const wxString titles[]);
-  ~wxMenuBar();
+    virtual ~wxMenuBar();
 
+    // menubar construction
     void Append(wxMenu *menu, const wxString& title);
-  // Must only be used AFTER menu has been attached to frame,
+    virtual void Delete(wxMenu *menu, int index = 0); /* Menu not destroyed */
+
+    // state control
+    // NB: must only be used AFTER menu has been attached to frame,
     //     otherwise use individual menus to enable/disable items
-  void Enable(int Id, bool Flag);
+        // enable the item
-  bool Enabled(int Id) const ;
+    void Enable(int id, bool enable);
-  inline bool IsEnabled(int Id) const { return Enabled(Id); };
+        // TRUE if item enabled
-  void EnableTop(int pos, bool Flag);
+    bool IsEnabled(int id) const;
-  void Check(int id, bool Flag);
+        //
-  bool Checked(int id) const ;
+    void EnableTop(int pos, bool enable);
-  inline bool IsChecked(int Id) const { return Checked(Id); };
+
+        // works only with checkable items
+    void Check(int id, bool check);
+        // TRUE if checked
+    bool IsChecked(int id) const;
+
     void SetLabel(int id, const wxString& label) ;
     wxString GetLabel(int id) const ;
+
+    virtual void SetHelpString(int id, const wxString& helpString);
+    virtual wxString GetHelpString(int id) const ;
+
     void SetLabelTop(int pos, const wxString& label) ;
     wxString GetLabelTop(int pos) const ;
-  virtual void Delete(wxMenu *menu, int index = 0); /* Menu not destroyed */
+
+    // notifications
     virtual bool OnAppend(wxMenu *menu, const char *title);
     virtual bool OnDelete(wxMenu *menu, int index);
 
-  virtual void SetHelpString(int Id, const wxString& helpString);
+    // item search
-  virtual wxString GetHelpString(int Id) const ;
+        // by menu and item names, returns wxNOT_FOUND if not found
-
+    virtual int FindMenuItem(const wxString& menuString,
-  virtual int FindMenuItem(const wxString& menuString, const wxString& itemString) const ;
+                             const wxString& itemString) const;
-
+        // returns NULL if not found
-  // Find wxMenuItem for item ID, and return item's
+    wxMenuItem* FindItem(int id) const { return FindItemForId(id); }
-  // menu too if itemMenu is non-NULL.
+        // returns NULL if not found, fills menuForItem if !NULL
     wxMenuItem *FindItemForId(int itemId, wxMenu **menuForItem = NULL) const;
 
-  inline void SetEventHandler(wxEvtHandler *handler) { m_eventHandler = handler; }
+    // submenus access
-  inline wxEvtHandler *GetEventHandler() { return m_eventHandler; }
+    int GetMenuCount() const { return m_menuCount; }
+    wxMenu *GetMenu(int i) const { return m_menus[i]; }
 
-  inline int GetMenuCount() const { return m_menuCount; }
+    void SetEventHandler(wxEvtHandler *handler) { m_eventHandler = handler; }
-  inline wxMenu* GetMenu(int i) const { return m_menus[i]; }
+    wxEvtHandler *GetEventHandler() { return m_eventHandler; }
+
+#ifdef WXWIN_COMPATIBILITY
+    // compatibility: these functions are deprecated
+    bool Enabled(int id) const { return IsEnabled(id); }
+    bool Checked(int id) const { return IsChecked(id); }
+#endif // WXWIN_COMPATIBILITY
 
 public:
     wxEvtHandler *            m_eventHandler;

src/gtk/frame.cpp

@@ -514,7 +514,7 @@ void wxFrame::OnSize( wxSizeEvent &WXUNUSED(event) )
 static void SetInvokingWindow( wxMenu *menu, wxWindow *win )
 {
     menu->SetInvokingWindow( win );
-    wxNode *node = menu->m_items.First();
+    wxNode *node = menu->GetItems().First();
     while (node)
     {
         wxMenuItem *menuitem = (wxMenuItem*)node->Data();
@@ -533,7 +533,7 @@ void wxFrame::SetMenuBar( wxMenuBar *menuBar )
 
     if (m_frameMenuBar)
     {
-        wxNode *node = m_frameMenuBar->m_menus.First();
+        wxNode *node = m_frameMenuBar->GetMenus().First();
         while (node)
         {
             wxMenu *menu = (wxMenu*)node->Data();
@@ -548,7 +548,8 @@ void wxFrame::SetMenuBar( wxMenuBar *menuBar )
                 m_frameMenuBar->m_widget, m_frameMenuBar->m_x, m_frameMenuBar->m_y );
 
             /* an mdi child menu bar might be underneath */
-            if (m_mdiMenuBar) m_frameMenuBar->Show( FALSE );
+            if (m_mdiMenuBar)
+                m_frameMenuBar->Show( FALSE );
         }
     }
 

src/gtk/mdi.cpp

@@ -260,7 +260,7 @@ void wxMDIChildFrame::AddChild( wxWindow *child )
 static void SetInvokingWindow( wxMenu *menu, wxWindow *win )
 {
     menu->SetInvokingWindow( win );
-    wxNode *node = menu->m_items.First();
+    wxNode *node = menu->GetItems().First();
     while (node)
     {
         wxMenuItem *menuitem = (wxMenuItem*)node->Data();
@@ -280,7 +280,7 @@ void wxMDIChildFrame::SetMenuBar( wxMenuBar *menu_bar )
 
        if (m_menuBar->m_parent != this)
        {
-            wxNode *node = m_menuBar->m_menus.First();
+            wxNode *node = m_menuBar->GetMenus().First();
             while (node)
             {
                 wxMenu *menu = (wxMenu*)node->Data();

src/gtk/menu.cpp

@@ -72,16 +72,19 @@ wxMenuBar::wxMenuBar()
 void wxMenuBar::Append( wxMenu *menu, const wxString &title )
 {
     m_menus.Append( menu );
-    menu->m_title = title; 
+    wxString title2 = title;
 
     int pos;
     do
     {
-        pos = menu->m_title.First( '&' );
+        pos = title2.First( '&' );
-        if (pos != -1) menu->m_title.Remove( pos, 1 );
+        if (pos != wxNOT_FOUND)
-    } while (pos != -1);
+            title2.Remove( pos, 1 );
+    } while (pos != wxNOT_FOUND);
 
-    menu->m_owner = gtk_menu_item_new_with_label( WXSTRINGCAST(menu->m_title) );
+    menu->SetTitle(title2);
+
+    menu->m_owner = gtk_menu_item_new_with_label( WXSTRINGCAST(title2) );
     gtk_widget_show( menu->m_owner );
     gtk_menu_item_set_submenu( GTK_MENU_ITEM(menu->m_owner), menu->m_menu );
 
@@ -90,13 +93,14 @@ void wxMenuBar::Append( wxMenu *menu, const wxString &title )
 
 static int FindMenuItemRecursive( const wxMenu *menu, const wxString &menuString, const wxString &itemString )
 {
-    if (menu->m_title == menuString)
+    if (menu->GetTitle() == menuString)
     {
         int res = menu->FindItem( itemString );
-        if (res != -1) return res;
+        if (res != wxNOT_FOUND)
+            return res;
     }
 
-    wxNode *node = menu->m_items.First();
+    wxNode *node = ((wxMenu *)menu)->GetItems().First();    // const_cast
     while (node)
     {
         wxMenuItem *item = (wxMenuItem*)node->Data();
@@ -106,7 +110,19 @@ static int FindMenuItemRecursive( const wxMenu *menu, const wxString &menuString
         node = node->Next();
     }
 
-    return -1;
+    return wxNOT_FOUND;
+}
+
+wxMenuItem *wxMenuBar::FindItemForId(int itemId, wxMenu **menuForItem = NULL) const
+{
+    if ( menuForItem )
+    {
+        // TODO return the pointer to the menu
+
+        *menuForItem = NULL;
+    }
+
+    return FindItem(itemId);
 }
 
 int wxMenuBar::FindMenuItem( const wxString &menuString, const wxString &itemString ) const
@@ -122,12 +138,12 @@ int wxMenuBar::FindMenuItem( const wxString &menuString, const wxString &itemStr
     return -1;
 }
 
-/* Find a wxMenuItem using its id. Recurses down into sub-menus */
+// Find a wxMenuItem using its id. Recurses down into sub-menus
 static wxMenuItem* FindMenuItemByIdRecursive(const wxMenu* menu, int id)
 {
     wxMenuItem* result = menu->FindItem(id);
 
-    wxNode *node = menu->m_items.First();
+    wxNode *node = ((wxMenu *)menu)->GetItems().First(); // const_cast
     while ( node && result == NULL )
     {
         wxMenuItem *item = (wxMenuItem*)node->Data();
@@ -141,7 +157,7 @@ static wxMenuItem* FindMenuItemByIdRecursive(const wxMenu* menu, int id)
     return result;
 }
 
-wxMenuItem* wxMenuBar::FindMenuItemById( int id ) const
+wxMenuItem* wxMenuBar::FindItem( int id ) const
 {
     wxMenuItem* result = 0;
     wxNode *node = m_menus.First();
@@ -158,44 +174,55 @@ wxMenuItem* wxMenuBar::FindMenuItemById( int id ) const
 void wxMenuBar::Check( int id, bool check )
 {
     wxMenuItem* item = FindMenuItemById( id );
-    if (item) item->Check(check);
+
+    wxCHECK_RET( item, "wxMenuBar::Check: no such item" );
+
+    item->Check(check);
 }
 
-bool wxMenuBar::Checked( int id ) const
+bool wxMenuBar::IsChecked( int id ) const
 {
     wxMenuItem* item = FindMenuItemById( id );
-    if (item) return item->IsChecked();
+
-    return FALSE;
+    wxCHECK_MSG( item, FALSE, "wxMenuBar::IsChecked: no such item" );
+
+    return item->IsChecked();
 }
 
 void wxMenuBar::Enable( int id, bool enable )
 {
     wxMenuItem* item = FindMenuItemById( id );
-    if (item) item->Enable(enable);
+
+    wxCHECK_RET( item, "wxMenuBar::Enable: no such item" );
+
+    item->Enable(enable);
 }
 
-bool wxMenuBar::Enabled( int id ) const
+bool wxMenuBar::IsEnabled( int id ) const
 {
     wxMenuItem* item = FindMenuItemById( id );
-    if (item) return item->IsEnabled();
 
-    return FALSE;
+    wxCHECK_MSG( item, FALSE, "wxMenuBar::IsEnabled: no such item" );
+
+    return item->IsEnabled();
 }
 
 wxString wxMenuBar::GetLabel( int id ) const
 {
     wxMenuItem* item = FindMenuItemById( id );
 
-    if (item) return item->GetText();
+    wxCHECK_MSG( item, "", "wxMenuBar::GetLabel: no such item" );
 
-    return wxString("");
+    return item->GetText();
 }
 
 void wxMenuBar::SetLabel( int id, const wxString &label )
 {
     wxMenuItem* item = FindMenuItemById( id );
 
-    if (item) item->SetText( label );
+    wxCHECK_RET( item, "wxMenuBar::SetLabel: no such item" );
+
+    item->SetText( label );
 }
 
 void wxMenuBar::EnableTop( int pos, bool flag )
@@ -206,7 +233,8 @@ void wxMenuBar::EnableTop( int pos, bool flag )
 
     wxMenu* menu = (wxMenu*)node->Data();
 
-    if (menu->m_owner) gtk_widget_set_sensitive( menu->m_owner, flag );
+    if (menu->m_owner)
+        gtk_widget_set_sensitive( menu->m_owner, flag );
 }
 
 wxString wxMenuBar::GetLabelTop( int pos ) const
@@ -235,17 +263,18 @@ void wxMenuBar::SetHelpString( int id, const wxString& helpString )
 {
     wxMenuItem* item = FindMenuItemById( id );
 
-    if (item) item->SetHelp( helpString );
+    wxCHECK_RET( item, "wxMenuBar::SetHelpString: no such item" );
+
+    item->SetHelp( helpString );
 }
 
 wxString wxMenuBar::GetHelpString( int id ) const
 {
     wxMenuItem* item = FindMenuItemById( id );
 
-    if (item) 
+    wxCHECK_MSG( item, "", "wxMenuBar::GetHelpString: no such item" );
+
     return item->GetHelp();
-    else
-        return wxString("");
 }
 
 //-----------------------------------------------------------------------------
@@ -259,14 +288,15 @@ static void gtk_menu_clicked_callback( GtkWidget *widget, wxMenu *menu )
     /* should find it for normal (not popup) menu */
     wxASSERT( (id != -1) || (menu->GetInvokingWindow() != NULL) );
 
-    if (!menu->IsEnabled(id)) return;
+    if (!menu->IsEnabled(id))
+        return;
 
     wxMenuItem* item = menu->FindItem( id );
     wxCHECK_RET( item, "error in menu item callback" );
 
-    if (item->m_isCheckMenu)
+    if (item->IsCheckable())
     {
-        if (item->m_isChecked == item->IsChecked())
+        if (item->GetCheckedFlag() == item->IsChecked())
         {
             /* the menu item has been checked by calling wxMenuItem->Check() */
             return;
@@ -274,7 +304,7 @@ static void gtk_menu_clicked_callback( GtkWidget *widget, wxMenu *menu )
         else
         {
             /* the user pressed on the menu item -> report */
-	    item->m_isChecked = item->IsChecked();  /* make consistent again */
+            item->SetCheckedFlag(item->IsChecked());  /* make consistent again */
         }
     }
 
@@ -282,16 +312,18 @@ static void gtk_menu_clicked_callback( GtkWidget *widget, wxMenu *menu )
     event.SetEventObject( menu );
     event.SetInt(id );
 
-    if (menu->m_callback)
+    if (menu->GetCallback())
     {
-        (void) (*(menu->m_callback)) (*menu, event);
+        (void) (*(menu->GetCallback())) (*menu, event);
         return;
     }
 
-    if (menu->GetEventHandler()->ProcessEvent(event)) return;
+    if (menu->GetEventHandler()->ProcessEvent(event))
+        return;
 
     wxWindow *win = menu->GetInvokingWindow();
-    if (win) win->GetEventHandler()->ProcessEvent( event );
+    if (win)
+        win->GetEventHandler()->ProcessEvent( event );
 }
 
 //-----------------------------------------------------------------------------
@@ -304,7 +336,8 @@ static void gtk_menu_hilight_callback( GtkWidget *widget, wxMenu *menu )
 
     wxASSERT( id != -1 ); // should find it!
 
-    if (!menu->IsEnabled(id)) return;
+    if (!menu->IsEnabled(id))
+        return;
 
     wxMenuEvent event( wxEVT_MENU_HIGHLIGHT, id );
     event.SetEventObject( menu );
@@ -318,7 +351,8 @@ static void gtk_menu_hilight_callback( GtkWidget *widget, wxMenu *menu )
      }
 */
 
-    if (menu->GetEventHandler()->ProcessEvent(event)) return;
+    if (menu->GetEventHandler()->ProcessEvent(event))
+        return;
 
     wxWindow *win = menu->GetInvokingWindow();
     if (win) win->GetEventHandler()->ProcessEvent( event );
@@ -334,15 +368,18 @@ static void gtk_menu_nolight_callback( GtkWidget *widget, wxMenu *menu )
 
     wxASSERT( id != -1 ); // should find it!
 
-    if (!menu->IsEnabled(id)) return;
+    if (!menu->IsEnabled(id))
+        return;
 
     wxMenuEvent event( wxEVT_MENU_HIGHLIGHT, -1 );
     event.SetEventObject( menu );
 
-    if (menu->GetEventHandler()->ProcessEvent(event)) return;
+    if (menu->GetEventHandler()->ProcessEvent(event))
+        return;
 
     wxWindow *win = menu->GetInvokingWindow();
-    if (win) win->GetEventHandler()->ProcessEvent( event );
+    if (win)
+        win->GetEventHandler()->ProcessEvent( event );
 }
 
 //-----------------------------------------------------------------------------
@@ -361,7 +398,7 @@ wxMenuItem::wxMenuItem()
     m_menuItem = (GtkWidget *) NULL;
 }
 
-/* it's valid for this function to be called even if m_menuItem == NULL */
+// it's valid for this function to be called even if m_menuItem == NULL
 void wxMenuItem::SetName( const wxString& str )
 {
     m_text = "";
@@ -438,7 +475,7 @@ wxMenu::wxMenu( const wxString& title, const wxFunction func )
 
 void wxMenu::SetTitle( const wxString& title )
 {
-    /* Waiting for something better. */
+    // TODO Waiting for something better
     m_title = title;
 }
 
@@ -573,93 +610,80 @@ int wxMenu::FindItem( const wxString itemString ) const
         node = node->Next();
     }
 
-    return -1;
+    return wxNOT_FOUND;
 }
 
 void wxMenu::Enable( int id, bool enable )
 {
     wxMenuItem *item = FindItem(id);
-    if (item)
+
-    { 
+    wxCHECK_RET( item, "wxMenu::Enable: no such item" );
+
     item->Enable(enable);
 }
-}
 
 bool wxMenu::IsEnabled( int id ) const
 {
     wxMenuItem *item = FindItem(id);
-    if (item)
+
-    {
+    wxCHECK_MSG( item, FALSE, "wxMenu::IsEnabled: no such item" );
+
     return item->IsEnabled();
 }
-    else
-    {
-        return FALSE;
-    }
-}
 
 void wxMenu::Check( int id, bool enable )
 {
     wxMenuItem *item = FindItem(id);
-    if (item)
+
-    {
+    wxCHECK_RET( item, "wxMenu::Check: no such item" );
+
     item->Check(enable);
 }
-}
 
 bool wxMenu::IsChecked( int id ) const
 {
     wxMenuItem *item = FindItem(id);
-    if (item)
+
-    {
+    wxCHECK_MSG( item, FALSE, "wxMenu::IsChecked: no such item" );
+
     return item->IsChecked();
 }
-    else
-    {
-        return FALSE;
-    }
-}
 
 void wxMenu::SetLabel( int id, const wxString &label )
 {
     wxMenuItem *item = FindItem(id);
-    if (item)
+
-    {
+    wxCHECK_RET( item, "wxMenu::SetLabel: no such item" );
+
     item->SetText(label);
 }
-}
 
 wxString wxMenu::GetLabel( int id ) const
 {
     wxMenuItem *item = FindItem(id);
-    if (item)
+
-    { 
+    wxCHECK_MSG( item, "", "wxMenu::GetLabel: no such item" );
+
     return item->GetText();
 }
-    else
-    {
-        return "";
-    }
-}
 
 void wxMenu::SetHelpString( int id, const wxString& helpString )
 {
     wxMenuItem *item = FindItem(id);
-    if (item) item->SetHelp( helpString );
+
+    wxCHECK_RET( item, "wxMenu::SetHelpString: no such item" );
+
+    item->SetHelp( helpString );
 }
 
 wxString wxMenu::GetHelpString( int id ) const
 {
     wxMenuItem *item = FindItem(id);
-    if (item)
+
-    { 
+    wxCHECK_MSG( item, "", "wxMenu::GetHelpString: no such item" );
+
     return item->GetHelp();
 }
-    else
-    {
-        return "";
-    }
-}
 
 int wxMenu::FindMenuIdByMenuItem( GtkWidget *menuItem ) const
 {
@@ -672,7 +696,7 @@ int wxMenu::FindMenuIdByMenuItem( GtkWidget *menuItem ) const
         node = node->Next();
     }
 
-    return -1;
+    return wxNOT_FOUND;
 }
 
 wxMenuItem *wxMenu::FindItem(int id) const
@@ -705,10 +729,9 @@ wxWindow *wxMenu::GetInvokingWindow()
     return m_invokingWindow;
 }
 
-// Update a menu and all submenus recursively.
+// Update a menu and all submenus recursively. source is the object that has
-// source is the object that has the update event handlers
+// the update event handlers defined for it. If NULL, the menu or associated
-// defined for it. If NULL, the menu or associated window
+// window will be used.
-// will be used.
 void wxMenu::UpdateUI(wxEvtHandler* source)
 {
   if (!source && GetInvokingWindow())
@@ -745,4 +768,3 @@ void wxMenu::UpdateUI(wxEvtHandler* source)
   }
 }
 
-

src/gtk/window.cpp

@@ -2750,7 +2750,7 @@ void wxWindow::InitDialog()
 static void SetInvokingWindow( wxMenu *menu, wxWindow *win )
 {
     menu->SetInvokingWindow( win );
-    wxNode *node = menu->m_items.First();
+    wxNode *node = menu->GetItems().First();
     while (node)
     {
         wxMenuItem *menuitem = (wxMenuItem*)node->Data();

src/gtk1/frame.cpp

@@ -514,7 +514,7 @@ void wxFrame::OnSize( wxSizeEvent &WXUNUSED(event) )
 static void SetInvokingWindow( wxMenu *menu, wxWindow *win )
 {
     menu->SetInvokingWindow( win );
-    wxNode *node = menu->m_items.First();
+    wxNode *node = menu->GetItems().First();
     while (node)
     {
         wxMenuItem *menuitem = (wxMenuItem*)node->Data();
@@ -533,7 +533,7 @@ void wxFrame::SetMenuBar( wxMenuBar *menuBar )
 
     if (m_frameMenuBar)
     {
-        wxNode *node = m_frameMenuBar->m_menus.First();
+        wxNode *node = m_frameMenuBar->GetMenus().First();
         while (node)
         {
             wxMenu *menu = (wxMenu*)node->Data();
@@ -548,7 +548,8 @@ void wxFrame::SetMenuBar( wxMenuBar *menuBar )
                 m_frameMenuBar->m_widget, m_frameMenuBar->m_x, m_frameMenuBar->m_y );
 
             /* an mdi child menu bar might be underneath */
-            if (m_mdiMenuBar) m_frameMenuBar->Show( FALSE );
+            if (m_mdiMenuBar)
+                m_frameMenuBar->Show( FALSE );
         }
     }
 

src/gtk1/mdi.cpp

@@ -260,7 +260,7 @@ void wxMDIChildFrame::AddChild( wxWindow *child )
 static void SetInvokingWindow( wxMenu *menu, wxWindow *win )
 {
     menu->SetInvokingWindow( win );
-    wxNode *node = menu->m_items.First();
+    wxNode *node = menu->GetItems().First();
     while (node)
     {
         wxMenuItem *menuitem = (wxMenuItem*)node->Data();
@@ -280,7 +280,7 @@ void wxMDIChildFrame::SetMenuBar( wxMenuBar *menu_bar )
 
        if (m_menuBar->m_parent != this)
        {
-            wxNode *node = m_menuBar->m_menus.First();
+            wxNode *node = m_menuBar->GetMenus().First();
             while (node)
             {
                 wxMenu *menu = (wxMenu*)node->Data();

src/gtk1/menu.cpp

@@ -72,16 +72,19 @@ wxMenuBar::wxMenuBar()
 void wxMenuBar::Append( wxMenu *menu, const wxString &title )
 {
     m_menus.Append( menu );
-    menu->m_title = title; 
+    wxString title2 = title;
 
     int pos;
     do
     {
-        pos = menu->m_title.First( '&' );
+        pos = title2.First( '&' );
-        if (pos != -1) menu->m_title.Remove( pos, 1 );
+        if (pos != wxNOT_FOUND)
-    } while (pos != -1);
+            title2.Remove( pos, 1 );
+    } while (pos != wxNOT_FOUND);
 
-    menu->m_owner = gtk_menu_item_new_with_label( WXSTRINGCAST(menu->m_title) );
+    menu->SetTitle(title2);
+
+    menu->m_owner = gtk_menu_item_new_with_label( WXSTRINGCAST(title2) );
     gtk_widget_show( menu->m_owner );
     gtk_menu_item_set_submenu( GTK_MENU_ITEM(menu->m_owner), menu->m_menu );
 
@@ -90,13 +93,14 @@ void wxMenuBar::Append( wxMenu *menu, const wxString &title )
 
 static int FindMenuItemRecursive( const wxMenu *menu, const wxString &menuString, const wxString &itemString )
 {
-    if (menu->m_title == menuString)
+    if (menu->GetTitle() == menuString)
     {
         int res = menu->FindItem( itemString );
-        if (res != -1) return res;
+        if (res != wxNOT_FOUND)
+            return res;
     }
 
-    wxNode *node = menu->m_items.First();
+    wxNode *node = ((wxMenu *)menu)->GetItems().First();    // const_cast
     while (node)
     {
         wxMenuItem *item = (wxMenuItem*)node->Data();
@@ -106,7 +110,19 @@ static int FindMenuItemRecursive( const wxMenu *menu, const wxString &menuString
         node = node->Next();
     }
 
-    return -1;
+    return wxNOT_FOUND;
+}
+
+wxMenuItem *wxMenuBar::FindItemForId(int itemId, wxMenu **menuForItem = NULL) const
+{
+    if ( menuForItem )
+    {
+        // TODO return the pointer to the menu
+
+        *menuForItem = NULL;
+    }
+
+    return FindItem(itemId);
 }
 
 int wxMenuBar::FindMenuItem( const wxString &menuString, const wxString &itemString ) const
@@ -122,12 +138,12 @@ int wxMenuBar::FindMenuItem( const wxString &menuString, const wxString &itemStr
     return -1;
 }
 
-/* Find a wxMenuItem using its id. Recurses down into sub-menus */
+// Find a wxMenuItem using its id. Recurses down into sub-menus
 static wxMenuItem* FindMenuItemByIdRecursive(const wxMenu* menu, int id)
 {
     wxMenuItem* result = menu->FindItem(id);
 
-    wxNode *node = menu->m_items.First();
+    wxNode *node = ((wxMenu *)menu)->GetItems().First(); // const_cast
     while ( node && result == NULL )
     {
         wxMenuItem *item = (wxMenuItem*)node->Data();
@@ -141,7 +157,7 @@ static wxMenuItem* FindMenuItemByIdRecursive(const wxMenu* menu, int id)
     return result;
 }
 
-wxMenuItem* wxMenuBar::FindMenuItemById( int id ) const
+wxMenuItem* wxMenuBar::FindItem( int id ) const
 {
     wxMenuItem* result = 0;
     wxNode *node = m_menus.First();
@@ -158,44 +174,55 @@ wxMenuItem* wxMenuBar::FindMenuItemById( int id ) const
 void wxMenuBar::Check( int id, bool check )
 {
     wxMenuItem* item = FindMenuItemById( id );
-    if (item) item->Check(check);
+
+    wxCHECK_RET( item, "wxMenuBar::Check: no such item" );
+
+    item->Check(check);
 }
 
-bool wxMenuBar::Checked( int id ) const
+bool wxMenuBar::IsChecked( int id ) const
 {
     wxMenuItem* item = FindMenuItemById( id );
-    if (item) return item->IsChecked();
+
-    return FALSE;
+    wxCHECK_MSG( item, FALSE, "wxMenuBar::IsChecked: no such item" );
+
+    return item->IsChecked();
 }
 
 void wxMenuBar::Enable( int id, bool enable )
 {
     wxMenuItem* item = FindMenuItemById( id );
-    if (item) item->Enable(enable);
+
+    wxCHECK_RET( item, "wxMenuBar::Enable: no such item" );
+
+    item->Enable(enable);
 }
 
-bool wxMenuBar::Enabled( int id ) const
+bool wxMenuBar::IsEnabled( int id ) const
 {
     wxMenuItem* item = FindMenuItemById( id );
-    if (item) return item->IsEnabled();
 
-    return FALSE;
+    wxCHECK_MSG( item, FALSE, "wxMenuBar::IsEnabled: no such item" );
+
+    return item->IsEnabled();
 }
 
 wxString wxMenuBar::GetLabel( int id ) const
 {
     wxMenuItem* item = FindMenuItemById( id );
 
-    if (item) return item->GetText();
+    wxCHECK_MSG( item, "", "wxMenuBar::GetLabel: no such item" );
 
-    return wxString("");
+    return item->GetText();
 }
 
 void wxMenuBar::SetLabel( int id, const wxString &label )
 {
     wxMenuItem* item = FindMenuItemById( id );
 
-    if (item) item->SetText( label );
+    wxCHECK_RET( item, "wxMenuBar::SetLabel: no such item" );
+
+    item->SetText( label );
 }
 
 void wxMenuBar::EnableTop( int pos, bool flag )
@@ -206,7 +233,8 @@ void wxMenuBar::EnableTop( int pos, bool flag )
 
     wxMenu* menu = (wxMenu*)node->Data();
 
-    if (menu->m_owner) gtk_widget_set_sensitive( menu->m_owner, flag );
+    if (menu->m_owner)
+        gtk_widget_set_sensitive( menu->m_owner, flag );
 }
 
 wxString wxMenuBar::GetLabelTop( int pos ) const
@@ -235,17 +263,18 @@ void wxMenuBar::SetHelpString( int id, const wxString& helpString )
 {
     wxMenuItem* item = FindMenuItemById( id );
 
-    if (item) item->SetHelp( helpString );
+    wxCHECK_RET( item, "wxMenuBar::SetHelpString: no such item" );
+
+    item->SetHelp( helpString );
 }
 
 wxString wxMenuBar::GetHelpString( int id ) const
 {
     wxMenuItem* item = FindMenuItemById( id );
 
-    if (item) 
+    wxCHECK_MSG( item, "", "wxMenuBar::GetHelpString: no such item" );
+
     return item->GetHelp();
-    else
-        return wxString("");
 }
 
 //-----------------------------------------------------------------------------
@@ -259,14 +288,15 @@ static void gtk_menu_clicked_callback( GtkWidget *widget, wxMenu *menu )
     /* should find it for normal (not popup) menu */
     wxASSERT( (id != -1) || (menu->GetInvokingWindow() != NULL) );
 
-    if (!menu->IsEnabled(id)) return;
+    if (!menu->IsEnabled(id))
+        return;
 
     wxMenuItem* item = menu->FindItem( id );
     wxCHECK_RET( item, "error in menu item callback" );
 
-    if (item->m_isCheckMenu)
+    if (item->IsCheckable())
     {
-        if (item->m_isChecked == item->IsChecked())
+        if (item->GetCheckedFlag() == item->IsChecked())
         {
             /* the menu item has been checked by calling wxMenuItem->Check() */
             return;
@@ -274,7 +304,7 @@ static void gtk_menu_clicked_callback( GtkWidget *widget, wxMenu *menu )
         else
         {
             /* the user pressed on the menu item -> report */
-	    item->m_isChecked = item->IsChecked();  /* make consistent again */
+            item->SetCheckedFlag(item->IsChecked());  /* make consistent again */
         }
     }
 
@@ -282,16 +312,18 @@ static void gtk_menu_clicked_callback( GtkWidget *widget, wxMenu *menu )
     event.SetEventObject( menu );
     event.SetInt(id );
 
-    if (menu->m_callback)
+    if (menu->GetCallback())
     {
-        (void) (*(menu->m_callback)) (*menu, event);
+        (void) (*(menu->GetCallback())) (*menu, event);
         return;
     }
 
-    if (menu->GetEventHandler()->ProcessEvent(event)) return;
+    if (menu->GetEventHandler()->ProcessEvent(event))
+        return;
 
     wxWindow *win = menu->GetInvokingWindow();
-    if (win) win->GetEventHandler()->ProcessEvent( event );
+    if (win)
+        win->GetEventHandler()->ProcessEvent( event );
 }
 
 //-----------------------------------------------------------------------------
@@ -304,7 +336,8 @@ static void gtk_menu_hilight_callback( GtkWidget *widget, wxMenu *menu )
 
     wxASSERT( id != -1 ); // should find it!
 
-    if (!menu->IsEnabled(id)) return;
+    if (!menu->IsEnabled(id))
+        return;
 
     wxMenuEvent event( wxEVT_MENU_HIGHLIGHT, id );
     event.SetEventObject( menu );
@@ -318,7 +351,8 @@ static void gtk_menu_hilight_callback( GtkWidget *widget, wxMenu *menu )
      }
 */
 
-    if (menu->GetEventHandler()->ProcessEvent(event)) return;
+    if (menu->GetEventHandler()->ProcessEvent(event))
+        return;
 
     wxWindow *win = menu->GetInvokingWindow();
     if (win) win->GetEventHandler()->ProcessEvent( event );
@@ -334,15 +368,18 @@ static void gtk_menu_nolight_callback( GtkWidget *widget, wxMenu *menu )
 
     wxASSERT( id != -1 ); // should find it!
 
-    if (!menu->IsEnabled(id)) return;
+    if (!menu->IsEnabled(id))
+        return;
 
     wxMenuEvent event( wxEVT_MENU_HIGHLIGHT, -1 );
     event.SetEventObject( menu );
 
-    if (menu->GetEventHandler()->ProcessEvent(event)) return;
+    if (menu->GetEventHandler()->ProcessEvent(event))
+        return;
 
     wxWindow *win = menu->GetInvokingWindow();
-    if (win) win->GetEventHandler()->ProcessEvent( event );
+    if (win)
+        win->GetEventHandler()->ProcessEvent( event );
 }
 
 //-----------------------------------------------------------------------------
@@ -361,7 +398,7 @@ wxMenuItem::wxMenuItem()
     m_menuItem = (GtkWidget *) NULL;
 }
 
-/* it's valid for this function to be called even if m_menuItem == NULL */
+// it's valid for this function to be called even if m_menuItem == NULL
 void wxMenuItem::SetName( const wxString& str )
 {
     m_text = "";
@@ -438,7 +475,7 @@ wxMenu::wxMenu( const wxString& title, const wxFunction func )
 
 void wxMenu::SetTitle( const wxString& title )
 {
-    /* Waiting for something better. */
+    // TODO Waiting for something better
     m_title = title;
 }
 
@@ -573,93 +610,80 @@ int wxMenu::FindItem( const wxString itemString ) const
         node = node->Next();
     }
 
-    return -1;
+    return wxNOT_FOUND;
 }
 
 void wxMenu::Enable( int id, bool enable )
 {
     wxMenuItem *item = FindItem(id);
-    if (item)
+
-    { 
+    wxCHECK_RET( item, "wxMenu::Enable: no such item" );
+
     item->Enable(enable);
 }
-}
 
 bool wxMenu::IsEnabled( int id ) const
 {
     wxMenuItem *item = FindItem(id);
-    if (item)
+
-    {
+    wxCHECK_MSG( item, FALSE, "wxMenu::IsEnabled: no such item" );
+
     return item->IsEnabled();
 }
-    else
-    {
-        return FALSE;
-    }
-}
 
 void wxMenu::Check( int id, bool enable )
 {
     wxMenuItem *item = FindItem(id);
-    if (item)
+
-    {
+    wxCHECK_RET( item, "wxMenu::Check: no such item" );
+
     item->Check(enable);
 }
-}
 
 bool wxMenu::IsChecked( int id ) const
 {
     wxMenuItem *item = FindItem(id);
-    if (item)
+
-    {
+    wxCHECK_MSG( item, FALSE, "wxMenu::IsChecked: no such item" );
+
     return item->IsChecked();
 }
-    else
-    {
-        return FALSE;
-    }
-}
 
 void wxMenu::SetLabel( int id, const wxString &label )
 {
     wxMenuItem *item = FindItem(id);
-    if (item)
+
-    {
+    wxCHECK_RET( item, "wxMenu::SetLabel: no such item" );
+
     item->SetText(label);
 }
-}
 
 wxString wxMenu::GetLabel( int id ) const
 {
     wxMenuItem *item = FindItem(id);
-    if (item)
+
-    { 
+    wxCHECK_MSG( item, "", "wxMenu::GetLabel: no such item" );
+
     return item->GetText();
 }
-    else
-    {
-        return "";
-    }
-}
 
 void wxMenu::SetHelpString( int id, const wxString& helpString )
 {
     wxMenuItem *item = FindItem(id);
-    if (item) item->SetHelp( helpString );
+
+    wxCHECK_RET( item, "wxMenu::SetHelpString: no such item" );
+
+    item->SetHelp( helpString );
 }
 
 wxString wxMenu::GetHelpString( int id ) const
 {
     wxMenuItem *item = FindItem(id);
-    if (item)
+
-    { 
+    wxCHECK_MSG( item, "", "wxMenu::GetHelpString: no such item" );
+
     return item->GetHelp();
 }
-    else
-    {
-        return "";
-    }
-}
 
 int wxMenu::FindMenuIdByMenuItem( GtkWidget *menuItem ) const
 {
@@ -672,7 +696,7 @@ int wxMenu::FindMenuIdByMenuItem( GtkWidget *menuItem ) const
         node = node->Next();
     }
 
-    return -1;
+    return wxNOT_FOUND;
 }
 
 wxMenuItem *wxMenu::FindItem(int id) const
@@ -705,10 +729,9 @@ wxWindow *wxMenu::GetInvokingWindow()
     return m_invokingWindow;
 }
 
-// Update a menu and all submenus recursively.
+// Update a menu and all submenus recursively. source is the object that has
-// source is the object that has the update event handlers
+// the update event handlers defined for it. If NULL, the menu or associated
-// defined for it. If NULL, the menu or associated window
+// window will be used.
-// will be used.
 void wxMenu::UpdateUI(wxEvtHandler* source)
 {
   if (!source && GetInvokingWindow())
@@ -745,4 +768,3 @@ void wxMenu::UpdateUI(wxEvtHandler* source)
   }
 }
 
-

src/gtk1/window.cpp

@@ -2750,7 +2750,7 @@ void wxWindow::InitDialog()
 static void SetInvokingWindow( wxMenu *menu, wxWindow *win )
 {
     menu->SetInvokingWindow( win );
-    wxNode *node = menu->m_items.First();
+    wxNode *node = menu->GetItems().First();
     while (node)
     {
         wxMenuItem *menuitem = (wxMenuItem*)node->Data();

src/msw/menu.cpp

@@ -36,9 +36,14 @@
 #include "wx/log.h"
 
 // other standard headers
-// ----------------------
 #include <string.h>
 
+// ----------------------------------------------------------------------------
+// global variables
+// ----------------------------------------------------------------------------
+
+extern wxMenu *wxCurrentPopupMenu;
+
 // ----------------------------------------------------------------------------
 // constants
 // ----------------------------------------------------------------------------
@@ -47,13 +52,17 @@
 static const int idMenuTitle = -2;
 
 // ----------------------------------------------------------------------------
-// wxWindows macros
+// macros
 // ----------------------------------------------------------------------------
+
 #if !USE_SHARED_LIBRARY
     IMPLEMENT_DYNAMIC_CLASS(wxMenu, wxEvtHandler)
     IMPLEMENT_DYNAMIC_CLASS(wxMenuBar, wxEvtHandler)
 #endif
 
+// convenience macro
+#define GetHMENU()      ((HMENU)GetHMenu())
+
 // ============================================================================
 // implementation
 // ============================================================================
@@ -61,9 +70,9 @@ static const int idMenuTitle = -2;
 // Menus
 
 // Construct a menu with optional title (then use append)
-wxMenu::wxMenu(const wxString& Title, const wxFunction func)
+wxMenu::wxMenu(const wxString& title, const wxFunction func)
+      : m_title(title)
 {
-  m_title = Title;
     m_parent = NULL;
     m_eventHandler = this;
     m_pInvokingWindow = NULL;
@@ -75,7 +84,7 @@ wxMenu::wxMenu(const wxString& Title, const wxFunction func)
     m_topLevelMenu = this;
     m_clientData = (void*) NULL;
 
-  if (m_title != "")
+    if ( !!m_title )
     {
         Append(idMenuTitle, m_title) ;
         AppendSeparator() ;
@@ -199,8 +208,7 @@ void wxMenu::Append(wxMenuItem *pItem)
         // TODO use SetMenuItemInfo(MFS_DEFAULT) to put it in bold face
     }
 
-  HMENU hMenu = (HMENU)((m_hMenu == 0) ? m_savehMenu : m_hMenu);
+    if ( !AppendMenu(GetHMENU(), flags, id, pData) )
-  if ( !AppendMenu(hMenu, flags, id, pData) )
     {
         wxLogLastError("AppendMenu");
     }
@@ -214,8 +222,8 @@ void wxMenu::AppendSeparator()
 }
 
 // Pullright item
-void wxMenu::Append(int Id, const wxString& label, wxMenu *SubMenu, 
+void wxMenu::Append(int Id, const wxString& label,
-                    const wxString& helpString)
+                    wxMenu *SubMenu, const wxString& helpString)
 {
     Append(new wxMenuItem(this, Id, label, helpString, FALSE, SubMenu));
 }
@@ -230,21 +238,19 @@ void wxMenu::Append(int Id, const wxString& label,
 
 void wxMenu::Delete(int id)
 {
-  wxNode *node;
-  int pos;
-  HMENU menu;
-
     wxMenuItem *item = NULL;
-  for (pos = 0, node = m_menuItems.First(); node; node = node->Next(), pos++) {
+    int pos;
+    wxNode *node;
+    for (pos = 0, node = m_menuItems.First(); node; node = node->Next(), pos++)
+    {
         item = (wxMenuItem *)node->Data();
         if ( item->GetId() == id )
             break;
     }
 
-  if (!node)
+    wxCHECK_RET( node, "wxMenu::Delete(): item doesn't exist" );
-	return;
 
-  menu = (HMENU)(m_hMenu ? m_hMenu : m_savehMenu);
+    HMENU menu = GetHMENU();
 
     wxMenu *pSubMenu = item->GetSubMenu();
     if ( pSubMenu != NULL ) {
@@ -275,10 +281,10 @@ void wxMenu::Enable(int Id, bool Flag)
     item->Enable(Flag);
 }
 
-bool wxMenu::Enabled(int Id) const
+bool wxMenu::IsEnabled(int Id) const
 {
     wxMenuItem *item = FindItemForId(Id);
-  wxCHECK( item != NULL, FALSE );
+    wxCHECK( item != NULL, FALSE, "invalid item id" );
 
     return item->IsEnabled();
 }
@@ -291,10 +297,10 @@ void wxMenu::Check(int Id, bool Flag)
     item->Check(Flag);
 }
 
-bool wxMenu::Checked(int Id) const
+bool wxMenu::IsChecked(int Id) const
 {
     wxMenuItem *item = FindItemForId(Id);
-  wxCHECK( item != NULL, FALSE );
+    wxCHECK( item != NULL, FALSE, "invalid item id" );
 
     return item->IsChecked();
 }
@@ -304,7 +310,7 @@ void wxMenu::SetTitle(const wxString& label)
     bool hasNoTitle = m_title.IsEmpty();
     m_title = label;
 
-  HMENU hMenu = (HMENU)((m_hMenu == 0) ? m_savehMenu : m_hMenu);
+    HMENU hMenu = GetHMENU();
 
     if ( hasNoTitle )
     {
@@ -371,16 +377,10 @@ void wxMenu::SetLabel(int Id, const wxString& label)
 
     if (item->GetSubMenu()==NULL)
     {
-    if (m_hMenu)
+        HMENU hMenu = GetHMENU();
-    {
+
-      UINT was_flag = GetMenuState((HMENU)m_hMenu,Id,MF_BYCOMMAND) ;
+        UINT was_flag = GetMenuState(hMenu, Id, MF_BYCOMMAND);
-      ModifyMenu((HMENU)m_hMenu,Id,MF_BYCOMMAND|MF_STRING|was_flag,Id,(const char *)label) ;
+        ModifyMenu(hMenu, Id, MF_BYCOMMAND | MF_STRING | was_flag, Id, label);
-    }
-    else if (m_savehMenu)
-    {
-      UINT was_flag = GetMenuState((HMENU)m_savehMenu,Id,MF_BYCOMMAND) ;
-      ModifyMenu((HMENU)m_savehMenu,Id,MF_BYCOMMAND|MF_STRING|was_flag,Id,(const char *)label) ;
-    }
     }
     else
     {
@@ -405,24 +405,14 @@ void wxMenu::SetLabel(int Id, const wxString& label)
 
 wxString wxMenu::GetLabel(int id) const
 {
-/*
+    wxString label;
-  static char tmp[128] ;
-  int len;
-  if (m_hMenu)
-    len = GetMenuString((HMENU)m_hMenu,Id,tmp,WXSIZEOF(tmp) - 1,MF_BYCOMMAND);
-  else if (m_savehMenu)
-    len = GetMenuString((HMENU)m_savehMenu,Id,tmp,WXSIZEOF(tmp) - 1,MF_BYCOMMAND);
-  else
-    len = 0 ;
-  tmp[len] = '\0' ;
-  return wxString(tmp) ;
-
-*/
     wxMenuItem *pItem = FindItemForId(id) ;
     if (pItem)
-        return pItem->GetName() ;
+        label = pItem->GetName() ;
     else
-        return wxEmptyString;
+        wxFAIL_MSG("wxMenu::GetLabel: item doesn't exist");
+
+    return label;
 }
 
 bool wxMenu::MSWCommand(WXUINT WXUNUSED(param), WXWORD id)
@@ -445,6 +435,7 @@ bool wxMenu::MSWCommand(WXUINT WXUNUSED(param), WXWORD id)
 // Finds the item id matching the given string, -1 if not found.
 int wxMenu::FindItem (const wxString& itemString) const
 {
+    // FIXME fixed size buffer
     char buf1[200];
     char buf2[200];
     wxStripMenuCodes ((char *)(const char *)itemString, buf1);
@@ -466,7 +457,7 @@ int wxMenu::FindItem (const wxString& itemString) const
         }
     }
 
-  return -1;
+    return wxNOT_FOUND;
 }
 
 wxMenuItem *wxMenu::FindItemForId(int itemId, wxMenu ** itemMenu) const
@@ -502,15 +493,20 @@ void wxMenu::SetHelpString(int itemId, const wxString& helpString)
     wxMenuItem *item = FindItemForId (itemId);
     if (item)
         item->SetHelp(helpString);
+    else
+        wxFAIL_MSG("wxMenu::SetHelpString: item doesn't exist");
 }
 
 wxString wxMenu::GetHelpString (int itemId) const
 {
+    wxString help;
     wxMenuItem *item = FindItemForId (itemId);
     if (item)
-    return item->GetHelp();
+        help = item->GetHelp();
     else
-    return wxEmptyString;
+        wxFAIL_MSG("wxMenu::GetHelpString: item doesn't exist");
+
+    return help;
 }
 
 void wxMenu::ProcessCommand(wxCommandEvent & event)
@@ -530,13 +526,13 @@ void wxMenu::ProcessCommand(wxCommandEvent & event)
         processed = GetEventHandler()->ProcessEvent(event);
     }
 
-  // Try the window the menu was popped up from (and up
+    // Try the window the menu was popped up from (and up through the
-  // through the hierarchy)
+    // hierarchy)
-  if ( !processed && GetInvokingWindow())
+    wxWindow *win = GetInvokingWindow();
-    processed = GetInvokingWindow()->GetEventHandler()->ProcessEvent(event);
+    if ( !processed && win )
+        processed = win->GetEventHandler()->ProcessEvent(event);
 }
 
-extern wxMenu *wxCurrentPopupMenu;
 bool wxWindow::PopupMenu(wxMenu *menu, int x, int y)
 {
     menu->SetInvokingWindow(this);
@@ -636,106 +632,64 @@ wxMenuBar::~wxMenuBar()
 
 // Must only be used AFTER menu has been attached to frame,
 // otherwise use individual menus to enable/disable items
-void wxMenuBar::Enable(int Id, bool Flag)
+void wxMenuBar::Enable(int Id, bool enable)
 {
-  int ms_flag;
+    int flag = enable ? MF_ENABLED : MF_GRAYED;
-  if (Flag)
-    ms_flag = MF_ENABLED;
-  else
-    ms_flag = MF_GRAYED;
 
     wxMenu *itemMenu = NULL;
     wxMenuItem *item = FindItemForId(Id, &itemMenu) ;
-  if (!item)
-    return;
 
-  if (itemMenu->m_hMenu)
+    wxCHECK_RET( item, "attempt to enable an item which doesn't exist" );
-    EnableMenuItem((HMENU)itemMenu->m_hMenu, Id, MF_BYCOMMAND | ms_flag);
-  else if (itemMenu->m_savehMenu)
-    EnableMenuItem((HMENU)itemMenu->m_savehMenu, Id, MF_BYCOMMAND | ms_flag);
 
+    EnableMenuItem(GetHMENU(), Id, MF_BYCOMMAND | flag);
 }
 
-void wxMenuBar::EnableTop(int pos, bool flag)
+void wxMenuBar::EnableTop(int pos, bool enable)
 {
-  int ms_flag;
+    int flag = enable ? MF_ENABLED : MF_GRAYED;;
-  if (flag)
-    ms_flag = MF_ENABLED;
-  else
-    ms_flag = MF_GRAYED;
 
-  EnableMenuItem((HMENU)m_hMenu, pos, MF_BYPOSITION | ms_flag);
+    EnableMenuItem((HMENU)m_hMenu, pos, MF_BYPOSITION | flag);
     DrawMenuBar((HWND) m_menuBarFrame->GetHWND()) ;
 }
 
 // Must only be used AFTER menu has been attached to frame,
 // otherwise use individual menus
-void wxMenuBar::Check(int Id, bool Flag)
+void wxMenuBar::Check(int Id, bool check)
 {
     wxMenu *itemMenu = NULL;
     wxMenuItem *item = FindItemForId(Id, &itemMenu) ;
-  if (!item)
-    return;
 
-  if (!item->IsCheckable())
+    wxCHECK_RET( item, "attempt to check an item which doesn't exist" );
-    return ;
+    wxCHECK_RET( item->IsCheckable(), "attempt to check an uncheckable item" );
-  int ms_flag;
-  if (Flag)
-    ms_flag = MF_CHECKED;
-  else
-    ms_flag = MF_UNCHECKED;
 
-  if (itemMenu->m_hMenu)
+    int flag = check ? MF_CHECKED : MF_UNCHECKED;
-    CheckMenuItem((HMENU)itemMenu->m_hMenu, Id, MF_BYCOMMAND | ms_flag);
+    CheckMenuItem(GetHMENU(), Id, MF_BYCOMMAND | flag);
-  else if (itemMenu->m_savehMenu)
-    CheckMenuItem((HMENU)itemMenu->m_savehMenu, Id, MF_BYCOMMAND | ms_flag);
-
-//  CheckMenuItem((HMENU)m_hMenu, Id, MF_BYCOMMAND | ms_flag);
 }
 
-bool wxMenuBar::Checked(int Id) const
+bool wxMenuBar::IsChecked(int Id) const
 {
     wxMenu *itemMenu = NULL;
     wxMenuItem *item = FindItemForId(Id, &itemMenu) ;
-  if (!item)
-    return FALSE;
 
-  int Flag = 0;
+    wxCHECK_MSG( item, FALSE, "wxMenuItem::IsChecked(): no such item" );
 
-  if (itemMenu->m_hMenu)
+    int flag = ::GetMenuState(GetHMENU(), Id, MF_BYCOMMAND);
-    Flag=GetMenuState((HMENU)itemMenu->m_hMenu, Id, MF_BYCOMMAND) ;
-  else if (itemMenu->m_savehMenu)
-    Flag=GetMenuState((HMENU)itemMenu->m_savehMenu, Id, MF_BYCOMMAND) ;
 
-//  Flag=GetMenuState((HMENU)m_hMenu, Id, MF_BYCOMMAND) ;
+    return (flag & MF_CHECKED) != 0;
-
-  if (Flag&MF_CHECKED)
-    return TRUE ;
-  else
-    return FALSE ;
 }
 
-bool wxMenuBar::Enabled(int Id) const
+bool wxMenuBar::IsEnabled(int Id) const
 {
     wxMenu *itemMenu = NULL;
     wxMenuItem *item = FindItemForId(Id, &itemMenu) ;
-  if (!item)
-    return FALSE;
 
-  int Flag ;
+    wxCHECK_MSG( item, FALSE, "wxMenuItem::IsEnabled(): no such item" );
 
-  if (itemMenu->m_hMenu)
+    int flag = ::GetMenuState(GetHMENU(), Id, MF_BYCOMMAND) ;
-    Flag=GetMenuState((HMENU)itemMenu->m_hMenu, Id, MF_BYCOMMAND) ;
-  else if (itemMenu->m_savehMenu)
-    Flag=GetMenuState((HMENU)itemMenu->m_savehMenu, Id, MF_BYCOMMAND) ;
 
-  if (Flag&MF_ENABLED)
+    return (flag & MF_ENABLED) != 0;
-    return TRUE ;
-  else
-    return FALSE ;
 }
 
-
 void wxMenuBar::SetLabel(int Id, const wxString& label)
 {
     wxMenu *itemMenu = NULL;
@@ -744,16 +698,9 @@ void wxMenuBar::SetLabel(int Id, const wxString& label)
     if (!item)
         return;
 
-  if (itemMenu->m_hMenu)
+    HMENU hMenu = GetHMENU();
-  {
+    UINT was_flag = ::GetMenuState(hMenu, Id, MF_BYCOMMAND);
-    UINT was_flag = GetMenuState((HMENU)itemMenu->m_hMenu,Id,MF_BYCOMMAND) ;
+    ::ModifyMenu(hMenu, Id, MF_BYCOMMAND | MF_STRING | was_flag, Id, label);
-    ModifyMenu((HMENU)itemMenu->m_hMenu,Id,MF_BYCOMMAND|MF_STRING|was_flag,Id,(const char *)label) ;
-  }
-  else if (itemMenu->m_savehMenu)
-  {
-    UINT was_flag = GetMenuState((HMENU)itemMenu->m_savehMenu,Id,MF_BYCOMMAND) ;
-    ModifyMenu((HMENU)itemMenu->m_savehMenu,Id,MF_BYCOMMAND|MF_STRING|was_flag,Id,(const char *)label) ;
-  }
 }
 
 wxString wxMenuBar::GetLabel(int Id) const
@@ -761,22 +708,12 @@ wxString wxMenuBar::GetLabel(int Id) const
     wxMenu *itemMenu = NULL;
     wxMenuItem *item = FindItemForId(Id, &itemMenu) ;
 
-  if (!item)
+    wxCHECK_MSG( item, "", "wxMenuItem::GetLabel(): no such item" );
-    return wxString("");
 
-  static char tmp[128] ;
+    char tmp[128];
-  int len = 0;
+    int len = GetMenuString(GetHMENU(), Id, tmp, WXSIZEOF(tmp), MF_BYCOMMAND);
-  if (itemMenu->m_hMenu)
-  {
-    len = GetMenuString((HMENU)itemMenu->m_hMenu,Id,tmp,127,MF_BYCOMMAND) ;
-  }
-  else if (itemMenu->m_savehMenu)
-  {
-    len = GetMenuString((HMENU)itemMenu->m_savehMenu,Id,tmp,127,MF_BYCOMMAND) ;
-  }
-
-//  int len = GetMenuString((HMENU)m_hMenu,Id,tmp,127,MF_BYCOMMAND) ;
     tmp[len] = '\0' ;
+
     return wxString(tmp);
 }
 
@@ -948,21 +885,33 @@ void wxMenuBar::SetHelpString (int Id, const wxString& helpString)
 
 wxString wxMenuBar::GetHelpString (int Id) const
 {
-  int i;
+    wxString helpString;
-  for (i = 0; i < m_menuCount; i++)
+
+    for (int i = 0; i < m_menuCount; i++)
     {
-      if (m_menus[i]->FindItemForId (Id))
+        wxMenuItem *item = m_menus[i]->FindItemForId(Id);
-	    return wxString(m_menus[i]->GetHelpString (Id));
+        if ( item )
+        {
+            helpString = item->GetHelp();
+
+            break;
         }
-  return wxString("");
     }
 
+    return helpString;
+}
+
+// ----------------------------------------------------------------------------
+// helper functions
+// ----------------------------------------------------------------------------
+
 wxWindow *wxMenu::GetWindow() const
 {
     if ( m_pInvokingWindow != NULL )
         return m_pInvokingWindow;
-  if ( m_menuBar != NULL)       
+    else if ( m_menuBar != NULL)
         return m_menuBar->m_menuBarFrame;
+
     return NULL;
 }
 
@@ -973,13 +922,14 @@ WXHMENU wxMenu::GetHMenu() const
     else if ( m_savehMenu != 0 )
         return m_savehMenu;
 
+    wxFAIL_MSG("wxMenu without HMENU");
+
     return 0;
 }
 
-// Update a menu and all submenus recursively.
+// Update a menu and all submenus recursively. source is the object that has
-// source is the object that has the update event handlers
+// the update event handlers defined for it. If NULL, the menu or associated
-// defined for it. If NULL, the menu or associated window
+// window will be used.
-// will be used.
 void wxMenu::UpdateUI(wxEvtHandler* source)
 {
     if (!source && GetInvokingWindow())