Fw: Service Control Updates - Round 2
=== - Original Message - From: "Guido Serassio" <[EMAIL PROTECTED]> To: <[EMAIL PROTECTED]> Cc: "Robert Collins" <[EMAIL PROTECTED]> Sent: Sunday, December 30, 2001 7:58 PM Subject: Service Control Updates - Round 2 > Hi, > > This patch implements all the remaining new Windows 2000 service functions > and Structures. > The first round was merged at 2001-12-02. > > 2001-12-30 Guido Serassio <[EMAIL PROTECTED]> > > * include/winsvc.h: Add EnumServiceStatusEx(), QueryServiceStatusEx() > & RegisterServiceCtrlHandlerEx() > > Guido > > > > - > === > Serassio Guido > Via Albenga, 11/4 10134 - Torino - ITALY > E-mail: [EMAIL PROTECTED] > [EMAIL PROTECTED] > WWW: http://www.serassio.it > w2kServiceEXupdate.patch Description: Binary data
Re: [PATCH] Setup.exe "other URL" functionality
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 linux machine. Also, your indent is changing -bool -PropSheet::SetActivePageByID (int resource_id) to +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: === int packagedb::installeddbread = 0; list < packagemeta, char const *, strcasecmp > packagedb::packages; list < Category, char const *, strcasecmp > packagedb::categories; PackageDBActions packagedb::task = PackageDB_Install; === 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. Rob
RE: [PATCH] Setup.exe "other URL" functionality
> 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
> linux machine.
>
NP. The official CVS can't handle text files with anything other than LF's?
Wait, don't answer that ;-(.
> Also, your indent is changing
> -bool
> -PropSheet::SetActivePageByID (int resource_id)
>
> to
>
> +bool PropSheet::SetActivePageByID (int resource_id)
>
> - and the first format is the correct one.
>
I see that, and I don't know what happened there. I reran indent and it changed
it to the GNU way. I know I indented, and these two functions aren't new. Plus
all other functions in the file are indented properly.
>
> 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.
>
site.cc as sent has definitely been run through indent. It's not generating the
"every line is different" problem, and looks fine to the unaided eye. What's
not formatted correctly? There *are* a very large number of additions and legit
changes - are you perhaps saying site.cc when you mean window.{cc,h}, which have
the "every line though identical is different to CVS" problem?
> Indent should be run with no options for cinstall. (It does the correct
> thing here).
Right, that's what I'm doing.
> However 2.2.7 is still very C++ broken. Sigh.
>
> (One specific example is:
> ===
> int
> packagedb::installeddbread =
> 0;
> list < packagemeta, char const *,
> strcasecmp >
> packagedb::packages;
> list < Category, char const *,
> strcasecmp >
> packagedb::categories;
> PackageDBActions
> packagedb::task =
> PackageDB_Install;
> ===
>
Yeah. Well, I would expect templates to cause it choke worse than this
actually; there was a time not very long ago when indent usually generated
uncompilable C++, even when thrown relatively few curves.
> 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).
Last one first: I don't think wchar_t and it's associated wstr* functions would
be appropriate at this time, since virtually nothing is currently using UNICODE.
TCHAR is a macro defined in the Windows headers that is esentially this:
#ifdef UNICODE
#define TCHAR wchar_t
#else
#define TCHAR char
#endif
This (and the plethora of other associated "FooA"/"FooW"-to-"Foo" defines, the
little-known "_tc" macros, etc) allows you to do an ASCII build,
an MBCS build, or a UNICODE build from the same sources by simply defining
UNICODE (or _UNICODE, can't remember which at this moment) on your compiler
command line, and everything sorts itself out. Using either "char" or
"wchar_t"/"WCHAR" locks you in.
Like the ChangeLog entry indicates, this is just another small step towards
internationalization of the entire app. TCHAR et al allows that to be done
incrementally instead of en masse (somebody at MS must have been asleep at the
switch to have gotten that right! ;-)).
> 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.).
It actually still behaves in that same way, but right, it doesn't look like it,
nor did I really grasp that that was how it was intended to work. I think we
need both "Add" and "Remove" in that case though.
> 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.
Well, the list of "known-about" URLs is definitely a two-part thing though: the
ones that get downloaded fresh every time you run setup, and the ones that the
user added, which I presume would be persistent across runs. The first wouldn't
presumably be subject to "Remove", while the second would pretty much require
it, and we'd need to have some way to inform the user of the difference
(different colors in the same list box perhaps?). I'll think on this, but I get
your drift - the combo box and radio buttons aren't an appropriate UI for the
intended functionality. Shoot.
> 3) If other.[cc|h] is not needed, we should rm them.
>
Right, sorry, forgot to request that in the post. They are not required for the
submitted patch; but then going to, say, an "Add new URL" button is going to
need a dialog box from somewhere
> 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 chan
Re: [PATCH] Setup.exe "other URL" functionality
- Original Message -
From: "Gary R. Van Sickle" <[EMAIL PROTECTED]>
>
> site.cc as sent has definitely been run through indent. It's not
generating the
> "every line is different" problem, and looks fine to the unaided eye.
What's
> not formatted correctly? There *are* a very large number of additions
and legit
> changes - are you perhaps saying site.cc when you mean window.{cc,h},
which have
> the "every line though identical is different to CVS" problem?
No, I mean site.cc. At the top in save_dialog, there is a new if
construct you've added, and at the end of it is visible
}
}
(thats two curly brackets at the same indent). Where you added the if,
the body thereof hasn't been indented. Running it through indent here
fixed it.
> Yeah. Well, I would expect templates to cause it choke worse than
this
> actually; there was a time not very long ago when indent usually
generated
> uncompilable C++, even when thrown relatively few curves.
:].
> Like the ChangeLog entry indicates, this is just another small step
towards
> internationalization of the entire app. TCHAR et al allows that to be
done
> incrementally instead of en masse (somebody at MS must have been
asleep at the
> switch to have gotten that right! ;-)).
Thank you.
> > 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.).
>
> It actually still behaves in that same way, but right, it doesn't look
like it,
> nor did I really grasp that that was how it was intended to work. I
think we
> need both "Add" and "Remove" in that case though.
Eventually, sure. For now, just adding the new URL will do, 'cause the
user can always CTRL-click to deselect any mistaken entries.
> > 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.
>
> Well, the list of "known-about" URLs is definitely a two-part thing
though: the
> ones that get downloaded fresh every time you run setup, and the ones
that the
> user added, which I presume would be persistent across runs. The
first wouldn't
> presumably be subject to "Remove", while the second would pretty much
require
> it, and we'd need to have some way to inform the user of the
difference
> (different colors in the same list box perhaps?). I'll think on this,
but I get
> your drift - the combo box and radio buttons aren't an appropriate UI
for the
> intended functionality. Shoot.
The ones that are downloaded fresh will eventually not get downloaded.
See README - it's in there.
Long term what will happen is:
1) A new user downloads the mirror.lst to bootstrap their local list.
2) Special sites - like sources.redhat.com are trimmed.
3) The user adds any sites.
4) The known list is stored on disk, as well as the users selections.
5) The user does an install/download, and _setup.ini_ contains an
additional list of known mirror sites, (and potentially a list of known
dead sites) that gets merged into the known list.
6) On subsequent runs 1) is skipped.
At the moment, 2,3,4 (minus storing the known list) are complete. I
don't see any point in indicating the difference in the list. Think of
apt, or rpm-find. The user should be able to do _anything_ they want to
that list. Remove ALL the sites if they desire (although setup.ini
contained ones would repopulate every run).
As far as UI goes, I think the combobox + a text box for the new site is
fine. But rather than a radio button to choose which is used, have an
Add button to the right of the textbox, and also make Enter in the
textbox trigger an add. Remove can be done by a button 'Delete selected
sites' that does just that.
Rob
