Re: [patch] Fix setup.exe chooser page header column borkage.

[Dave Korn <dave dot korn dot cygwin at googlemail dot com>]

Christopher Faylor wrote:
> On Mon, Jun 29, 2009 at 04:40:08PM +0200, Corinna Vinschen wrote:
>> On Jun 29 10:26, Ralph Hempel wrote:
>>> Christopher Faylor wrote:
>>>
>>>> Remember that setup won the obfuscated c++ code contest in 2002.  We're
>>>> very proud of that.
>>> I thought you had to submit code that was intentionally obfuscated. Code
>>> that just got all crufty by itself doesn't normally qualify.
>> Huh?  I always thought that setup *is* intentionally obfuscated.
>> Did I miss something?
> 
> I think you're both right.  There was intentional obfuscation in the
> original design but it was improved upon by the handful of setup
> developers (I among them) over time.

[  Heh.  Legacy code FTW.  To me it looks like some of the code was developed
in parallel with some of the developers learning the ins and outs of writing
big C++ applications for the first time.  That's only to be expected; there's
a lot of stuff about a language that you just don't learn until you really use
it in anger for a while.  Anyway, it is what it is and it's not _that_ bad.  ]

  I found the problem.  PickView::init_headers() recalculates the width of all
the header columns, but it takes no account of the cumulative WM_SIZE width
deltas that have been applied to the final column to keep the header bar at
the full width of the chooser panel, so loses them, and when we later minimise
they get subtracted out from a width value that never had them added in in the
first place.  Oops.

  I think simply keeping track of the cumulative deltas applied in response to
WM_SIZE messages and adding them back in each time we recalculate the
headerbar layout is the simplest solution.  (If reviewing the patch out of
context, the comment in PickView::init_headers might look like it's missing
capitalisation, but it's actually part of a run-on sentence spread across
several comments.)

	* PickView.h (PickView::total_delta_x):  New int member.
	(PickView::set_header_column_order):  Add prototype.
	* PickView.cc (PickView::set_header_column_order):  New function,
	broken out from ...
	(PickView::set_headers):  ... here.  Call it.
	(PickView::init_headers):  Apply total_delta_x to last_col width.
	(PickView::PickView):  Initialise new total_delta_x member to zero.
	(PickView::WindowProc):  Use set_header_column_order to find and
	adjust final column for both sets of headers.

  Ok?

    cheers,
      DaveK

Index: PickView.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/PickView.cc,v
retrieving revision 2.34
diff -p -u -r2.34 PickView.cc
--- PickView.cc	26 Jun 2009 15:09:07 -0000	2.34
+++ PickView.cc	30 Jun 2009 16:57:47 -0000
@@ -86,15 +86,13 @@ DoInsertItem (HWND hwndHeader, int iInse
   return index;
 }
 
-void
-PickView::set_headers ()
+int
+PickView::set_header_column_order (views vm)
 {
-  if (view_mode == views::Unknown)
-    return;
-  if (view_mode == views::PackageFull ||
-      view_mode == views::Package ||
-      view_mode == views::PackageKeeps ||
-      view_mode == views::PackageSkips)
+  if (vm == views::Unknown)
+    return -1;
+  else if (vm == views::PackageFull || vm == views::Package
+      || vm == views::PackageKeeps || vm == views::PackageSkips)
     {
       headers = pkg_headers;
       current_col = 0;
@@ -106,19 +104,27 @@ PickView::set_headers ()
       pkg_col = size_col + 1;
       last_col = pkg_col;
     }
-  else if (view_mode == views::Category)
+  else if (vm == views::Category)
     {
       headers = cat_headers;
+      cat_col = 0;
       current_col = 1;
       new_col = current_col + 1;
       bintick_col = new_col + 1;
       srctick_col = bintick_col + 1;
-      cat_col = 0;
       size_col = srctick_col + 1;
       pkg_col = size_col + 1;
       last_col = pkg_col;
     }
   else
+    return -1;
+  return last_col;
+}
+
+void
+PickView::set_headers ()
+{
+  if (set_header_column_order (view_mode) == -1)
     return;
   while (int n = SendMessage (listheader, HDM_GETITEMCOUNT, 0, 0))
     {
@@ -513,12 +519,15 @@ PickView::init_headers (HDC dc)
   headers[0].x = 0;
   for (i = 1; i <= last_col; i++)
     headers[i].x = headers[i - 1].x + headers[i - 1].width;
+  // and allow for resizing to ensure the last column reaches
+  // all the way to the end of the chooser box.
+  headers[last_col].width += total_delta_x;
 }
 
 
 PickView::PickView (Category &cat) : deftrust (TRUST_UNKNOWN),
 contents (*this, cat, 0, false, true), showObsolete (false), 
-packageFilterString (), hasClientRect (false)
+packageFilterString (), hasClientRect (false), total_delta_x (0)
 {
 }
 
@@ -810,11 +819,14 @@ PickView::WindowProc (UINT message, WPAR
             if ((dx = clientRect.right - clientRect.left -
                         lastClientRect.width ()) != 0)
               {
-                headers[last_col].width += dx;
+                cat_headers[set_header_column_order (views::Category)].width += dx;
+                pkg_headers[set_header_column_order (views::Package)].width += dx;
+                set_header_column_order (view_mode);
                 set_headers ();
                 ::MoveWindow (listheader, -scroll_ulc_x, 0,
                             headers[last_col].x +
                             headers[last_col].width, header_height, TRUE);
+		total_delta_x += dx;
               }
           }
         else
Index: PickView.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/PickView.h,v
retrieving revision 2.18
diff -p -u -r2.18 PickView.h
--- PickView.h	24 Apr 2009 21:04:13 -0000	2.18
+++ PickView.h	30 Jun 2009 16:57:47 -0000
@@ -148,7 +148,9 @@ private:
   // Stuff needed to handle resizing
   bool hasClientRect;
   RECTWrapper lastClientRect;
-  
+  int total_delta_x;
+
+  int set_header_column_order (views vm);
   void set_headers ();
   void init_headers (HDC dc);
   void note_width (Header *hdrs, HDC dc, const std::string& string,