This is the mail archive of the cygwin-apps 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: [PATCH] make setup mirror list more like web page not just urls

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


Input	->
    Key		->;;CA;Ucalgary
CA - Ucalgary -
CA Ucalgary ca ucalgary cpsc mirror

[cyg]file://server/c$/usr/local/cygwin/var/cache/setup/packages	->
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		->
Local - Localhost - cygfile://var/cache/setup/packages
Local Localhost localhost cygfile://var/cache/setup/packages/

file:///c|/usr/local/cygwin/var/cache/setup/packages		->
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.


\\server\c$\usr\local\cygwin\var\cache\setup\packages		->
Local - Server - \\server\c$
Local Server server \\server\c$\usr\local\cygwin\var\cache\setup\packages/

c:\usr\local\cygwin\var\cache\setup\packages			->
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.

Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Attachment: 0001-setup-mirror-site-list-default-members-display-and-order-by-area-and-location.patch
Description: Text document

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