[PATCH setup 0/4] Distinguish between user URLs and cygwin mirrors in UI

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH setup 0/4] Distinguish between user URLs and cygwin mirrors in UI

Ken Brown-6
This is a followup to

  https://cygwin.com/ml/cygwin-apps/2011-04/msg00014.html .

In that message Jon suggested splitting site selection into two pages,
one for cygwin.com mirrors and one for additional user URLs.  The
latter would be visible only if the user had previously checked a
suitable checkbox.

I attempted to implement this but found it to be too complicated.
This patch series implements a variant of that suggestion.  There is
still only one site selection page, but it now has a checkbox "Allow
sites that are not cygwin.com mirrors".  The box is initially
unchecked unless the "last-mirror" user setting in setup.rc contains a
non-mirror.

The button for adding a user URL is disabled unless the box is
checked.

Ken Brown (4):
  Add checkbox IDC_ALLOW_USER_URL to IDD_SITE dialog
  Set the initial state of IDC_ALLOW_USER_URL
  Adjust site list if IDC_ALLOW_USER_URL is unchecked
  Display full URL of non-mirrors in the site list

 res.rc     | 15 +++++-----
 resource.h |  1 +
 site.cc    | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 site.h     |  1 +
 4 files changed, 103 insertions(+), 7 deletions(-)

--
2.15.1

Reply | Threaded
Open this post in threaded view
|

[PATCH setup 1/4] Add checkbox IDC_ALLOW_USER_URL to IDD_SITE dialog

Ken Brown-6
Enable the "User URL" edit box and "Add" button only if
IDC_ALLOW_USER_URL is checked.
---
 res.rc     | 15 ++++++++-------
 resource.h |  1 +
 site.cc    | 14 ++++++++++++++
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/res.rc b/res.rc
index a4d7e70..b5dcb3e 100644
--- a/res.rc
+++ b/res.rc
@@ -135,21 +135,22 @@ CAPTION "Cygwin Setup - Choose Download Site(s)"
 FONT 8, "MS Shell Dlg"
 BEGIN
     ICON            IDI_CYGWIN,IDC_HEADICON,SETUP_HEADICON_X,0,21,20
-    LISTBOX         IDC_URL_LIST,66,45,185,110,LBS_NOINTEGRALHEIGHT |
+    LISTBOX         IDC_URL_LIST,66,34,185,110,LBS_NOINTEGRALHEIGHT |
                     LBS_EXTENDEDSEL | WS_VSCROLL | WS_HSCROLL | WS_GROUP |
                     WS_TABSTOP
-    LTEXT           "Available Download Sites:",IDC_STATIC,66,34,183,8,NOT
-                    WS_GROUP
     CONTROL         "",IDC_HEADSEPARATOR,"Static",SS_BLACKFRAME | SS_SUNKEN,0,28,
                     SETUP_STANDARD_DIALOG_W,1
     LTEXT           "Choose a site from this list, or add your own sites to the list",
                     IDC_STATIC,21,9,239,16,NOT WS_GROUP
     LTEXT           "Choose A Download Site",IDC_STATIC_HEADER_TITLE,7,0,258,
                     8,NOT WS_GROUP
-    EDITTEXT        IDC_EDIT_USER_URL,65,160,185,14,ES_AUTOHSCROLL |
-                    WS_GROUP
-    LTEXT           "User URL:",IDC_SITE_USERURL,15,162,45,8,NOT WS_GROUP
-    PUSHBUTTON      "Add",IDC_BUTTON_ADD_URL,255,160,50,14
+    EDITTEXT        IDC_EDIT_USER_URL,65,165,185,14,ES_AUTOHSCROLL |
+                    WS_GROUP | WS_DISABLED
+    LTEXT           "User URL:",IDC_SITE_USERURL,15,167,45,8,NOT WS_GROUP |
+                    WS_DISABLED
+    PUSHBUTTON      "Add",IDC_BUTTON_ADD_URL,255,165,50,14,WS_DISABLED
+    CONTROL         "Allow sites that are not cygwin.com mirrors",
+                    IDC_ALLOW_USER_URL,"Button",BS_AUTOCHECKBOX,15,150,200,8
 END
 
 IDD_NET DIALOG DISCARDABLE  0, 0, SETUP_STANDARD_DIALOG_DIMS
diff --git a/resource.h b/resource.h
index a2e867f..d111853 100644
--- a/resource.h
+++ b/resource.h
@@ -177,3 +177,4 @@
 #define IDC_FILE_INUSE_HELP               592
 #define IDC_NET_DIRECT_LEGACY             593
 #define IDC_DOWNLOAD_EDIT                 594
+#define IDC_ALLOW_USER_URL                595
diff --git a/site.cc b/site.cc
index 230d82c..4fbf30d 100644
--- a/site.cc
+++ b/site.cc
@@ -78,6 +78,7 @@ SitePage::SitePage ()
 
 using namespace std;
 
+bool allow_user_url;
 bool cache_is_usable;
 bool cache_needs_writing;
 string cache_warn_urls;
@@ -637,6 +638,10 @@ SitePage::CheckControlsAndDisableAccordingly () const
       ButtonFlags |= PSWIZB_NEXT;
     }
   GetOwner ()->SetButtons (ButtonFlags);
+
+  EnableWindow (GetDlgItem (IDC_EDIT_USER_URL), allow_user_url);
+  EnableWindow (GetDlgItem (IDC_SITE_USERURL), allow_user_url);
+  EnableWindow (GetDlgItem (IDC_BUTTON_ADD_URL), allow_user_url);
 }
 
 void
@@ -720,6 +725,15 @@ bool SitePage::OnMessageCmd (int id, HWND hwndctl, UINT code)
   }
  break;
       }
+    case IDC_ALLOW_USER_URL:
+      {
+ if (code == BN_CLICKED)
+  {
+    allow_user_url = IsButtonChecked (IDC_ALLOW_USER_URL);
+    CheckControlsAndDisableAccordingly ();
+  }
+ break;
+      }
     default:
       // Wasn't recognized or handled.
       return false;
--
2.15.1

Reply | Threaded
Open this post in threaded view
|

[PATCH setup 2/4] Set the initial state of IDC_ALLOW_USER_URL

Ken Brown-6
In reply to this post by Ken Brown-6
It starts checked if all_site_list contains a site that is not a
cygwin.com mirror.  Such a site would have to come from the
"last-mirror" user setting, indicating that the user used a non-mirror
on the last run.  Introduce a new function check_for_user_urls() to
help with this.
---
 site.cc | 26 ++++++++++++++++++++++++++
 site.h  |  1 +
 2 files changed, 27 insertions(+)

diff --git a/site.cc b/site.cc
index 4fbf30d..dbb07d4 100644
--- a/site.cc
+++ b/site.cc
@@ -241,6 +241,23 @@ save_dialog (HWND h)
     }
 }
 
+// Does the_site_list contain any sites that are not mirrors (possibly stale)?
+static void
+check_for_user_urls ()
+{
+  for (SiteList::const_iterator i = all_site_list.begin ();
+       i != all_site_list.end (); i++)
+    {
+      if (!i->from_mirrors_lst
+  && (find (cached_site_list.begin (), cached_site_list.end (), *i)
+      == cached_site_list.end ()))
+ {
+  allow_user_url = true;
+  return;
+ }
+    }
+}
+
 // This is called only for lists of mirrors that came (now or in a
 // previous setup run) from mirrors.lst.
 void
@@ -365,6 +382,8 @@ get_site_list (HINSTANCE h, HWND owner)
   delete[] theMirrorString;
   delete[] theCachedString;
 
+  check_for_user_urls ();
+
   return 0;
 }
 
@@ -568,6 +587,13 @@ bool SitePage::Create ()
   return PropertyPage::Create (IDD_SITE);
 }
 
+void
+SitePage::OnInit ()
+{
+  int a = allow_user_url ? BST_CHECKED : BST_UNCHECKED;
+  CheckDlgButton (GetHWND (), IDC_ALLOW_USER_URL, a);
+}
+
 long
 SitePage::OnNext ()
 {
diff --git a/site.h b/site.h
index d16db8e..c517ac2 100644
--- a/site.h
+++ b/site.h
@@ -31,6 +31,7 @@ public:
 
   bool Create ();
 
+  virtual void OnInit ();
   virtual void OnActivate ();
   virtual long OnNext ();
   virtual long OnBack ();
--
2.15.1

Reply | Threaded
Open this post in threaded view
|

[PATCH setup 3/4] Adjust site list if IDC_ALLOW_USER_URL is unchecked

Ken Brown-6
In reply to this post by Ken Brown-6
If the user unchecks the box, remove all sites except for fresh
mirrors and selected stale mirrors.  Introduce a new function
remove_user_urls() to help with this
---
 site.cc | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/site.cc b/site.cc
index dbb07d4..d4550a1 100644
--- a/site.cc
+++ b/site.cc
@@ -258,6 +258,35 @@ check_for_user_urls ()
     }
 }
 
+static void
+remove_user_urls ()
+{
+  SiteList result;
+  for (SiteList::const_iterator i = all_site_list.begin ();
+       i != all_site_list.end (); i++)
+    {
+      if (i->from_mirrors_lst)
+ // It's a fresh mirror.
+ result.push_back (*i);
+      else
+ {
+  // Is it selected?
+  SiteList::iterator j = find (site_list.begin (),
+       site_list.end (), *i);
+  if (j != site_list.end ())
+    {
+      if (find (cached_site_list.begin (), cached_site_list.end (), *i)
+  != cached_site_list.end ())
+ // It's a selected stale mirror.
+ result.push_back (*i);
+      else
+ site_list.erase (j);
+    }
+ }
+    }
+  all_site_list = result;
+}
+
 // This is called only for lists of mirrors that came (now or in a
 // previous setup run) from mirrors.lst.
 void
@@ -756,6 +785,12 @@ bool SitePage::OnMessageCmd (int id, HWND hwndctl, UINT code)
  if (code == BN_CLICKED)
   {
     allow_user_url = IsButtonChecked (IDC_ALLOW_USER_URL);
+    if (!allow_user_url)
+      {
+ // The button was just unchecked.
+ remove_user_urls ();
+ PopulateListBox ();
+      }
     CheckControlsAndDisableAccordingly ();
   }
  break;
--
2.15.1

Reply | Threaded
Open this post in threaded view
|

[PATCH setup 4/4] Display full URL of non-mirrors in the site list

Ken Brown-6
In reply to this post by Ken Brown-6
Set displayed_url to full URL in the site_list_type constructor if
from_mirrors_lst is false.  Modify check_for_user_urls() to change
displayed_url back to the short form if a site in all_site_list turns
out to be a stale mirror.
---
 site.cc | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/site.cc b/site.cc
index d4550a1..ef0c218 100644
--- a/site.cc
+++ b/site.cc
@@ -182,6 +182,12 @@ site_list_type::site_list_type (const string &_url,
       idx = 0;
   } while (idx > 0);
   key += url;
+  /* Display the full URL (without trailing slash) of a non-mirror.  */
+  if (!from_mirrors_lst)
+    {
+      displayed_url = url;
+      displayed_url.erase (displayed_url.end () - 1);
+    }
 }
 
 site_list_type::site_list_type (site_list_type const &rhs)
@@ -241,19 +247,31 @@ save_dialog (HWND h)
     }
 }
 
-// Does the_site_list contain any sites that are not mirrors (possibly stale)?
+// Set allow_user_url = true if all_site_list contains any sites that
+// are not mirrors (possibly stale).  At the same time, fixup the
+// stored information about stale mirrors.
 static void
 check_for_user_urls ()
 {
-  for (SiteList::const_iterator i = all_site_list.begin ();
+  for (SiteList::iterator i = all_site_list.begin ();
        i != all_site_list.end (); i++)
     {
-      if (!i->from_mirrors_lst
-  && (find (cached_site_list.begin (), cached_site_list.end (), *i)
-      == cached_site_list.end ()))
+      if (!i->from_mirrors_lst)
  {
-  allow_user_url = true;
-  return;
+  // Is it a stale mirror?
+  SiteList::iterator j = find (cached_site_list.begin (),
+       cached_site_list.end (), *i);
+  if (j == cached_site_list.end ())
+    allow_user_url = true;
+  else
+    {
+      i->servername = j->servername;
+      i->area = j->area;
+      i->location = j->location;
+      // i->from_mirrors_lst stays false.
+      i->displayed_url = j->displayed_url;
+      i->key = j->key;
+    }
  }
     }
 }
--
2.15.1

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH setup 0/4] Distinguish between user URLs and cygwin mirrors in UI

Brian Inglis
In reply to this post by Ken Brown-6
On 2017-12-03 08:22, Ken Brown wrote:

> This is a followup to
>
>   https://cygwin.com/ml/cygwin-apps/2011-04/msg00014.html .
>
> In that message Jon suggested splitting site selection into two pages,
> one for cygwin.com mirrors and one for additional user URLs.  The
> latter would be visible only if the user had previously checked a
> suitable checkbox.
>
> I attempted to implement this but found it to be too complicated.
> This patch series implements a variant of that suggestion.  There is
> still only one site selection page, but it now has a checkbox "Allow
> sites that are not cygwin.com mirrors".  The box is initially
> unchecked unless the "last-mirror" user setting in setup.rc contains a
> non-mirror.

Will this interfere with running setup -gM using an "unofficial" last-mirror
to skip to the package upgrade/selection page?

There are a bunch of distro mirrors that are not "official"/listed publicly:
many unis and local UUGs mirror distros on publicly accessible hosts, that are
not on distros' mirror lists, probably because they don't want any extra admin,
support, or network load.

--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH setup 0/4] Distinguish between user URLs and cygwin mirrors in UI

Ken Brown-6
On 12/3/2017 1:06 PM, Brian Inglis wrote:

> On 2017-12-03 08:22, Ken Brown wrote:
>> This is a followup to
>>
>>    https://cygwin.com/ml/cygwin-apps/2011-04/msg00014.html .
>>
>> In that message Jon suggested splitting site selection into two pages,
>> one for cygwin.com mirrors and one for additional user URLs.  The
>> latter would be visible only if the user had previously checked a
>> suitable checkbox.
>>
>> I attempted to implement this but found it to be too complicated.
>> This patch series implements a variant of that suggestion.  There is
>> still only one site selection page, but it now has a checkbox "Allow
>> sites that are not cygwin.com mirrors".  The box is initially
>> unchecked unless the "last-mirror" user setting in setup.rc contains a
>> non-mirror.
>
> Will this interfere with running setup -gM using an "unofficial" last-mirror
> to skip to the package upgrade/selection page?

No, it doesn't seem to.

Ken

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH setup 1/4] Add checkbox IDC_ALLOW_USER_URL to IDD_SITE dialog

Ken Brown-6
In reply to this post by Ken Brown-6
On 12/3/2017 10:22 AM, Ken Brown wrote:
> Enable the "User URL" edit box and "Add" button only if
> IDC_ALLOW_USER_URL is checked.
> ---
>   res.rc     | 15 ++++++++-------
>   resource.h |  1 +
>   site.cc    | 14 ++++++++++++++
>   3 files changed, 23 insertions(+), 7 deletions(-)

Sorry, I need to add the following hunk:

diff --git a/site.cc b/site.cc
index ef0c218..9972a29 100644
--- a/site.cc
+++ b/site.cc
@@ -64,6 +64,7 @@ static ControlAdjuster::ControlInfo SiteControlsInfo[] = {
    {IDC_EDIT_USER_URL,          CP_STRETCH, CP_BOTTOM},
    {IDC_BUTTON_ADD_URL,         CP_RIGHT,   CP_BOTTOM},
    {IDC_SITE_USERURL,            CP_LEFT,    CP_BOTTOM},
+  {IDC_ALLOW_USER_URL,          CP_LEFT,    CP_BOTTOM},
    {0, CP_LEFT, CP_TOP}
  };

Ken