From ec36eb990bc2a95b7f16ce5ebb74a14e97e134c1 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 10 Mar 2020 22:32:05 +0100 Subject: [PATCH 1/5] Avoid code duplication in wxAuiToolBar::DeleteTool() Delegate to the existing DeleteByIndex() instead of repeating its code in DeleteTool(). No real changes, this is just a refactoring. --- src/aui/auibar.cpp | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/aui/auibar.cpp b/src/aui/auibar.cpp index 4b6f3379c0..61a3780481 100644 --- a/src/aui/auibar.cpp +++ b/src/aui/auibar.cpp @@ -1165,15 +1165,7 @@ void wxAuiToolBar::Clear() bool wxAuiToolBar::DeleteTool(int tool_id) { - int idx = GetToolIndex(tool_id); - if (idx >= 0 && idx < (int)m_items.GetCount()) - { - m_items.RemoveAt(idx); - Realize(); - return true; - } - - return false; + return DeleteByIndex(GetToolIndex(tool_id)); } bool wxAuiToolBar::DeleteByIndex(int idx) From 931f4b8d20000c1558189aa0d656fdef3445a734 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 10 Mar 2020 23:18:48 +0100 Subject: [PATCH 2/5] Rename local variable in wxAUI to not have "m_" prefix This was just confusing, as only member variables are supposed to use this prefix. No real changes. --- src/aui/auibar.cpp | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/aui/auibar.cpp b/src/aui/auibar.cpp index 61a3780481..2d2fd80bb3 100644 --- a/src/aui/auibar.cpp +++ b/src/aui/auibar.cpp @@ -1938,14 +1938,14 @@ bool wxAuiToolBar::RealizeHelper(wxClientDC& dc, bool horizontal) for (i = 0, count = m_items.GetCount(); i < count; ++i) { wxAuiToolBarItem& item = m_items.Item(i); - wxSizerItem* m_sizerItem = NULL; + wxSizerItem* sizerItem = NULL; switch (item.m_kind) { case wxITEM_LABEL: { wxSize size = m_art->GetLabelSize(dc, this, item); - m_sizerItem = sizer->Add(size.x + (m_toolBorderPadding*2), + sizerItem = sizer->Add(size.x + (m_toolBorderPadding*2), size.y + (m_toolBorderPadding*2), item.m_proportion, item.m_alignment); @@ -1962,7 +1962,7 @@ bool wxAuiToolBar::RealizeHelper(wxClientDC& dc, bool horizontal) case wxITEM_RADIO: { wxSize size = m_art->GetToolSize(dc, this, item); - m_sizerItem = sizer->Add(size.x + (m_toolBorderPadding*2), + sizerItem = sizer->Add(size.x + (m_toolBorderPadding*2), size.y + (m_toolBorderPadding*2), 0, item.m_alignment); @@ -1978,9 +1978,9 @@ bool wxAuiToolBar::RealizeHelper(wxClientDC& dc, bool horizontal) case wxITEM_SEPARATOR: { if (horizontal) - m_sizerItem = sizer->Add(separatorSize, 1, 0, wxEXPAND); + sizerItem = sizer->Add(separatorSize, 1, 0, wxEXPAND); else - m_sizerItem = sizer->Add(1, separatorSize, 0, wxEXPAND); + sizerItem = sizer->Add(1, separatorSize, 0, wxEXPAND); // add tool packing if (i+1 < count) @@ -1993,14 +1993,13 @@ bool wxAuiToolBar::RealizeHelper(wxClientDC& dc, bool horizontal) case wxITEM_SPACER: if (item.m_proportion > 0) - m_sizerItem = sizer->AddStretchSpacer(item.m_proportion); + sizerItem = sizer->AddStretchSpacer(item.m_proportion); else - m_sizerItem = sizer->Add(item.m_spacerPixels, 1); + sizerItem = sizer->Add(item.m_spacerPixels, 1); break; case wxITEM_CONTROL: { - //m_sizerItem = sizer->Add(item.m_window, item.m_proportion, wxEXPAND); wxSizerItem* ctrl_m_sizerItem; wxBoxSizer* vert_sizer = new wxBoxSizer(wxVERTICAL); @@ -2016,7 +2015,7 @@ bool wxAuiToolBar::RealizeHelper(wxClientDC& dc, bool horizontal) } - m_sizerItem = sizer->Add(vert_sizer, item.m_proportion, wxEXPAND); + sizerItem = sizer->Add(vert_sizer, item.m_proportion, wxEXPAND); wxSize min_size = item.m_minSize; @@ -2030,7 +2029,7 @@ bool wxAuiToolBar::RealizeHelper(wxClientDC& dc, bool horizontal) if (min_size.IsFullySpecified()) { - m_sizerItem->SetMinSize(min_size); + sizerItem->SetMinSize(min_size); ctrl_m_sizerItem->SetMinSize(min_size); } @@ -2042,7 +2041,7 @@ bool wxAuiToolBar::RealizeHelper(wxClientDC& dc, bool horizontal) } } - item.m_sizerItem = m_sizerItem; + item.m_sizerItem = sizerItem; } // add "right" padding From 95b1f7b7ea0f300220ee70acfea92c2659cc09c4 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 10 Mar 2020 23:34:06 +0100 Subject: [PATCH 3/5] Document wxAuiToolBar::DeleteTool() and DeleteByIndex() The behaviour of these functions for the tools containing controls is counter-intuitive but changing it now would silently break existing code working around the current semantics, so just document it instead. See #16552. --- interface/wx/aui/auibar.h | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/interface/wx/aui/auibar.h b/interface/wx/aui/auibar.h index 899b956ff2..a3e23752d7 100644 --- a/interface/wx/aui/auibar.h +++ b/interface/wx/aui/auibar.h @@ -722,8 +722,32 @@ public: void ClearTools(); void Clear(); + + /** + Removes the tool with the given ID from the tool bar. + + Note that if this tool was added by AddControl(), the associated + control is @e not deleted and must either be reused (e.g. by + reparenting it under a different window) or destroyed by caller. + + @param tool_id ID of a previously added tool. + @return @true if the tool was removed or @false otherwise, e.g. if the + tool with the given ID was not found. + */ bool DeleteTool(int tool_id); - bool DeleteByIndex(int tool_id); + + /** + Removes the tool at the given position from the tool bar. + + Note that if this tool was added by AddControl(), the associated + control is @e not deleted and must either be reused (e.g. by + reparenting it under a different window) or destroyed by caller. + + @param idx The index, or position, of a previously added tool. + @return @true if the tool was removed or @false otherwise, e.g. if the + provided index is out of range. + */ + bool DeleteByIndex(int idx); size_t GetToolCount() const; int GetToolPos(int tool_id) const; From 700eaff131179e27cc2e70fbd4b57c4f1d429756 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 10 Mar 2020 23:37:42 +0100 Subject: [PATCH 4/5] Add wxAuiToolBar::DestroyTool() and DestroyToolByIndex() These new functions destroy the associated window too, unlike the existing DeleteTool() and DeleteByIndex(). Closes #16552. --- include/wx/aui/auibar.h | 6 ++++++ interface/wx/aui/auibar.h | 28 ++++++++++++++++++++++++++-- src/aui/auibar.cpp | 16 ++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/include/wx/aui/auibar.h b/include/wx/aui/auibar.h index 05b841983a..6b612b545a 100644 --- a/include/wx/aui/auibar.h +++ b/include/wx/aui/auibar.h @@ -525,6 +525,12 @@ public: void ClearTools() { Clear() ; } void Clear(); + + bool DestroyTool(int toolId); + bool DestroyToolByIndex(int idx); + + // Note that these methods do _not_ delete the associated control, if any. + // Use DestroyTool() or DestroyToolByIndex() if this is wanted. bool DeleteTool(int toolId); bool DeleteByIndex(int toolId); diff --git a/interface/wx/aui/auibar.h b/interface/wx/aui/auibar.h index a3e23752d7..04e261b7a3 100644 --- a/interface/wx/aui/auibar.h +++ b/interface/wx/aui/auibar.h @@ -724,11 +724,33 @@ public: void Clear(); /** - Removes the tool with the given ID from the tool bar. + Destroys the tool with the given ID and its associated window, if any. + + @param tool_id ID of a previously added tool. + @return @true if the tool was destroyed or @false otherwise, e.g. if + the tool with the given ID was not found. + + @since 3.1.4 + */ + bool DestroyTool(int tool_id); + + /** + Destroys the tool at the given position and its associated window, if + any. + + @param idx The index, or position, of a previously added tool. + @return @true if the tool was destroyed or @false otherwise, e.g. if + the provided index is out of range. + */ + bool DestroyToolByIndex(int idx); + + /** + Removes the tool with the given ID from the toolbar. Note that if this tool was added by AddControl(), the associated control is @e not deleted and must either be reused (e.g. by reparenting it under a different window) or destroyed by caller. + If this behaviour is unwanted, prefer using DestroyTool() instead. @param tool_id ID of a previously added tool. @return @true if the tool was removed or @false otherwise, e.g. if the @@ -737,11 +759,13 @@ public: bool DeleteTool(int tool_id); /** - Removes the tool at the given position from the tool bar. + Removes the tool at the given position from the toolbar. Note that if this tool was added by AddControl(), the associated control is @e not deleted and must either be reused (e.g. by reparenting it under a different window) or destroyed by caller. + If this behaviour is unwanted, prefer using DestroyToolByIndex() + instead. @param idx The index, or position, of a previously added tool. @return @true if the tool was removed or @false otherwise, e.g. if the diff --git a/src/aui/auibar.cpp b/src/aui/auibar.cpp index 2d2fd80bb3..22c1b45d3f 100644 --- a/src/aui/auibar.cpp +++ b/src/aui/auibar.cpp @@ -1180,6 +1180,22 @@ bool wxAuiToolBar::DeleteByIndex(int idx) return false; } +bool wxAuiToolBar::DestroyTool(int tool_id) +{ + return DestroyToolByIndex(GetToolIndex(tool_id)); +} + +bool wxAuiToolBar::DestroyToolByIndex(int idx) +{ + if ( idx < 0 || static_cast(idx) >= m_items.GetCount() ) + return false; + + if ( wxWindow* window = m_items[idx].GetWindow() ) + window->Destroy(); + + return DeleteByIndex(idx); +} + wxControl* wxAuiToolBar::FindControl(int id) { From ea359e02ab79ccb988f6b5fa6174bd8e075cc451 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 15 Mar 2020 17:09:54 +0100 Subject: [PATCH 5/5] Rename tool ID parameter in wxAuiToolBar documentation Use "toolId" to match the actual name in the header instead of "tool_id". No real changes. --- interface/wx/aui/auibar.h | 64 +++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/interface/wx/aui/auibar.h b/interface/wx/aui/auibar.h index 04e261b7a3..f7dee0092f 100644 --- a/interface/wx/aui/auibar.h +++ b/interface/wx/aui/auibar.h @@ -681,13 +681,13 @@ public: bool SetFont(const wxFont& font); - wxAuiToolBarItem* AddTool(int tool_id, + wxAuiToolBarItem* AddTool(int toolId, const wxString& label, const wxBitmap& bitmap, const wxString& short_help_string = wxEmptyString, wxItemKind kind = wxITEM_NORMAL); - wxAuiToolBarItem* AddTool(int tool_id, + wxAuiToolBarItem* AddTool(int toolId, const wxString& label, const wxBitmap& bitmap, const wxBitmap& disabled_bitmap, @@ -696,7 +696,7 @@ public: const wxString& long_help_string, wxObject* client_data); - wxAuiToolBarItem* AddTool(int tool_id, + wxAuiToolBarItem* AddTool(int toolId, const wxBitmap& bitmap, const wxBitmap& disabled_bitmap, bool toggle = false, @@ -704,7 +704,7 @@ public: const wxString& short_help_string = wxEmptyString, const wxString& long_help_string = wxEmptyString); - wxAuiToolBarItem* AddLabel(int tool_id, + wxAuiToolBarItem* AddLabel(int toolId, const wxString& label = wxEmptyString, const int width = -1); wxAuiToolBarItem* AddControl(wxControl* control, @@ -718,7 +718,7 @@ public: wxControl* FindControl(int window_id); wxAuiToolBarItem* FindToolByPosition(wxCoord x, wxCoord y) const; wxAuiToolBarItem* FindToolByIndex(int idx) const; - wxAuiToolBarItem* FindTool(int tool_id) const; + wxAuiToolBarItem* FindTool(int toolId) const; void ClearTools(); void Clear(); @@ -726,13 +726,13 @@ public: /** Destroys the tool with the given ID and its associated window, if any. - @param tool_id ID of a previously added tool. + @param toolId ID of a previously added tool. @return @true if the tool was destroyed or @false otherwise, e.g. if the tool with the given ID was not found. @since 3.1.4 */ - bool DestroyTool(int tool_id); + bool DestroyTool(int toolId); /** Destroys the tool at the given position and its associated window, if @@ -752,11 +752,11 @@ public: reparenting it under a different window) or destroyed by caller. If this behaviour is unwanted, prefer using DestroyTool() instead. - @param tool_id ID of a previously added tool. + @param toolId ID of a previously added tool. @return @true if the tool was removed or @false otherwise, e.g. if the tool with the given ID was not found. */ - bool DeleteTool(int tool_id); + bool DeleteTool(int toolId); /** Removes the tool at the given position from the toolbar. @@ -774,11 +774,11 @@ public: bool DeleteByIndex(int idx); size_t GetToolCount() const; - int GetToolPos(int tool_id) const; - int GetToolIndex(int tool_id) const; - bool GetToolFits(int tool_id) const; - wxRect GetToolRect(int tool_id) const; - bool GetToolFitsByIndex(int tool_id) const; + int GetToolPos(int toolId) const; + int GetToolIndex(int toolId) const; + bool GetToolFits(int toolId) const; + wxRect GetToolRect(int toolId) const; + bool GetToolFitsByIndex(int toolId) const; bool GetToolBarFits() const; void SetMargins(const wxSize& size); @@ -794,11 +794,11 @@ public: bool GetGripperVisible() const; void SetGripperVisible(bool visible); - void ToggleTool(int tool_id, bool state); - bool GetToolToggled(int tool_id) const; + void ToggleTool(int toolId, bool state); + bool GetToolToggled(int toolId) const; - void EnableTool(int tool_id, bool state); - bool GetToolEnabled(int tool_id) const; + void EnableTool(int toolId, bool state); + bool GetToolEnabled(int toolId) const; /** Set whether the specified toolbar item has a drop down button. @@ -807,7 +807,7 @@ public: @see wxAuiToolBarItem::SetHasDropDown() */ - void SetToolDropDown(int tool_id, bool dropdown); + void SetToolDropDown(int toolId, bool dropdown); /** Returns whether the specified toolbar item has an associated drop down @@ -815,7 +815,7 @@ public: @see wxAuiToolBarItem::HasDropDown() */ - bool GetToolDropDown(int tool_id) const; + bool GetToolDropDown(int toolId) const; void SetToolBorderPadding(int padding); int GetToolBorderPadding() const; @@ -826,26 +826,26 @@ public: void SetToolPacking(int packing); int GetToolPacking() const; - void SetToolProportion(int tool_id, int proportion); - int GetToolProportion(int tool_id) const; + void SetToolProportion(int toolId, int proportion); + int GetToolProportion(int toolId) const; void SetToolSeparation(int separation); int GetToolSeparation() const; - void SetToolSticky(int tool_id, bool sticky); - bool GetToolSticky(int tool_id) const; + void SetToolSticky(int toolId, bool sticky); + bool GetToolSticky(int toolId) const; - wxString GetToolLabel(int tool_id) const; - void SetToolLabel(int tool_id, const wxString& label); + wxString GetToolLabel(int toolId) const; + void SetToolLabel(int toolId, const wxString& label); - wxBitmap GetToolBitmap(int tool_id) const; - void SetToolBitmap(int tool_id, const wxBitmap& bitmap); + wxBitmap GetToolBitmap(int toolId) const; + void SetToolBitmap(int toolId, const wxBitmap& bitmap); - wxString GetToolShortHelp(int tool_id) const; - void SetToolShortHelp(int tool_id, const wxString& help_string); + wxString GetToolShortHelp(int toolId) const; + void SetToolShortHelp(int toolId, const wxString& help_string); - wxString GetToolLongHelp(int tool_id) const; - void SetToolLongHelp(int tool_id, const wxString& help_string); + wxString GetToolLongHelp(int toolId) const; + void SetToolLongHelp(int toolId, const wxString& help_string); void SetCustomOverflowItems(const wxAuiToolBarItemArray& prepend, const wxAuiToolBarItemArray& append);