[PATCH] Setup.exe "other URL" functionality
Fri Nov 9 05:24:00 GMT 2001
Another request: please d2u all patches before you send them: The CVS
versions are in unix format, so your patches will fail if applied on a
Also, your indent is changing
-PropSheet::SetActivePageByID (int resource_id)
+bool PropSheet::SetActivePageByID (int resource_id)
- and the first format is the correct one.
Secondly, you should run indent on all modified fiels before diffing.
Indent created differences don't need changelog entries (unless the only
change in a file is due to diff). site.cc is definately incorrectly
formatted by your patch.
Indent should be run with no options for cinstall. (It does the correct
thing here). However 2.2.7 is still very C++ broken. Sigh.
(One specific example is:
list < packagemeta, char const *,
list < Category, char const *,
Moving along to the actual patch...
1) Whats a TCHAR? (site.cc). Is there some reason a wchar is not
appropriate? (i.e. Using gettext statically linked).
2) I don't like what you've done with the 'user URL'. The current
implementation allows the user to add 'n' arbitrary URL's, and merge
them with the downloaded list. I like the idea of combining the windows,
but the capabilities must stay the same as they are now. (ie on the
current CVS code, each time you click on 'other' the new URL is added to
the list, and added to the select URL's.). IOW it's not a boolean
user-or-offical choice, it's purely a list of URLs that are known about
and a list of select URL's. The source of the URL is irrelevant.
3) If other.[cc|h] is not needed, we should rm them.
Other than that it looks good, if you can explain 1) to me :], correct
2) and answer 3) and then send a new diff, with all modified files
indented, I'll re-review 2) and check it in. If you want to split out
the other changes from 2) and do two patches the other stuff can go in
without any more round trips.
More information about the Cygwin-patches