This is the mail archive of the cygwin-apps@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [setup PATCH] SetDlgItemFont


Robert Collins wrote:
> On Thu, 2003-07-31 at 22:20, Max Bowsher wrote:
>> ChangeLog says it all really - this is just an incremental tweak to make
it
>> easier to create a "Finished" page, and to have all our font stylings in
one
>> place.
>>
>> Note Gary had the SetDlgItemFont calls moved out in a seperate method.
>> Given that they are almost the entire contents of the "case
WM_INITDIALOG",
>> I don't think this is appropriate.
>>
>> Max.
>
> Well, factoring out code for clarity is a good thing - the separate
> function is appropriate.
>
> Can you resubmit with that..

OK, but I actually believe that a seperate function makes things less clear
in this case, so I'm going to attempt to convince you to go with my original
submission.

Here is the block of code with the changes made:
============================================================================
=
  switch (message)
    {
    case WM_INITDIALOG:
      {
        OnInit ();

        // These font settings will just silently fail when the resource id
        // is not present on a page.
        // Set header title font of each internal page
        SetDlgItemFont(IDC_STATIC_HEADER_TITLE, "MS Shell Dlg", 8, FW_BOLD);
        // Set the font for the IDC_STATIC_WELCOME_TITLE
        SetDlgItemFont(IDC_STATIC_WELCOME_TITLE, "Arial", 12, FW_BOLD);

        // TRUE = Set focus to default control (in wParam).
        return TRUE;
        break;
      }
    case WM_NOTIFY:
      switch (((NMHDR FAR *) lParam)->code)
        {
============================================================================
=

Now, I don't think moving 2 lines of code and 4 lines of comments into a
seperate function makes this clearer - rather, it obfuscates what is
happening here.

In case you are not convinced, here is the alternate patch, to avoid another
round-trip of emails:
I'm not particularly fond of the name "SetFontPolicy". Better names
welcomed.

+2003-08-02  Gary R. Van Sickle  <g.r.vansickle@worldnet.att.net>
+
+ Changes modified by Max Bowsher  <maxb@ukf.net>
+ * splash.cc (Copyright): Update copyright dates.
+ (SplashPage::OnInit): Remove call to SetDlgItemFont().  Now handled in
+ base class.
+ * proppage.h (PropertyPage::SetFontPolicy): Declare.
+ * proppage.cc (Copyright): Update copyright dates.
+ (PropertyPage::DialogProc WM_INITDIALOG): Move all font setting code,
+ including that from splash.cc into new function...
+ (PropertyPage::SetFontPolicy): Create, using moved code. Change font
+ "MS Sans Serif" to "MS Shell Dlg" in line with recent res.rc change.
+ Set font for IDC_STATIC_WELCOME_TITLE here, to allow easy re-use of
+ style for future "Finished" page.

Index: proppage.cc
===================================================================
RCS file: /home/max/cvsmirror/cygwin-apps-cvs/setup/proppage.cc,v
retrieving revision 2.8
diff -u -p -r2.8 proppage.cc
--- proppage.cc 1 Aug 2003 22:20:13 -0000 2.8
+++ proppage.cc 1 Aug 2003 23:03:36 -0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, Gary R. Van Sickle.
+ * Copyright (c) 2001, 2002, 2003 Gary R. Van Sickle.
  *
  *     This program is free software; you can redistribute it and/or modify
  *     it under the terms of the GNU General Public License as published by
@@ -116,9 +116,7 @@ PropertyPage::DialogProc (UINT message,
       {
  OnInit ();

- // Set header title font of each internal page to MS Sans Serif, Bold, 8
Pt.
- // This will just silently fail on the first and last pages.
- SetDlgItemFont(IDC_STATIC_HEADER_TITLE, "MS Sans Serif", 8, FW_BOLD);
+ SetFontPolicy ();

  // TRUE = Set focus to default control (in wParam).
  return TRUE;
@@ -269,4 +267,15 @@ PropertyPage::DialogProc (UINT message,

   // Wasn't handled
   return FALSE;
+}
+
+void
+PropertyPage::SetFontPolicy ()
+{
+  // These font settings will just silently fail when the resource id
+  // is not present on a page.
+  // Set header title font of each internal page
+  SetDlgItemFont(IDC_STATIC_HEADER_TITLE, "MS Shell Dlg", 8, FW_BOLD);
+  // Set the font for the IDC_STATIC_WELCOME_TITLE
+  SetDlgItemFont(IDC_STATIC_WELCOME_TITLE, "Arial", 12, FW_BOLD);
 }
Index: proppage.h
===================================================================
RCS file: /home/max/cvsmirror/cygwin-apps-cvs/setup/proppage.h,v
retrieving revision 2.7
diff -u -p -r2.7 proppage.h
--- proppage.h 1 Aug 2003 22:17:04 -0000 2.7
+++ proppage.h 1 Aug 2003 23:05:10 -0000
@@ -45,6 +45,7 @@ class PropertyPage:public Window
        LPARAM lParam);
   static BOOL CALLBACK DialogProcReflector (HWND hwnd, UINT message,
          WPARAM wParam, LPARAM lParam);
+  void SetFontPolicy ();

 protected:
     virtual BOOL CALLBACK DialogProc (UINT message, WPARAM wParam,
Index: splash.cc
===================================================================
RCS file: /home/max/cvsmirror/cygwin-apps-cvs/setup/splash.cc,v
retrieving revision 2.11
diff -u -p -r2.11 splash.cc
--- splash.cc 29 Jul 2003 14:14:06 -0000 2.11
+++ splash.cc 31 Jul 2003 11:49:30 -0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, Gary R. Van Sickle.
+ * Copyright (c) 2001, 2002, 2003 Gary R. Van Sickle.
  *
  *     This program is free software; you can redistribute it and/or modify
  *     it under the terms of the GNU General Public License as published by
@@ -36,7 +36,4 @@ SplashPage::OnInit ()
   ver.Format (IDS_VERSION_INFO, version[0] ? version : "[unknown]");

   ::SetWindowText (GetDlgItem (IDC_VERSION), ver.c_str ());
-
-  // Set the font for the IDC_STATIC_WELCOME_TITLE
-  SetDlgItemFont(IDC_STATIC_WELCOME_TITLE, "Ariel", 12, FW_BOLD);
 }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]