Gary R. Van Sickle wrote:
Throwing an exception would indicate a problem actually, if you want
to get all by-the-book about it.
I'm with Gary here, I'd prefer see an exception thrown for a problem.
Unless there's some issues with using exceptions that I don't know about...
(which, given
Gary R. Van Sickle wrote:
Gary,
Here is a partial list of issues from your mega-patch.
I still bristle at the mega ;-). 43K including the bulk of res.rc ain't
even *close* to mega ;-).
It it if you think about in terms of number of separate concepts included,
instead of byte size :-)
*
Gary R. Van Sickle wrote:
I'm still letting you guys fight this out, but I'm going to snipe from the
sidelines ;-):
...
I once again leave you two to fight it out while I get some actual code
written.
That's not particularly helpful, Gary. Anyway, as per what I said in what I
snipped above, I
Robert Collins wrote:
On Mon, 2003-07-21 at 04:17, Gary R. Van Sickle wrote:
Unless there will ever be a need to ask a page whether
it would take activation in the future, but not activate it
immediately, even if it is possible to do so, I think the 2 calls
should be merged. Will there ever
On Mon, 2003-07-21 at 17:32, Morrison, John wrote:
Would...
if (canActivate())
OnActivate()
be better? (although the OnXXX functions always make me think that
they should be callbacks.)
Yes - I was simply leaving method names alone until I had an answer on
the ordering breaking
On Mon, 2003-07-21 at 15:25, Gary R. Van Sickle wrote:
Well, my current code appears to work if changed to do that. But then
OnAcceptActivate() is equivalent to my original return value changes (i.e. just
leave OnActivate() empty and OnAcceptActivate() is your message handler).
Maybe I'm not
OK, this is a general reply to multiple messages.
I still believe bool OnActivate() to be the better option - here's why:
The if(canActivate()){OnActivate()} scheme makes 2 method calls where only
one is required. It also opens the possibility for OnActivate to be called
when activation is not
Gary R. Van Sickle wrote:
I'll do my best to get something up yet tonight. Again though Max, please
keep in mind that I posted the SetupXP stuff mainly so people could try
out
the now-proven-to-not-work-right XP theme feature, not because I had loads
of
time to get back on the bigger
Gary,
Here is a partial list of issues from your mega-patch.
* Issue: Drop -r HEAD
Please do this ASAP. If you need further evidence for the desirability of
this, just look to res.rc, specifically at the way your diff removes my
multiline comment about MS Shell Dlg.
* Issue: LogFile::Exit
On Tue, 2003-07-22 at 00:02, Max Bowsher wrote:
OK, this is a general reply to multiple messages.
I still believe bool OnActivate() to be the better option - here's why:
The if(canActivate()){OnActivate()} scheme makes 2 method calls where only
one is required.
Premature optimisation.
I'm still letting you guys fight this out, but I'm going to snipe from the
sidelines ;-):
[snip]
I do not see bool OnActivate() as being confusing, nor as less intuitive
that firing 2 event handlers consecutively.
There is only one handler. I'm glad that it wouldn't confuse you though
:}.
Gary,
Here is a partial list of issues from your mega-patch.
I still bristle at the mega ;-). 43K including the bulk of res.rc ain't even
*close* to mega ;-).
* Issue: Drop -r HEAD
Please do this ASAP. If you need further evidence for the desirability of
this, just look to res.rc,
First, please drop
-r HEAD from your diff command. All that accomplishes is to make the
generated patch *revert all changes to HEAD that you haven't merged into
your local copy*.
Ouch, ok, important safety tip. I thought I had gotten all the
changes to HEAD,
but as you discovered
Robert Collins wrote:
On Sat, 2003-07-19 at 23:40, Max Bowsher wrote:
Gary's current SetupXP patchset calls 2 member functions on page
activation:
OnActivate (returns void), and OnAcceptActivation (returns bool). I think
this is unnecessarily messy. AFAICS, OnAcceptActivation only exists
| * res.rc
| (IDD_SPLASH): Move icon.
Actually, you just changed the width.
Indeed. Not sure what happened there. Remember that those entries are a bit
old, I may have un-changed things in the interim.
Widths of 21 and 20 are used at
various places in res.rc. I don't know why. If you can
I cannot think of one. It exists soley to give OnActivate a default return
code. It *can't* be called anywhere else, since in the general case,
OnAcceptActivation won't know if it needs to refuse activation until after
OnAccept is called.
OnActivate
--
Gary R. Van Sickle
On Sun, 20 Jul 2003, Gary R. Van Sickle wrote:
| (IDD_DESKTOP): Move controls. Add Cygwin icon.
Actually you moved, not added, the Cygwin icon.
Also, though I really like the idea of a Finished page, I'm not entirely
convinces that it should be merged with the Create Icons page. What
I would like to propose NOT moving the global font settings into
PropertyPage::OnInit, and consequently not requiring Call base class
OnInit() changes in all derived classes.
Don't quite follow the former, agree with the latter if there's another way to
do it. The pages themselves are
Gary R. Van Sickle wrote:
* res.rc
(IDD_SPLASH): Move icon.
Actually, you just changed the width.
Indeed. Not sure what happened there. Remember that those entries are a
bit
old, I may have un-changed things in the interim.
OK, but please work out what you actually want to change, or
Gary R. Van Sickle wrote:
I would like to propose NOT moving the global font settings into
PropertyPage::OnInit, and consequently not requiring Call base class
OnInit() changes in all derived classes.
Don't quite follow the former, agree with the latter if there's another
way to
do it. The
Gary R. Van Sickle wrote:
Gary's current SetupXP patchset calls 2 member functions on page
activation:
OnActivate (returns void), and OnAcceptActivation (returns bool). I think
this is unnecessarily messy. AFAICS, OnAcceptActivation only exists to
prevent the need to change the return type
Gary R. Van Sickle wrote:
Robert Collins wrote:
On Sat, 2003-07-19 at 23:40, Max Bowsher wrote:
Gary's current SetupXP patchset calls 2 member functions on page
activation: OnActivate (returns void), and OnAcceptActivation (returns
bool). I think this is unnecessarily messy. AFAICS
On Mon, 2003-07-21 at 04:17, Gary R. Van Sickle wrote:
Unless there will ever be a need to ask a page whether
it would take activation in the future, but not activate it immediately,
even if it is possible to do so, I think the 2 calls should be merged. Will
there ever be such a case?
On Mon, 2003-07-21 at 06:44, Max Bowsher wrote:
Gary R. Van Sickle wrote:
On unknown, Max Bowsher wrote:
I would very much prefer changing OnActivate to return bool, combining
the
purpose of both functions. Yes, this does require changes in all derived
classes, but the changes are
Gary R. Van Sickle wrote:
Gary's current SetupXP patchset calls 2 member functions on page
activation:
OnActivate (returns void), and OnAcceptActivation (returns bool). I think
this is unnecessarily messy. AFAICS, OnAcceptActivation only exists to
prevent the need to change the return
Gary R. Van Sickle wrote:
[snip]
Gary, I don't think the clarity of setup's code is trivial.
And, I can't imagine that any project would accept a monolithic patch
encompassing multiple concepts.
I'm not referring to multiple concepts, I'm referring to this
OnAcceptActivation() thing. Like I
[snip]
I cannot think of one. It exists soley to give OnActivate a default return
code. It *can't* be called anywhere else, since in the general case,
OnAcceptActivation won't know if it needs to refuse activation until after
OnAccept is called.
Hmm. My intention when I suggested a
Robert Collins wrote:
On Sat, 2003-07-19 at 11:32, Gary R. Van Sickle wrote:
Wait! The status quo must remain, until we have confirmed evidence that
static destructors do always run on exit from -mno-cygwin programs.
As Rob said, he's said that too. What am I missing? Neither of
LogFile's
Gary R. Van Sickle wrote:
On Sat, 2003-07-19 at 11:32, Gary R. Van Sickle wrote:
Wait! The status quo must remain, until we have confirmed evidence that
static destructors do always run on exit from -mno-cygwin programs.
As Rob said, he's said that too. What am I missing? Neither of
| * res.rc
| (IDD_SPLASH): Move icon.
Actually, you just changed the width. Widths of 21 and 20 are used at
various places in res.rc. I don't know why. If you can work out which is
correct, please send a patch updating all uses to it. But, please don't
change only a single instance.
| Change
I would like to propose NOT moving the global font settings into
PropertyPage::OnInit, and consequently not requiring Call base class
OnInit() changes in all derived classes.
Since the font settings are *already* set up to fail silently if the
relevant control is not present, I don't think we
On Sat, 2003-07-19 at 23:33, Max Bowsher wrote:
I would like to propose NOT moving the global font settings into
PropertyPage::OnInit, and consequently not requiring Call base class
OnInit() changes in all derived classes.
Since the font settings are *already* set up to fail silently if the
Robert Collins wrote:
On Sat, 2003-07-19 at 23:40, Max Bowsher wrote:
Gary's current SetupXP patchset calls 2 member functions on page
activation:
OnActivate (returns void), and OnAcceptActivation (returns bool). I think
this is unnecessarily messy. AFAICS, OnAcceptActivation only exists
Robert Collins wrote:
On Sat, 2003-07-19 at 23:33, Max Bowsher wrote:
I would like to propose NOT moving the global font settings into
PropertyPage::OnInit, and consequently not requiring Call base class
OnInit() changes in all derived classes.
Since the font settings are *already* set up to
Gary R. Van Sickle wrote:
Gary R. Van Sickle wrote:
and a reversion of the wizard page titles to Cygwin Setup
(which I need to bring up for discussion separately).
Indeed. First, I *really* don't see this change happening as is. It was
originally done to assist automation programs. I don't
On Fri, 2003-07-18 at 17:13, Max Bowsher wrote:
Wait! The status quo must remain, until we have confirmed evidence that
static destructors do always run on exit from -mno-cygwin programs. Then and
*only* then can we move to using simple CRT exit(). Until then I don't think
there is any harm
Robert Collins wrote:
On Fri, 2003-07-18 at 17:13, Max Bowsher wrote:
Wait! The status quo must remain, until we have confirmed evidence that
static destructors do always run on exit from -mno-cygwin programs. Then
and
*only* then can we move to using simple CRT exit(). Until then I don't
On Sat, 2003-07-19 at 11:32, Gary R. Van Sickle wrote:
Well, the harm is mainly in its use spreading. The longer it sits, the more
entrenched it will become, and the larger the patch required to fix it.
Agreed - but..
I'd really rather shrink your outstanding patch list down before we
start
On Sat, 2003-07-19 at 11:32, Gary R. Van Sickle wrote:
Wait! The status quo must remain, until we have confirmed evidence that
static destructors do always run on exit from -mno-cygwin programs.
As Rob said, he's said that too. What am I missing? Neither of
LogFile's nor
Gary R. Van Sickle wrote:
and a reversion of the wizard page titles to Cygwin Setup
(which I need to bring up for discussion separately).
Indeed. First, I *really* don't see this change happening as is. It was
originally done to assist automation programs. I don't think we should pull
the plug
Gary R. Van Sickle wrote:
and a reversion of the wizard page titles to Cygwin Setup
(which I need to bring up for discussion separately).
Indeed. First, I *really* don't see this change happening as is. It was
originally done to assist automation programs. I don't think we should pull
the
Hi Gary,
More stuff to decrease the size of your diff:
I think you are only supposed to update Copyright comments when you actually
change something in a file? Unless I am wrong, please drop your
copyright-comment-only change to splash.h from your local mods.
Please rm ChangeLog.window.txt,
On Thu, 2003-07-17 at 00:46, Max Bowsher wrote:
Hi Gary,
Please add the following to your generated files filter when making your
tarball:
inilex.cc iniparse.cc iniparse.h res.aps setup_version.c Setup.aep Setup.aew
Setup.dev Setup.dev.bak Setup.layout
Don't tarball by hand. Use make
On Thu, 2003-07-17 at 00:46, Max Bowsher wrote:
Hi Gary,
Please add the following to your generated files filter when making your
tarball:
inilex.cc iniparse.cc iniparse.h res.aps setup_version.c Setup.aep Setup.aew
Setup.dev Setup.dev.bak Setup.layout
Don't tarball by hand. Use
Hi Gary,
More stuff to decrease the size of your diff:
I think you are only supposed to update Copyright comments when you actually
change something in a file? Unless I am wrong, please drop your
copyright-comment-only change to splash.h from your local mods.
No, I know, I noticed that
[snip]
Seems I spoke too soon. The diff is still messy in parts.
Oh I know. I'm not submitting anything for inclusion yet, just trying to get
some general feedback.
First, please drop
-r HEAD from your diff command. All that accomplishes is to make the
generated patch *revert all changes
Gary R. Van Sickle wrote:
Synced to current cvs HEAD, use at your own risk.
Max Bowsher wrote:
^^
Doesn't seem to be. In fact, it doesn't even seem to be based on a
consistent set of files. This makes diffing out your changes virtually
impossible.
Gary R. Van
Gary R. Van Sickle wrote:
Anybody curious as to what an XP-ified Setup with a bigger chooser would
look
like can check out such a hypothetical beast ri-cheer:
http://home.att.net/~g.r.vansickle/cygwin/setup/
While it will also run on any non-XP Windows, you won't get the cool new
common
Gary R. Van Sickle wrote:
Anybody curious as to what an XP-ified Setup with a bigger chooser would
look
like can check out such a hypothetical beast ri-cheer:
http://home.att.net/~g.r.vansickle/cygwin/setup/
While it will also run on any non-XP Windows, you won't get the cool new
Gary R. Van Sickle wrote:
Anybody curious as to what an XP-ified Setup with a bigger chooser would
look
like can check out such a hypothetical beast ri-cheer:
http://home.att.net/~g.r.vansickle/cygwin/setup/
Do you plan to integrate this into the mainline?
Synced to current cvs HEAD, use at
Gary R. Van Sickle wrote:
Anybody curious as to what an XP-ified Setup with a bigger chooser would
look
like can check out such a hypothetical beast ri-cheer:
http://home.att.net/~g.r.vansickle/cygwin/setup/
Do you plan to integrate this into the mainline?
Don't know what else I'd do
Anybody curious as to what an XP-ified Setup with a bigger chooser would look
like can check out such a hypothetical beast ri-cheer:
http://home.att.net/~g.r.vansickle/cygwin/setup/
While it will also run on any non-XP Windows, you won't get the cool new common
controls (buttons, progress bars,
52 matches
Mail list logo
