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]

User URLs in setup.exe


A few things are currently (2.249.2.4) interacting to make setup's
user interface for adding "User URLs" extremely susceptible to typos.

Suppose you have a bunch of packages living outside the usual set of
Cygwin mirrors, so you tell your users to add (random example :-))

	http://prc-tools.sourceforge.net/install

to their list of mirror sites (i.e. on setup's site.cc page).  Then

1. It gets listed as just "http://prc-tools.sourceforge.net"; so they
   can't tell if they actually typed it all in correctly.  While I think
   hiding the details of the URL is good for the ones in mirrors.lst,
   I'm not sure it's right for ones the users type in themselves.

2. The URL's unique key in all_site_list is (derived from) the
   abbreviated one that's displayed, not the full URL.
   
3. If you type in another URL with the same key as one that's already in
   the list (say you're trying to fix a suspected (and invisible! :-))
   typo), your new URL will get discarded (!) instead of overwriting the
   old one or appearing beside it (SitePage::OnMessageCmd, site.cc:418).

Example:  Add "http://example.com/foo"; as a User URL.  Now try to add
"http://example.com/bar";.  To get the change to stick you have to either
manually delete /etc/setup/last-mirror and rerun setup, or ensure that
"http://example.com"; is not selected and quit setup (thus getting it out
of last-mirror) and rerun setup.  Both of these are difficult to explain
to clueless newbies, especially ones who don't believe that they could
possibly have made a typo.

Dumb user #1 example: user types in
"http://prc-tools.sourceforge.net/install/";, which sadly doesn't work.
They get this error message:

 Unable to retrieve setup.ini from http://prc-tools.sourceforge.net/install/

so with luck they can see that they need to retype it.  So they try
again with "http://prc-tools.sourceforge.net/install"; and check it
*really carefully* before they press Add.  Unfortunately they can check
it as much as they like; setup will ignore the change and they'll still
get the same error.

Dumb user #2 example: it turns out that instead of typing in the URL
above, many of the users cut&paste it from the instructions Web page,
which until today would often give them a trailing space.  This would
lead to an error message that looks like this:

 Unable to retrieve setup.ini from http://prc-tools.sourceforge.net/install

and they *really* couldn't tell what they'd done wrong!  And similarly
trying to fix the invisible typo wasn't actually doing anything.


I'm not sure what the right fix is.  I think probably #2 above is right
(because if http://example.com appears twice in the list but the full
URLs behind them are different... that's confusing), but #1 and #3 are
wrong.  I suspect displayed_url for an URL that you type in yourself
should be the full thing you typed, so that you can see it.  (And then
#3 would no longer be an issue.)

If others agree (and want me to back my words with actions :-)), I'll
send a patch to do this.


There is also the question of whether setup User URLs should be
susceptible to trailing slashes and spaces.  Here's a (ugly and
untested) patch that strips trailing spaces:

Index: site.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/site.cc,v
retrieving revision 2.18
diff -u -p -r2.18 site.cc
--- site.cc	6 May 2002 14:40:41 -0000	2.18
+++ site.cc	26 Jun 2002 13:54:51 -0000
@@ -25,6 +25,7 @@ static const char *cvsid =
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <ctype.h>
 #include <process.h>
 
 #include "dialog.h"
@@ -50,7 +51,13 @@ list < site_list_type, String, String::c
 void
 site_list_type::init (String const &newurl)
 {
-  url = newurl;
+  char *spaces = newurl.cstr();
+  char *space = spaces + strlen (spaces);
+  while (space > spaces && isspace (space[-1]))
+    --space;
+  *space = '\0';
+  url = String (spaces);
+  delete[] spaces;
 
   char *dots = newurl.cstr();
   char *dot = strchr (dots, '.');

It might also be worth stripping trailing '/' characters; if some mirror
really needed to end in a space, you could presumably use %20 notation.
Comments?  Me, I'm not particularly comfortable about changing what the
user typed in, but...


If that turned out not to be a good idea, it might be worth making the
spaces visible in the error message, e.g. like this:

Index: res.rc
===================================================================
RCS file: /cvs/cygwin-apps/setup/res.rc,v
retrieving revision 2.40
diff -u -p -r2.40 res.rc
--- res.rc	12 May 2002 11:28:22 -0000	2.40
+++ res.rc	26 Jun 2002 13:54:51 -0000
@@ -439,7 +439,7 @@ BEGIN
     IDS_CYGWIN_FUNC_MISSING "Error: unable to find function `%s' in %s"
     IDS_DOWNLOAD_SHORT      "Download error: %s too short (%d, wanted %d)"
     IDS_ERR_OPEN_WRITE      "Can't open %s for writing: %s"
-    IDS_SETUPINI_MISSING    "Unable to get setup.ini from %s"
+    IDS_SETUPINI_MISSING    "Unable to get setup.ini from <%s>"
     IDS_OLD_SETUPINI        "This setup.ini is older than the one you used last time you installed cygwin.  Proceed anyway?"
     IDS_ERR_RENAME          "Can't rename %s to %s: %s"
     IDS_NOTHING_INSTALLED   "Nothing needed to be installed"
Index: site.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/site.cc,v
retrieving revision 2.18
diff -u -p -r2.18 site.cc
--- site.cc	6 May 2002 14:40:41 -0000	2.18
+++ site.cc	26 Jun 2002 13:54:51 -0000
@@ -415,7 +422,7 @@ bool SitePage::OnMessageCmd (int id, HWN
 	    else
 	      {
 		// Log the adding of this new URL.
-		log (LOG_BABBLE, String ("Adding site: ") + other_url);
+		log (LOG_BABBLE, String ("Adding site: <") + other_url + String (">"));
 	      }
 
 	    // Assume the user wants to use it and select it for him.

    John


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