From 30ce892ed5c48c91c1b6d74a69e00bfc5da30296 Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Mon, 13 Dec 2021 14:31:49 +0000
Subject: [PATCH] Let wxListCtrl::ShowSortIndicator() implicitly enable
 indicators

It doesn't seem right for ShowSortIndicator() to silently do nothing if
EnableSortIndicator() hadn't been called before, so make it enable the
sort indicators if they hadn't been enabled yet.

The alternative would be to assert in this function, but this seems less
useful.

Also add some comments to wxMSW version.
---
 include/wx/msw/listctrl.h |  8 +++++++-
 interface/wx/listctrl.h   |  7 ++++++-
 src/generic/listctrl.cpp  |  1 +
 src/msw/listctrl.cpp      | 25 ++++++++++++++++++++-----
 4 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/include/wx/msw/listctrl.h b/include/wx/msw/listctrl.h
index 02e891353e..f058fe69cc 100644
--- a/include/wx/msw/listctrl.h
+++ b/include/wx/msw/listctrl.h
@@ -426,6 +426,11 @@ 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;
@@ -457,7 +462,8 @@ private:
     // in-place editor control.
     void OnCharHook(wxKeyEvent& event);
 
-    // Draw the sort arrow arror in the header.
+    // Draw the sort arrow in the header. Should only be called when sort
+    // indicators are enabled.
     void DrawSortArrow();
 
     // Object using for header custom drawing if necessary, may be NULL.
diff --git a/interface/wx/listctrl.h b/interface/wx/listctrl.h
index 1fb82c5f9d..e1b387f278 100644
--- a/interface/wx/listctrl.h
+++ b/interface/wx/listctrl.h
@@ -1428,6 +1428,9 @@ public:
         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
@@ -1448,7 +1451,9 @@ public:
 
     /**
         Show the sort indicator of a specific column in a specific direction.
-        Sort indicators have to be enabled using EnableSortIndicator().
+
+        This function also enables showing sort indicators if
+        EnableSortIndicator() hadn't been called.
 
         @note This does not actually sort the list, use SortItems() for this.
 
diff --git a/src/generic/listctrl.cpp b/src/generic/listctrl.cpp
index a2366dce73..f7eb3f45b4 100644
--- a/src/generic/listctrl.cpp
+++ b/src/generic/listctrl.cpp
@@ -5108,6 +5108,7 @@ void wxGenericListCtrl::ShowSortIndicator(int idx, bool ascending)
                 (idx != m_headerWin->m_sortCol ||
                  ascending != m_headerWin->m_sortAsc) )
     {
+        m_headerWin->m_enableSortCol = true;
         m_headerWin->m_sortCol = idx;
         m_headerWin->m_sortAsc = ascending;
         m_headerWin->Refresh();
diff --git a/src/msw/listctrl.cpp b/src/msw/listctrl.cpp
index 283b8b6b5b..6b7be675f2 100644
--- a/src/msw/listctrl.cpp
+++ b/src/msw/listctrl.cpp
@@ -1459,6 +1459,13 @@ bool wxListCtrl::IsItemChecked(long item) const
 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();
 }
 
@@ -1477,7 +1484,11 @@ void wxListCtrl::ShowSortIndicator(int idx, bool ascending)
     {
         m_sortCol = idx;
         m_sortAsc = ascending;
-        DrawSortArrow();
+
+        // 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();
     }
 }
 
@@ -1485,7 +1496,9 @@ void wxListCtrl::RemoveSortIndicator()
 {
     m_sortCol = -1;
     m_sortAsc = true;
-    DrawSortArrow();
+
+    if ( IsSortIndicatorEnabled() )
+        DrawSortArrow();
 }
 
 int wxListCtrl::GetSortIndicator() const
@@ -2091,7 +2104,8 @@ long wxListCtrl::DoInsertColumn(long col, const wxListItem& item)
         SetColumnWidth(n, wxLIST_AUTOSIZE_USEHEADER);
     }
 
-    DrawSortArrow();
+    if ( IsSortIndicatorEnabled() )
+        DrawSortArrow();
 
     return n;
 }
@@ -2472,7 +2486,8 @@ bool wxListCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result)
                 else
                     m_sortAsc = !m_sortAsc;
                 m_sortCol = nmLV->iSubItem;
-                DrawSortArrow();
+                if ( IsSortIndicatorEnabled() )
+                    DrawSortArrow();
 
                 eventType = wxEVT_LIST_COL_CLICK;
                 event.m_itemIndex = -1;
@@ -3432,7 +3447,7 @@ void wxListCtrl::DrawSortArrow()
     {
         if ( ListView_GetColumn(GetHwnd(), col, &lvCol) )
         {
-            if ( m_enableSortCol && col == m_sortCol )
+            if ( col == m_sortCol )
             {
                 if ( m_sortAsc )
                     lvCol.fmt = (lvCol.fmt & ~HDF_SORTDOWN) | HDF_SORTUP;