Remove wxListCtrl::EnableSortIndicator()

The old API seems unnecessarily complex, it is simpler to just let the
application call ShowSortIndicator() itself from its
wxEVT_LIST_COL_CLICK handler, which needs to be defined anyhow in order
to actually sort the items, rather than require it to enable sort
indicator, explicitly set it initially and then remember to not set it
any longer in response to the column clicks.

Also make RemoveSortIndicator() non-virtual and implement it simply as
ShowSortIndicator(-1) because this actually simplifies the code too.

include/wx/generic/listctrl.h

@@ -115,10 +115,7 @@ public:
     bool IsItemChecked(long item) const wxOVERRIDE;
     void CheckItem(long item, bool check) wxOVERRIDE;
 
-    void EnableSortIndicator(bool enable = true) wxOVERRIDE;
-    bool IsSortIndicatorEnabled() const wxOVERRIDE;
     void ShowSortIndicator(int idx, bool ascending = true) wxOVERRIDE;
-    void RemoveSortIndicator() wxOVERRIDE;
     int GetSortIndicator() const wxOVERRIDE;
     bool IsAscendingSortIndicator() const wxOVERRIDE;
 

include/wx/generic/private/listctrl.h

@@ -381,7 +381,6 @@ public:
     int m_colToSend;
     int m_widthToSend;
 
-    bool m_enableSortCol;
     bool m_sortAsc;
     int m_sortCol;
 

include/wx/listbase.h

@@ -447,10 +447,8 @@ public:
     virtual void CheckItem(long WXUNUSED(item), bool WXUNUSED(check)) { }
 
     // Sort indicator in header.
-    virtual void EnableSortIndicator(bool WXUNUSED(enable) = true) { }
-    virtual bool IsSortIndicatorEnabled() const { return false; }
     virtual void ShowSortIndicator(int WXUNUSED(idx), bool WXUNUSED(ascending) = true) { }
-    virtual void RemoveSortIndicator() { }
+    void RemoveSortIndicator() { ShowSortIndicator(-1); }
     virtual int GetSortIndicator() const { return -1; }
     virtual bool IsAscendingSortIndicator() const { return true; }
 

include/wx/msw/listctrl.h

@@ -226,10 +226,7 @@ public:
     void CheckItem(long item, bool check) wxOVERRIDE;
 
     // Sort indicator in header
-    void EnableSortIndicator(bool enable = true) wxOVERRIDE;
-    bool IsSortIndicatorEnabled() const wxOVERRIDE;
     void ShowSortIndicator(int idx, bool ascending = true) wxOVERRIDE;
-    void RemoveSortIndicator() wxOVERRIDE;
     int GetSortIndicator() const wxOVERRIDE;
     bool IsAscendingSortIndicator() const wxOVERRIDE;
 
@@ -426,21 +423,16 @@ protected:
     int               m_colCount;   // Windows doesn't have GetColumnCount so must
                                     // keep track of inserted/deleted columns
 
-    // m_sortCol and m_sortAsc are used only if m_enableSortCol is true.
-    //
-    // Note that m_sortCol may be set to -1, but this is not the same as
-    // setting m_enableSortCol to false, as the control updates the sort
-    // indicator on column click in the former case, but not in the latter.
-    bool m_enableSortCol;
-    bool m_sortAsc;
-    int m_sortCol;
-
     // all wxMSWListItemData objects we use
     wxVector<wxMSWListItemData *> m_internalData;
 
     // true if we have any items with custom attributes
     bool m_hasAnyAttr;
 
+    // m_sortAsc is only used if m_sortCol != -1
+    bool m_sortAsc;
+    int m_sortCol;
+
 private:
     // process NM_CUSTOMDRAW notification message
     WXLPARAM OnCustomDraw(WXLPARAM lParam);
@@ -462,8 +454,7 @@ private:
     // in-place editor control.
     void OnCharHook(wxKeyEvent& event);
 
-    // Draw the sort arrow in the header. Should only be called when sort
+    // Draw the sort arrow in the header.
-    // indicators are enabled.
     void DrawSortArrow();
 
     // Object using for header custom drawing if necessary, may be NULL.

interface/wx/listctrl.h

@@ -1420,40 +1420,15 @@ public:
     */
     void ExtendRulesAndAlternateColour(bool extend = true);
 
-    /**
-        Enable or disable showing a sort indicator in the header bar.
-        Sort indicators are only shown in report view.
-
-        When clicking on the header of a column, this column will get the sort-
-        indicator in ascending order, or toggle it in the opposite order. To
-        sort the list, call SortItems() in EVT_LIST_COL_CLICK.
-
-        Note that calling ShowSortIndicator() implicitly enables sort
-        indicators.
-
-        @note In wxMSW, this will disable the header icon of the column.
-
-        @param enable
-            If @true, enable showing a sort indicator, otherwise disable.
-
-        @since 3.1.6
-    */
-    void EnableSortIndicator(bool enable);
-
-    /**
-        Returns true if a sort indicator is enabled.
-
-        @see EnableSortIndicator()
-
-        @since 3.1.6
-    */
-    bool IsSortIndicatorEnabled() const;
-
     /**
         Show the sort indicator of a specific column in a specific direction.
 
-        This function also enables showing sort indicators if
+        Sort indicators are only shown in report view and in the native wxMSW
-        EnableSortIndicator() hadn't been called.
+        version override any column icon, i.e. if the sort indicator is shown
+        for a column, no (other) icon is shown.
+
+        This function should typically be called from EVT_LIST_COL_CLICK
+        handler.
 
         @note This does not actually sort the list, use SortItems() for this.
 
@@ -1479,6 +1454,8 @@ public:
     /**
         Returns the column that shows the sort indicator.
 
+        Can return @c -1 if there is no sort indicator currently shown.
+
         @since 3.1.6
     */
     int GetSortIndicator() const;

samples/listctrl/listtest.cpp

@@ -461,8 +461,6 @@ void MyFrame::RecreateList(long flags, bool withText)
                                     flags |
                                     wxBORDER_THEME | wxLC_EDIT_LABELS);
 
-        m_listCtrl->EnableSortIndicator();
-
         if ( old )
         {
             wxSizer* const sizer = m_panel->GetSizer();
@@ -1091,10 +1089,18 @@ void MyListCtrl::OnColClick(wxListEvent& event)
         return; // clicked outside any column.
     }
 
-    if ( IsSortIndicatorEnabled() )
+    // If clicking on the same column by which we already sort, toggle the sort
+    // direction, otherwise use ascending sort by default.
+    bool ascending;
+    if ( col == GetSortIndicator() )
+        ascending = !IsAscendingSortIndicator();
+    else
+        ascending = true;
+
+    // sort on item data (SetItemData)
+    if ( SortItems(MyCompareFunction, ascending) )
     {
-        // sort on  item data (SetItemData), disable when sorting fails
+        ShowSortIndicator(col, ascending);
-        EnableSortIndicator( SortItems(MyCompareFunction, IsAscendingSortIndicator()) );
     }
 
     // set or unset image

src/generic/listctrl.cpp

@@ -982,7 +982,6 @@ wxListHeaderWindow::wxListHeaderWindow()
     m_owner = NULL;
     m_resizeCursor = NULL;
 
-    m_enableSortCol = false;
     m_sortAsc = true;
     m_sortCol = -1;
 }
@@ -1104,7 +1103,7 @@ void wxListHeaderWindow::OnPaint( wxPaintEvent &WXUNUSED(event) )
 #endif
 
         wxHeaderSortIconType sortArrow = wxHDR_SORT_ICON_NONE;
-        if ( m_enableSortCol && i == m_sortCol )
+        if ( i == m_sortCol )
         {
             if ( m_sortAsc )
                 sortArrow = wxHDR_SORT_ICON_UP;
@@ -1334,12 +1333,6 @@ void wxListHeaderWindow::OnMouse( wxMouseEvent &event )
                             colItem.SetState(state & ~wxLIST_STATE_SELECTED);
                         m_owner->SetColumn(i, colItem);
                     }
-
-                    if ( m_sortCol != m_column )
-                        m_sortAsc = true;
-                    else
-                        m_sortAsc = !m_sortAsc;
-                    m_sortCol = m_column;
                 }
 
                 SendListEvent( event.LeftDown()
@@ -5084,47 +5077,18 @@ bool wxGenericListCtrl::IsItemChecked(long item) const
     return m_mainWin->IsItemChecked(item);
 }
 
-void wxGenericListCtrl::EnableSortIndicator(bool enable)
-{
-    if ( m_headerWin && enable != m_headerWin->m_enableSortCol )
-    {
-        m_headerWin->m_enableSortCol = enable;
-        m_headerWin->Refresh();
-    }
-}
-
-bool wxGenericListCtrl::IsSortIndicatorEnabled() const
-{
-    return m_headerWin && m_headerWin->m_enableSortCol;
-}
-
 void wxGenericListCtrl::ShowSortIndicator(int idx, bool ascending)
 {
-    if ( idx == -1 )
+    if ( m_headerWin &&
-    {
-        RemoveSortIndicator();
-    }
-    else if ( m_headerWin &&
                 (idx != m_headerWin->m_sortCol ||
-                 ascending != m_headerWin->m_sortAsc) )
+                 (idx != -1 && ascending != m_headerWin->m_sortAsc)) )
     {
-        m_headerWin->m_enableSortCol = true;
         m_headerWin->m_sortCol = idx;
         m_headerWin->m_sortAsc = ascending;
         m_headerWin->Refresh();
     }
 }
 
-void wxGenericListCtrl::RemoveSortIndicator()
-{
-    if ( m_headerWin && m_headerWin->m_sortCol != -1 )
-    {
-        m_headerWin->m_sortCol = -1;
-        m_headerWin->m_sortAsc = true;
-        m_headerWin->Refresh();
-    }
-}
-
 int wxGenericListCtrl::GetSortIndicator() const
 {
     if ( m_headerWin )

src/msw/listctrl.cpp

@@ -278,7 +278,6 @@ void wxListCtrl::Init()
     m_colCount = 0;
     m_textCtrl = NULL;
 
-    m_enableSortCol = false;
     m_sortAsc = true;
     m_sortCol = -1;
 
@@ -1456,50 +1455,16 @@ bool wxListCtrl::IsItemChecked(long item) const
     return ListView_GetCheckState(GetHwnd(), (UINT)item) != 0;
 }
 
-void wxListCtrl::EnableSortIndicator(bool enable)
-{
-    m_enableSortCol = enable;
-
-    // This is the only place where we call this function even with disabled
-    // sort indicators because we may need to reset the currently shown
-    // indicator after disabling it.
-    //
-    // Also note that we should *not* skip calling it m_enableSortCol didn't
-    // change, as ShowSortIndicator() relies on it being called here.
-    DrawSortArrow();
-}
-
-bool wxListCtrl::IsSortIndicatorEnabled() const
-{
-    return m_enableSortCol;
-}
-
 void wxListCtrl::ShowSortIndicator(int idx, bool ascending)
 {
-    if ( idx == -1 )
+    if ( idx != m_sortCol || (idx != -1 && ascending != m_sortAsc) )
-    {
-        RemoveSortIndicator();
-    }
-    else
     {
         m_sortCol = idx;
         m_sortAsc = ascending;
 
-        // We need to enable the sort indicators if they're not enabled yet and
-        // if they're already enabled, this will update the actually shown sort
-        // indicator.
-        EnableSortIndicator();
-    }
-}
-
-void wxListCtrl::RemoveSortIndicator()
-{
-    m_sortCol = -1;
-    m_sortAsc = true;
-
-    if ( IsSortIndicatorEnabled() )
         DrawSortArrow();
     }
+}
 
 int wxListCtrl::GetSortIndicator() const
 {
@@ -2104,7 +2069,9 @@ long wxListCtrl::DoInsertColumn(long col, const wxListItem& item)
         SetColumnWidth(n, wxLIST_AUTOSIZE_USEHEADER);
     }
 
-    if ( IsSortIndicatorEnabled() )
+    // Update the sort indicator if the index of the column for which it was
+    // set changed. Note that this condition works even if m_sortCol == -1.
+    if ( col <= m_sortCol )
         DrawSortArrow();
 
     return n;
@@ -2481,14 +2448,6 @@ bool wxListCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result)
                 break;
 
             case LVN_COLUMNCLICK:
-                if ( m_sortCol != nmLV->iSubItem )
-                    m_sortAsc = true;
-                else
-                    m_sortAsc = !m_sortAsc;
-                m_sortCol = nmLV->iSubItem;
-                if ( IsSortIndicatorEnabled() )
-                    DrawSortArrow();
-
                 eventType = wxEVT_LIST_COL_CLICK;
                 event.m_itemIndex = -1;
                 event.m_col = nmLV->iSubItem;