From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 52207 invoked by alias); 20 Nov 2017 22:59:09 -0000 Mailing-List: contact cygwin-apps-help@cygwin.com; run by ezmlm Precedence: bulk Sender: cygwin-apps-owner@cygwin.com List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Mail-Followup-To: cygwin-apps@cygwin.com Received: (qmail 52191 invoked by uid 89); 20 Nov 2017 22:59:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,KB_WAM_FROM_NAME_SINGLEWORD,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=california, California, Area, States X-HELO: limerock04.mail.cornell.edu Received: from limerock04.mail.cornell.edu (HELO limerock04.mail.cornell.edu) (128.84.13.244) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 20 Nov 2017 22:59:05 +0000 X-CornellRouted: This message has been Routed already. Received: from authusersmtp.mail.cornell.edu (granite3.serverfarm.cornell.edu [10.16.197.8]) by limerock04.mail.cornell.edu (8.14.4/8.14.4_cu) with ESMTP id vAKMx3OT010011 for ; Mon, 20 Nov 2017 17:59:03 -0500 Received: from [192.168.0.4] (mta-68-175-129-7.twcny.rr.com [68.175.129.7] (may be forged)) (authenticated bits=0) by authusersmtp.mail.cornell.edu (8.14.4/8.12.10) with ESMTP id vAKMx2qu016564 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT) for ; Mon, 20 Nov 2017 17:59:03 -0500 Subject: Re: [PATCH] make setup mirror list more like web page not just urls To: cygwin-apps@cygwin.com References: <09c0dc5b-f975-42a9-a204-b02edef6a3ad@SystematicSw.ab.ca> <53a8001e-10ae-6318-ba3d-31d0cfa18f14@SystematicSw.ab.ca> From: Ken Brown Message-ID: Date: Mon, 20 Nov 2017 22:59:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <53a8001e-10ae-6318-ba3d-31d0cfa18f14@SystematicSw.ab.ca> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-PMX-Cornell-Gauge: Gauge=XXXXX X-PMX-CORNELL-AUTH-RESULTS: dkim-out=none; X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg00090.txt.bz2 On 11/20/2017 4:30 PM, Brian Inglis wrote: > On 2017-11-17 06:53, Jon Turney wrote: > On 11/17/2017 8:48 AM, Ken Brown wrote: >> On 15/11/2017 21:35, Brian Inglis wrote: > > Thanks for your comments, I understand all your points, and agree with them. > I merged the posts and responses as they addressed the same issues. > I also applied Ken's setup patch and rejigged mine where required. > > KB> Ken Brown > JT> Jon Turney > > KB> I think this is a good idea in principle, but it still needs some work. > > JT> use a grid control to present this information. > > Don't think I want to learn that much about Windows controls! > The width of the longest fields in each column would need to adjust the grid > and dialogue box widths, and Add button position, dynamically, which I did > manually, and the internal blank space may be a bit harder to scan for users > who are interested only in their geographic region, country, or territory > - these are pretty well aligned as strings. > > KB> Adding the protocol to the key instead of the full url isn't enough to > KB> guarantee uniqueness of the key. > KB> Users can add their own sites for which that fails. > KB> Also, you shouldn't be relying on the servername field being filled in. > KB> For example, the function SiteSetting::registerSavedSite() explicitly > KB> constructs a site 'tempSite' that has an empty servername. > > JT> This code needs to handle manually added URLs with non-FQDNs. > JT> - - url...'s going to need fixing. > > If host name provided, use as servername, or default to localhost. > > If at least two domain components: > if CC TLD, use as area in uppercase, or > if gTLD, use as area with init cap; > if 2-3 char TLD and 2-3 char 2nd level domain, may be subTLD, so if 3rd level > domain exists and does not start with (or contain?) "cygwin", "ftp", "mirror", > "www", add space plus 2-3 char 2nd level domain with init cap to area. > Use next level domain as location with init cap and uppercase after any "-". > Otherwise default area to Local, and use servername as location with init cap. > This adhoc approach seems to work well enough and avoids having to drag in and > process the rules in the Public Suffix List (libpsl5 and > publicsuffix-list-dafsa). > > Examples: > > Input -> > List > Displayed > Key > > http://mirror.cpsc.ucalgary.ca/mirror/cygwin.com -> > http://mirror.cpsc.ucalgary.ca/mirror/cygwin.com/;mirror.cpsc.ucalgary.ca;CA;Ucalgary > CA - Ucalgary - http://mirror.cpsc.ucalgary.ca/mirror/cygwin.com > CA Ucalgary ca ucalgary cpsc mirror > http://mirror.cpsc.ucalgary.ca/mirror/cygwin.com/ > > [cyg]file://server/c$/usr/local/cygwin/var/cache/setup/packages -> > [cyg]file://server/c$/usr/local/cygwin/var/cache/setup/packages/;server;Local;Server > Local - Server - file://server/c$/usr/local/cygwin/var/cache/setup/packages > Local Server server file://server/c$/usr/local/cygwin/var/cache/setup/packages/ > > cygfile:///var/cache/setup/packages -> > cygfile:///var/cache/setup/packages/;localhost;Local;Localhost > Local - Localhost - cygfile://var/cache/setup/packages > Local Localhost localhost cygfile://var/cache/setup/packages/ > > file:///c|/usr/local/cygwin/var/cache/setup/packages -> > file:///c|/usr/local/cygwin/var/cache/setup/packages/;localhost;Local;Localhost > Local - Localhost - file:///c|/usr/local/cygwin/var/cache/setup/packages > Local Localhost localhost file:///c|/usr/local/cygwin/var/cache/setup/packages/ > > Cygwin file:// does not appear to use /// or support anything but local files, > so should always be treated as localhost; or does it support servers, but they > can be treated as if directories under Windows, so we need to do a server or > directory check, or that check is done at a lower level? > > JT> you can also just add a path. > > If UNC path with server, use as above, but include share name in display, > otherwise default as above. > > Examples: > > \\server\c$\usr\local\cygwin\var\cache\setup\packages -> > \\server\c$\usr\local\cygwin\var\cache\setup\packages/;server;Local;Server > Local - Server - \\server\c$ > Local Server server \\server\c$\usr\local\cygwin\var\cache\setup\packages/ > > c:\usr\local\cygwin\var\cache\setup\packages -> > c:\usr\local\cygwin\var\cache\setup\packages/;localhost;Local;Localhost > Local - Localhost - c:\usr\local\cygwin\var\cache\setup\packages > Local Localhost localhost c:\usr\local\cygwin\var\cache\setup\packages/ > > Where are the cached sites stashed - setup keeps showing my old test entries? > > JT> if you add two URLs with the same protocol/host, they aren't distinguishable > > Check for and eliminate duplicates after sort if same site_list fields. > Handled by KB patch. > Note on flag added by that patch - would is_current, is_good, or is_known be a > better reflection of what that flag means? > Should we add ==(string) to compare only the url member? > Should we add a constructor(string,is_official) to refactor string to site > conversions, and the url additions? > > KB> If the saved url corresponds to a site in the list, I think setup should > KB> recognize this rather than listing the same site again as " - - url". > > JT> If the URL matches one in the list, that item should be selected. > > Check for URL match with site list and use match as site: > added for tempSite in RegisterSavedSites and newsite on Add; > required as key now includes area and location, not just URL. > Could we add a check to a string contructor to return only a reference to an > existing entry, if the url matches, to refactor the above changes? > > JT> Store area and location so we can populate those fields if it doesn't. > > See above. > > JT> Please, please use git-format-patch in future. > > I can't tell from looking, does it differ from diff output, and is there a > reason to use git-format-patch for review? > I use diffs for feedback until it DTRT. > I can manage one commit and one format-patch on a good day. > I should just stick to using what Tortoise Git supports. > My record with git includes a lot of rm -rf and re-clone, as the files, trees, > and indices get out of whack when I forget some incantation and try to fix it! > Very brittle and unfriendly - if it's going to bork itself, just don't do that! > I never had a single problem with CVS or Hg in many years ;^> > > JT> The patch also has some spurious changes to the mode of the manifest files. > > Realized I shouldn't tidy up unnecessary x modes under git control - habit > - gets me into trouble and I learn new things - checked out masters. Hi Brian, The issue of recognizing a URL that's already in the list is not fixed. Here's what I tried: I ran setup with the last mirror (as stored in /etc/setup/setup.rc) being http://mirrors.kernel.org/sourceware/cygwin/. The resulting mirror list display contained the following, highlighted: Org - Kernel - http://mirrors.kernel.org So setup didn't recognize that http://mirrors.kernel.org/sourceware/cygwin/ was already in the list, displayed as United States - California - http://mirrors.kernel.org Also, I don't like the fake 'area' and 'location' fields for user-added URLs. For example, "Org" and "Kernel" are startling in the display above and add no information to the plain URL. Another example is my own personal repository, http://sanibeltranquility.com/cygwin, which displays as Com - Sanibeltranquility - http://sanibeltranquility.com I would rather see the area and location left blank, or perhaps replaced by "Unknown". A third alternative would be to replace "Area - Location" by "User-added" or something similar. Ken