Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
Ping?..
Igor
On Sat, 1 Feb 2003, Igor Pechtchanski wrote:
On 2 Feb 2003, Robert Collins wrote:
On Sun, 2003-02-02 at 07:37, Igor Pechtchanski wrote:
Yes, it does recompile everything when resource.h is modified, but that
only happens once. A necessary evil...
Even so... resource.h urrghh.
Sorry about the (null) bit. Should be fixed now. I also took your
suggestion regarding log file. The third iteration of the patch is
attached...
Igor
ChangeLog:
2002-10-17 Igor Pechtchanski [EMAIL PROTECTED]
* res.rc (IDS_INSTALL_INCOMPLETE): Change hard-coded
log filename to %s.
(IDS_MISSING_LOG): New string resource.
* resource.h (IDS_MISSING_LOG): New resource.
* LogFile.cc (LogFile::exit): Pass log filename for
LOG_BABBLE to note().
(LogFile::getFile): New function.
* LogFile.h (LogFile::getFile): New function.
getFile should be getFilename or getFileName.
getFileName doesn't take a minlevel, it takes an exactLevel - your
parameter name is misleading, or you've got the logic in the loop wrong
:}.
IDS_MISSING_LOG should be No log was in use, not setup.log.full.
It'll be ugly in the output, but at least accurate.
Do those three changes, and I think this is ready. nice work guys.
Rob
Rob,
I was mislead by the setFile() prototype. I agree with your proposed
changes (and Max's use of angle brackets). Take four is attached.
Thanks.
Igor
==
ChangeLog:
2003-02-01 Igor Pechtchanski [EMAIL PROTECTED]
* res.rc (IDS_INSTALL_INCOMPLETE): Change hard-coded
log filename to %s.
(IDS_MISSING_LOG): New string resource.
* resource.h (IDS_MISSING_LOG): New resource.
* LogFile.cc (LogFile::exit): Pass log filename for
LOG_BABBLE to note().
(LogFile::getFileName): New function.
* LogFile.h (LogFile::getFileName): New function.
--
http://cs.nyu.edu/~pechtcha/
|\ _,,,---,,_[EMAIL PROTECTED]
ZZZzz /,`.-'`'-. ;-;;,_[EMAIL PROTECTED]
|,4- ) )-,_. ,\ ( `'-' Igor Pechtchanski
'---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow!
Oh, boy, virtual memory! Now I'm gonna make myself a really *big* RAMdisk!
-- /usr/games/fortune
Index: LogFile.cc
===
RCS file: /cvs/cygwin-apps/setup/LogFile.cc,v
retrieving revision 2.6
diff -u -p -r2.6 LogFile.cc
--- LogFile.cc 25 Nov 2002 00:41:24 - 2.6
+++ LogFile.cc 1 Feb 2003 23:11:30 -
@@ -31,6 +31,8 @@ static const char *cvsid =
#include time.h
#include string
#include AntiVirus.h
+#include filemanip.h
+#include cistring.h
using namespace std;
@@ -101,6 +103,20 @@ LogFile::setFile (int minlevel, String c
files.insert (t);
}
+String
+LogFile::getFileName (int level) const
+{
+ for (FileSet::iterator i = files.begin();
+ i != files.end(); ++i)
+{
+ if (i-level == level)
+return i-key;
+}
+ cistring bad_file;
+ bad_file.Format(IDS_MISSING_LOG);
+ return bad_file.c_str();
+}
+
void
LogFile::exit (int const exit_code)
{
@@ -115,7 +131,7 @@ LogFile::exit (int const exit_code)
been_here = 1;
if (exit_msg)
-note (NULL, exit_msg);
+note (NULL, exit_msg, backslash(getFileName(LOG_BABBLE)).cstr_oneuse());
log (LOG_TIMESTAMP) Ending cygwin install endLog;
Index: LogFile.h
===
RCS file: /cvs/cygwin-apps/setup/LogFile.h,v
retrieving revision 2.3
diff -u -p -r2.3 LogFile.h
--- LogFile.h 10 Nov 2002 03:56:05 - 2.3
+++ LogFile.h 1 Feb 2003 23:11:30 -
@@ -23,6 +23,7 @@ public:
LogFile();
void clearFiles(); // delete all target filenames
void setFile (int minlevel, String const path, bool append);
+ String getFileName (int level) const;
/* Some platforms don't call destructors. So this call exists
* which guarantees to flush any log data...
* but doesn't call generic C++ destructors
Index: res.rc
===
RCS file: /cvs/cygwin-apps/setup/res.rc,v
retrieving revision 2.45
diff -u -p -r2.45 res.rc
--- res.rc 19 Jan 2003 20:31:53 - 2.45
+++ res.rc 1 Feb 2003 23:11:47 -
@@ -483,11 +483,12 @@ BEGIN
IDS_DOWNLOAD_FAILED Unable to download %s
IDS_DOWNLOAD_INCOMPLETE Download Incomplete. Try again?
IDS_INSTALL_ERROR Installation error (%s), Continue with other packages?
-IDS_INSTALL_INCOMPLETE Installation incomplete. Check /setup.log.full for
details
+IDS_INSTALL_INCOMPLETE Installation incomplete. Check %s for details
IDS_VERSION_INFOSetup.exe version %1
IDS_CYGWIN_SETUPCygwin Setup
Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
Igor Pechtchanski wrote:
Ping? (Just making sure this was seen).
On Fri, 24 Jan 2003, Igor Pechtchanski wrote:
2002-10-17 Igor Pechtchanski [EMAIL PROTECTED]
* res.rc (IDS_INSTALL_INCOMPLETE): Change hard-coded
log filename to %s.
* LogFile.cc (LogFile::exit): Pass log filename for
LOG_BABBLE to note().
(LogFile::getFile): New function.
* LogFile.h (LogFile::getFile): New function.
Definitely got here - I imagine Robert is just busy.
What do you think about the following suggestions ?
In:
+static String bad_file = the log;
+
+String const
+LogFile::getFile (int minlevel) const
+{
+ for (FileSet::iterator i = files.begin();
+ i != files.end(); ++i)
+{
+ if (i-level == minlevel)
+return i-key;
+}
+ return bad_file;
+}
just do: return the log; , and remove the bad_file variable?
And:
+ String log_full = cygpath(getFile(LOG_BABBLE));
if (exit_msg)
-note (NULL, exit_msg);
+note (NULL, exit_msg, log_full.cstr_oneuse());
Just do:
*note (NULL, exit_msg, cygpath(getFile(LOG_BABBLE)).cstr_oneuse());
Just a matter of personal opinion, really, so feel free to say no :-)
Max.
Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
Max,
Thanks for the feedback. Replies inline below.
On Sat, 1 Feb 2003, Max Bowsher wrote:
Igor Pechtchanski wrote:
Ping? (Just making sure this was seen).
On Fri, 24 Jan 2003, Igor Pechtchanski wrote:
2002-10-17 Igor Pechtchanski [EMAIL PROTECTED]
* res.rc (IDS_INSTALL_INCOMPLETE): Change hard-coded
log filename to %s.
* LogFile.cc (LogFile::exit): Pass log filename for
LOG_BABBLE to note().
(LogFile::getFile): New function.
* LogFile.h (LogFile::getFile): New function.
Definitely got here - I imagine Robert is just busy.
Right. No problem at all.
What do you think about the following suggestions ?
In:
+static String bad_file = the log;
+
+String const
+LogFile::getFile (int minlevel) const
+{
+ for (FileSet::iterator i = files.begin();
+ i != files.end(); ++i)
+{
+ if (i-level == minlevel)
+return i-key;
+}
+ return bad_file;
+}
just do: return the log; , and remove the bad_file variable?
Hmm. Personally, I prefer to use named constants whenever possible.
However, static variables aren't the way to go -- I must have been
programming too much Java lately... :-) I should have used a #define, but
then got a better idea - that string should really be a resource, as it's
a phrase, not a filename.
And:
+ String log_full = cygpath(getFile(LOG_BABBLE));
if (exit_msg)
-note (NULL, exit_msg);
+note (NULL, exit_msg, log_full.cstr_oneuse());
Just do:
*note (NULL, exit_msg, cygpath(getFile(LOG_BABBLE)).cstr_oneuse());
Yes, definitely. I thought I was paving the way for Robert's suggested
table of logs (that we would have to somehow postprocess), but then
realized that the postprocessing could be wrapped in a function that
returns the formatted string.
Just a matter of personal opinion, really, so feel free to say no :-)
Max.
By all means, thanks for your suggestions. New version of the patch
attached.
Igor
ChangeLog:
2002-10-17 Igor Pechtchanski [EMAIL PROTECTED]
* res.rc (IDS_INSTALL_INCOMPLETE): Change hard-coded
log filename to %s.
(IDS_MISSING_LOG): New string resource.
* LogFile.cc (LogFile::exit): Pass log filename for
LOG_BABBLE to note().
(LogFile::getFile): New function.
* LogFile.h (LogFile::getFile): New function.
--
http://cs.nyu.edu/~pechtcha/
|\ _,,,---,,_[EMAIL PROTECTED]
ZZZzz /,`.-'`'-. ;-;;,_[EMAIL PROTECTED]
|,4- ) )-,_. ,\ ( `'-' Igor Pechtchanski
'---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow!
Oh, boy, virtual memory! Now I'm gonna make myself a really *big* RAMdisk!
-- /usr/games/fortune
Index: LogFile.cc
===
RCS file: /cvs/cygwin-apps/setup/LogFile.cc,v
retrieving revision 2.6
diff -u -p -r2.6 LogFile.cc
--- LogFile.cc 25 Nov 2002 00:41:24 - 2.6
+++ LogFile.cc 1 Feb 2003 17:25:10 -
@@ -31,6 +31,8 @@ static const char *cvsid =
#include time.h
#include string
#include AntiVirus.h
+#include mount.h
+#include cistring.h
using namespace std;
@@ -101,6 +103,20 @@ LogFile::setFile (int minlevel, String c
files.insert (t);
}
+String
+LogFile::getFile (int minlevel) const
+{
+ for (FileSet::iterator i = files.begin();
+ i != files.end(); ++i)
+{
+ if (i-level == minlevel)
+return i-key;
+}
+ cistring bad_file;
+ bad_file.Format(IDS_MISSING_LOG);
+ return bad_file.c_str();
+}
+
void
LogFile::exit (int const exit_code)
{
@@ -115,7 +131,7 @@ LogFile::exit (int const exit_code)
been_here = 1;
if (exit_msg)
-note (NULL, exit_msg);
+note (NULL, exit_msg, cygpath(getFile(LOG_BABBLE)).cstr_oneuse());
log (LOG_TIMESTAMP) Ending cygwin install endLog;
Index: LogFile.h
===
RCS file: /cvs/cygwin-apps/setup/LogFile.h,v
retrieving revision 2.3
diff -u -p -r2.3 LogFile.h
--- LogFile.h 10 Nov 2002 03:56:05 - 2.3
+++ LogFile.h 1 Feb 2003 17:25:10 -
@@ -23,6 +23,7 @@ public:
LogFile();
void clearFiles(); // delete all target filenames
void setFile (int minlevel, String const path, bool append);
+ String getFile (int minlevel) const;
/* Some platforms don't call destructors. So this call exists
* which guarantees to flush any log data...
* but doesn't call generic C++ destructors
Index: res.rc
===
RCS file: /cvs/cygwin-apps/setup/res.rc,v
retrieving revision 2.45
diff -u -p -r2.45 res.rc
--- res.rc 19 Jan 2003 20:31:53 - 2.45
+++ res.rc 1 Feb 2003 17:25:22 -
@@ -483,11 +483,12 @@ BEGIN
IDS_DOWNLOAD_FAILED Unable to download %s
IDS_DOWNLOAD_INCOMPLETE
Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
Igor Pechtchanski wrote: just do: return the log; , and remove the bad_file variable? Hmm. Personally, I prefer to use named constants whenever possible. It seems a little silly to use a constant when it is required only once. However, static variables aren't the way to go -- I must have been programming too much Java lately... :-) I should have used a #define, but then got a better idea - that string should really be a resource, as it's a phrase, not a filename. General question: What advantage do resources give us over string literals, for messages that are only used once in setup? Max.
RE: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
From: Max Bowsher General question: What advantage do resources give us over string literals, for messages that are only used once in setup? I18N J.
Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
On Sat, 1 Feb 2003, Max Bowsher wrote: Igor Pechtchanski wrote: just do: return the log; , and remove the bad_file variable? Hmm. Personally, I prefer to use named constants whenever possible. It seems a little silly to use a constant when it is required only once. I know, but your programming style is beaten into you when you learn programming, and is usually very hard to unlearn. ;-) However, static variables aren't the way to go -- I must have been programming too much Java lately... :-) I should have used a #define, but then got a better idea - that string should really be a resource, as it's a phrase, not a filename. General question: What advantage do resources give us over string literals, for messages that are only used once in setup? Max. Resources have the advantage that whole resource tables can be substituted at once. If we ever want to internationalize setup, for example, all that'd be needed is translating the resource file. Having hard-coded string constants sprinkled all over the code makes translation impossible. Not that anyone has any plans to translate setup any time soon... Again, I guess it's a question of programming style. Igor -- http://cs.nyu.edu/~pechtcha/ |\ _,,,---,,_[EMAIL PROTECTED] ZZZzz /,`.-'`'-. ;-;;,_[EMAIL PROTECTED] |,4- ) )-,_. ,\ ( `'-' Igor Pechtchanski '---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow! Oh, boy, virtual memory! Now I'm gonna make myself a really *big* RAMdisk! -- /usr/games/fortune
Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
Igor Pechtchanski wrote: On Sat, 1 Feb 2003, Max Bowsher wrote: What advantage do resources give us over string literals, for messages that are only used once in setup? Resources have the advantage that whole resource tables can be substituted at once. If we ever want to internationalize setup, for example, all that'd be needed is translating the resource file. Having hard-coded string constants sprinkled all over the code makes translation impossible. Not that anyone has any plans to translate setup any time soon... Again, I guess it's a question of programming style. Fair enough. The thing I *really* hate, is that any change to resource.h, and you end up recompiling *everything*. Anyway... I just tried it out, and was told Check (null) for details. I don't think you want to call cygpath at all. LogFile::getFile already returns a Windows path. (With rather ugly looking mixed slashes. backslash()-ing the path might be nice. One other point - it might be more informative for the fallback text to be setup.log.full rather than log file. Max.
Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
On Sat, 1 Feb 2003, Max Bowsher wrote:
Igor Pechtchanski wrote:
On Sat, 1 Feb 2003, Max Bowsher wrote:
What advantage do resources give us over string literals, for
messages that are only used once in setup?
Resources have the advantage that whole resource tables can be
substituted at once. If we ever want to internationalize setup, for
example, all
that'd be needed is translating the resource file. Having hard-coded
string constants sprinkled all over the code makes translation
impossible. Not that anyone has any plans to translate setup any time
soon... Again,
I guess it's a question of programming style.
Fair enough. The thing I *really* hate, is that any change to resource.h,
and you end up recompiling *everything*.
Anyway...
I just tried it out, and was told Check (null) for details.
I don't think you want to call cygpath at all. LogFile::getFile already
returns a Windows path.
(With rather ugly looking mixed slashes. backslash()-ing the path might be
nice.
One other point - it might be more informative for the fallback text to be
setup.log.full rather than log file.
Max.
Yes, it does recompile everything when resource.h is modified, but that
only happens once. A necessary evil...
Sorry about the (null) bit. Should be fixed now. I also took your
suggestion regarding log file. The third iteration of the patch is
attached...
Incidentally, there are two backslash() functions defined: one in
filemanip.h, and another in concat.h. This is the only function defined
in concat.h, and it doesn't seem to be used anywhere anymore. Is concat.h
dead? Should it be removed? Ditto for concat.cc?
Igor
ChangeLog:
2002-10-17 Igor Pechtchanski [EMAIL PROTECTED]
* res.rc (IDS_INSTALL_INCOMPLETE): Change hard-coded
log filename to %s.
(IDS_MISSING_LOG): New string resource.
* resource.h (IDS_MISSING_LOG): New resource.
* LogFile.cc (LogFile::exit): Pass log filename for
LOG_BABBLE to note().
(LogFile::getFile): New function.
* LogFile.h (LogFile::getFile): New function.
--
http://cs.nyu.edu/~pechtcha/
|\ _,,,---,,_[EMAIL PROTECTED]
ZZZzz /,`.-'`'-. ;-;;,_[EMAIL PROTECTED]
|,4- ) )-,_. ,\ ( `'-' Igor Pechtchanski
'---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow!
Oh, boy, virtual memory! Now I'm gonna make myself a really *big* RAMdisk!
-- /usr/games/fortune
Index: LogFile.cc
===
RCS file: /cvs/cygwin-apps/setup/LogFile.cc,v
retrieving revision 2.6
diff -u -p -r2.6 LogFile.cc
--- LogFile.cc 25 Nov 2002 00:41:24 - 2.6
+++ LogFile.cc 1 Feb 2003 20:31:02 -
@@ -31,6 +31,8 @@ static const char *cvsid =
#include time.h
#include string
#include AntiVirus.h
+#include filemanip.h
+#include cistring.h
using namespace std;
@@ -101,6 +103,20 @@ LogFile::setFile (int minlevel, String c
files.insert (t);
}
+String
+LogFile::getFile (int minlevel) const
+{
+ for (FileSet::iterator i = files.begin();
+ i != files.end(); ++i)
+{
+ if (i-level == minlevel)
+return i-key;
+}
+ cistring bad_file;
+ bad_file.Format(IDS_MISSING_LOG);
+ return bad_file.c_str();
+}
+
void
LogFile::exit (int const exit_code)
{
@@ -115,7 +131,7 @@ LogFile::exit (int const exit_code)
been_here = 1;
if (exit_msg)
-note (NULL, exit_msg);
+note (NULL, exit_msg, backslash(getFile(LOG_BABBLE)).cstr_oneuse());
log (LOG_TIMESTAMP) Ending cygwin install endLog;
Index: LogFile.h
===
RCS file: /cvs/cygwin-apps/setup/LogFile.h,v
retrieving revision 2.3
diff -u -p -r2.3 LogFile.h
--- LogFile.h 10 Nov 2002 03:56:05 - 2.3
+++ LogFile.h 1 Feb 2003 20:31:02 -
@@ -23,6 +23,7 @@ public:
LogFile();
void clearFiles(); // delete all target filenames
void setFile (int minlevel, String const path, bool append);
+ String getFile (int minlevel) const;
/* Some platforms don't call destructors. So this call exists
* which guarantees to flush any log data...
* but doesn't call generic C++ destructors
Index: res.rc
===
RCS file: /cvs/cygwin-apps/setup/res.rc,v
retrieving revision 2.45
diff -u -p -r2.45 res.rc
--- res.rc 19 Jan 2003 20:31:53 - 2.45
+++ res.rc 1 Feb 2003 20:31:19 -
@@ -483,11 +483,12 @@ BEGIN
IDS_DOWNLOAD_FAILED Unable to download %s
IDS_DOWNLOAD_INCOMPLETE Download Incomplete. Try again?
IDS_INSTALL_ERROR Installation error (%s), Continue with other packages?
-IDS_INSTALL_INCOMPLETE Installation incomplete. Check
Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
Igor Pechtchanski wrote: Incidentally, there are two backslash() functions defined: one in filemanip.h, and another in concat.h. This is the only function defined in concat.h, and it doesn't seem to be used anywhere anymore. Is concat.h dead? Should it be removed? Ditto for concat.cc? Seems so. Robert? Max.
Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
On Sun, 2003-02-02 at 07:37, Igor Pechtchanski wrote: Yes, it does recompile everything when resource.h is modified, but that only happens once. A necessary evil... Even so... resource.h urrghh. Sorry about the (null) bit. Should be fixed now. I also took your suggestion regarding log file. The third iteration of the patch is attached... Incidentally, there are two backslash() functions defined: one in filemanip.h, and another in concat.h. This is the only function defined in concat.h, and it doesn't seem to be used anywhere anymore. Is concat.h dead? Should it be removed? Ditto for concat.cc? Not sure, lets leave this for now. Igor ChangeLog: 2002-10-17 Igor Pechtchanski [EMAIL PROTECTED] * res.rc (IDS_INSTALL_INCOMPLETE): Change hard-coded log filename to %s. (IDS_MISSING_LOG): New string resource. * resource.h (IDS_MISSING_LOG): New resource. * LogFile.cc (LogFile::exit): Pass log filename for LOG_BABBLE to note(). (LogFile::getFile): New function. * LogFile.h (LogFile::getFile): New function. getFile should be getFilename or getFileName. getFileName doesn't take a minlevel, it takes an exactLevel - your parameter name is misleading, or you've got the logic in the loop wrong :}. IDS_MISSING_LOG should be No log was in use, not setup.log.full. It'll be ugly in the output, but at least accurate. Do those three changes, and I think this is ready. nice work guys. Rob -- GPG key available at: http://users.bigpond.net.au/robertc/keys.txt. signature.asc Description: This is a digitally signed message part
Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
Robert Collins wrote:
getFileName doesn't take a minlevel, it takes an exactLevel - your
parameter name is misleading, or you've got the logic in the loop
wrong :}.
Or, you could say that it gets the file whose minlevel is what you request.
In which case its correct :-)
IDS_MISSING_LOG should be No log was in use, not setup.log.full.
It'll be ugly in the output, but at least accurate.
Check No log was in use for details.
That's worse than what we have in CVS now.
How about:
Index: LogFile.cc
===
RCS file: /home/max/cvsmirror/cygwin-apps-cvs/setup/LogFile.cc,v
retrieving revision 2.6
diff -u -p -r2.6 LogFile.cc
--- LogFile.cc 25 Nov 2002 00:41:24 - 2.6
+++ LogFile.cc 1 Feb 2003 21:18:17 -
@@ -31,6 +31,7 @@ static const char *cvsid =
#include time.h
#include string
#include AntiVirus.h
+#include filemanip.h
using namespace std;
@@ -101,9 +102,22 @@ LogFile::setFile (int minlevel, String c
files.insert (t);
}
+String
+LogFile::getFile (int minlevel) const
+{
+ for (FileSet::iterator i = files.begin();
+ i != files.end(); ++i)
+{
+ if (i-level == minlevel)
+return i-key;
+}
+ return String();
+}
+
void
LogFile::exit (int const exit_code)
{
+ String tempString;
AntiVirus::AtExit();
static int been_here = 0;
if (been_here)
@@ -114,8 +128,15 @@ LogFile::exit (int const exit_code)
#endif
been_here = 1;
+ if (exit_msg == IDS_INSTALL_INCOMPLETE)
+{
+ tempString = backslash(getFile(LOG_BABBLE));
+ if (tempString.size() == 0)
+exit_msg = IDS_INSTALL_INCOMPLETE_NO_LOG;
+}
+
if (exit_msg)
-note (NULL, exit_msg);
+note (NULL, exit_msg, tempString.cstr_oneuse());
log (LOG_TIMESTAMP) Ending cygwin install endLog;
Index: LogFile.h
===
RCS file: /home/max/cvsmirror/cygwin-apps-cvs/setup/LogFile.h,v
retrieving revision 2.3
diff -u -p -r2.3 LogFile.h
--- LogFile.h 10 Nov 2002 03:56:05 - 2.3
+++ LogFile.h 1 Feb 2003 20:48:07 -
@@ -23,6 +23,7 @@ public:
LogFile();
void clearFiles(); // delete all target filenames
void setFile (int minlevel, String const path, bool append);
+ String getFile (int minlevel) const;
/* Some platforms don't call destructors. So this call exists
* which guarantees to flush any log data...
* but doesn't call generic C++ destructors
Index: res.rc
===
RCS file: /home/max/cvsmirror/cygwin-apps-cvs/setup/res.rc,v
retrieving revision 2.45
diff -u -p -r2.45 res.rc
--- res.rc 19 Jan 2003 20:31:53 - 2.45
+++ res.rc 1 Feb 2003 21:14:55 -
@@ -483,7 +483,8 @@ BEGIN
IDS_DOWNLOAD_FAILED Unable to download %s
IDS_DOWNLOAD_INCOMPLETE Download Incomplete. Try again?
IDS_INSTALL_ERROR Installation error (%s), Continue with other
packages?
-IDS_INSTALL_INCOMPLETE Installation incomplete. Check
/setup.log.full for details
+IDS_INSTALL_INCOMPLETE Installation incomplete. Check %s for
details
+IDS_INSTALL_INCOMPLETE_NO_LOG Installation incomplete. No log was in
use
IDS_VERSION_INFOSetup.exe version %1
IDS_CYGWIN_SETUPCygwin Setup
IDS_CYGWIN_SETUP_WITH_PROGRESS %1!d!%% - Cygwin Setup
Index: resource.h
===
RCS file: /home/max/cvsmirror/cygwin-apps-cvs/setup/resource.h,v
retrieving revision 2.20
diff -u -p -r2.20 resource.h
--- resource.h 19 Jan 2003 20:31:53 - 2.20
+++ resource.h 1 Feb 2003 21:14:00 -
@@ -30,6 +30,7 @@
#define IDS_VERSION_INFO28
#define IDS_CYGWIN_SETUP29
#define IDS_CYGWIN_SETUP_WITH_PROGRESS 30
+#define IDS_INSTALL_INCOMPLETE_NO_LOG 31
#define IDD_ROOT101
#define IDD_SOURCE 102
#define IDD_OTHER_URL 103
Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
Max Bowsher wrote:
Robert Collins wrote:
getFileName doesn't take a minlevel, it takes an exactLevel - your
parameter name is misleading, or you've got the logic in the loop
wrong :}.
Or, you could say that it gets the file whose minlevel is what you
request. In which case its correct :-)
IDS_MISSING_LOG should be No log was in use, not setup.log.full.
It'll be ugly in the output, but at least accurate.
Check No log was in use for details.
That's worse than what we have in CVS now.
How about:
Or maybe not, since setup just terminated abnormally when I forced getFile
to always return an empty string.
Not immediately sure where that crash came from, and I've run out of time to
spend in front of the computer for today.
Anyway, you get the idea of what I was trying to do:
Index: LogFile.cc
===
+ if (exit_msg == IDS_INSTALL_INCOMPLETE)
+{
+ tempString = backslash(getFile(LOG_BABBLE));
+ if (tempString.size() == 0)
+exit_msg = IDS_INSTALL_INCOMPLETE_NO_LOG;
+}
Max.
Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
On Sun, 2003-02-02 at 08:22, Max Bowsher wrote: Robert Collins wrote: getFileName doesn't take a minlevel, it takes an exactLevel - your parameter name is misleading, or you've got the logic in the loop wrong :}. Or, you could say that it gets the file whose minlevel is what you request. In which case its correct :-) Huh? min stands for minimum. In the log context that makes sense, as you are passed different levels of log entries. but for a search through the log files? It is misleading on first read, and thats the point. perhaps 'minLevelOfLogFile' would be better. IDS_MISSING_LOG should be No log was in use, not setup.log.full. It'll be ugly in the output, but at least accurate. Check No log was in use for details. That's worse than what we have in CVS now. No it's not, because the current setup code *always* uses a log file of the appropriate level. It's a fallback to catch future changes. How about: No. I like Igors approach better. For yours, we end up with duplicate resources : you should split the 'installation incomplete' and the log to check into two separate messages. Secondly, you've still got getFile unrenamed. Let me clarify this point: we have a structure for each file we are logging to. getFile should return one of those structures - not the name of the file. However, as we only care about the name today, getFileName will both not collide with getFile, and be clear what it does from the header. Lastly we should be looking at how we pull conditionals out of LogFile, not inserting them. Rob -- GPG key available at: http://users.bigpond.net.au/robertc/keys.txt. signature.asc Description: This is a digitally signed message part
Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
Robert Collins wrote: On Sun, 2003-02-02 at 08:22, Max Bowsher wrote: Robert Collins wrote: IDS_MISSING_LOG should be No log was in use, not setup.log.full. It'll be ugly in the output, but at least accurate. Check No log was in use for details. That's worse than what we have in CVS now. No it's not, because the current setup code *always* uses a log file of the appropriate level. It's a fallback to catch future changes. Well, at least make it no log was in use (with the brackets) to make the message easier to read. How about: No. I like Igors approach better. For yours, we end up with duplicate resources : you should split the 'installation incomplete' and the log to check into two separate messages. Then you have to tangle with temporary buffers to combine them. Secondly, you've still got getFile unrenamed. Let me clarify this point: we have a structure for each file we are logging to. getFile should return one of those structures - not the name of the file. However, as we only care about the name today, getFileName will both not collide with getFile, and be clear what it does from the header. Sure, I wasn't attempting to address this issue. Lastly we should be looking at how we pull conditionals out of LogFile, not inserting them. I can't see how else to acheive the result I intended without major restructuring. Max.
Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
On 2 Feb 2003, Robert Collins wrote:
On Sun, 2003-02-02 at 07:37, Igor Pechtchanski wrote:
Yes, it does recompile everything when resource.h is modified, but that
only happens once. A necessary evil...
Even so... resource.h urrghh.
Sorry about the (null) bit. Should be fixed now. I also took your
suggestion regarding log file. The third iteration of the patch is
attached...
Igor
ChangeLog:
2002-10-17 Igor Pechtchanski [EMAIL PROTECTED]
* res.rc (IDS_INSTALL_INCOMPLETE): Change hard-coded
log filename to %s.
(IDS_MISSING_LOG): New string resource.
* resource.h (IDS_MISSING_LOG): New resource.
* LogFile.cc (LogFile::exit): Pass log filename for
LOG_BABBLE to note().
(LogFile::getFile): New function.
* LogFile.h (LogFile::getFile): New function.
getFile should be getFilename or getFileName.
getFileName doesn't take a minlevel, it takes an exactLevel - your
parameter name is misleading, or you've got the logic in the loop wrong
:}.
IDS_MISSING_LOG should be No log was in use, not setup.log.full.
It'll be ugly in the output, but at least accurate.
Do those three changes, and I think this is ready. nice work guys.
Rob
Rob,
I was mislead by the setFile() prototype. I agree with your proposed
changes (and Max's use of angle brackets). Take four is attached.
Thanks.
Igor
==
ChangeLog:
2003-02-01 Igor Pechtchanski [EMAIL PROTECTED]
* res.rc (IDS_INSTALL_INCOMPLETE): Change hard-coded
log filename to %s.
(IDS_MISSING_LOG): New string resource.
* resource.h (IDS_MISSING_LOG): New resource.
* LogFile.cc (LogFile::exit): Pass log filename for
LOG_BABBLE to note().
(LogFile::getFileName): New function.
* LogFile.h (LogFile::getFileName): New function.
--
http://cs.nyu.edu/~pechtcha/
|\ _,,,---,,_[EMAIL PROTECTED]
ZZZzz /,`.-'`'-. ;-;;,_[EMAIL PROTECTED]
|,4- ) )-,_. ,\ ( `'-' Igor Pechtchanski
'---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow!
Oh, boy, virtual memory! Now I'm gonna make myself a really *big* RAMdisk!
-- /usr/games/fortune
Index: LogFile.cc
===
RCS file: /cvs/cygwin-apps/setup/LogFile.cc,v
retrieving revision 2.6
diff -u -p -r2.6 LogFile.cc
--- LogFile.cc 25 Nov 2002 00:41:24 - 2.6
+++ LogFile.cc 1 Feb 2003 23:11:30 -
@@ -31,6 +31,8 @@ static const char *cvsid =
#include time.h
#include string
#include AntiVirus.h
+#include filemanip.h
+#include cistring.h
using namespace std;
@@ -101,6 +103,20 @@ LogFile::setFile (int minlevel, String c
files.insert (t);
}
+String
+LogFile::getFileName (int level) const
+{
+ for (FileSet::iterator i = files.begin();
+ i != files.end(); ++i)
+{
+ if (i-level == level)
+return i-key;
+}
+ cistring bad_file;
+ bad_file.Format(IDS_MISSING_LOG);
+ return bad_file.c_str();
+}
+
void
LogFile::exit (int const exit_code)
{
@@ -115,7 +131,7 @@ LogFile::exit (int const exit_code)
been_here = 1;
if (exit_msg)
-note (NULL, exit_msg);
+note (NULL, exit_msg, backslash(getFileName(LOG_BABBLE)).cstr_oneuse());
log (LOG_TIMESTAMP) Ending cygwin install endLog;
Index: LogFile.h
===
RCS file: /cvs/cygwin-apps/setup/LogFile.h,v
retrieving revision 2.3
diff -u -p -r2.3 LogFile.h
--- LogFile.h 10 Nov 2002 03:56:05 - 2.3
+++ LogFile.h 1 Feb 2003 23:11:30 -
@@ -23,6 +23,7 @@ public:
LogFile();
void clearFiles(); // delete all target filenames
void setFile (int minlevel, String const path, bool append);
+ String getFileName (int level) const;
/* Some platforms don't call destructors. So this call exists
* which guarantees to flush any log data...
* but doesn't call generic C++ destructors
Index: res.rc
===
RCS file: /cvs/cygwin-apps/setup/res.rc,v
retrieving revision 2.45
diff -u -p -r2.45 res.rc
--- res.rc 19 Jan 2003 20:31:53 - 2.45
+++ res.rc 1 Feb 2003 23:11:47 -
@@ -483,11 +483,12 @@ BEGIN
IDS_DOWNLOAD_FAILED Unable to download %s
IDS_DOWNLOAD_INCOMPLETE Download Incomplete. Try again?
IDS_INSTALL_ERROR Installation error (%s), Continue with other packages?
-IDS_INSTALL_INCOMPLETE Installation incomplete. Check /setup.log.full for
details
+IDS_INSTALL_INCOMPLETE Installation incomplete. Check %s for details
IDS_VERSION_INFOSetup.exe version %1
IDS_CYGWIN_SETUPCygwin Setup
IDS_CYGWIN_SETUP_WITH_PROGRESS
Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
I've sent this a couple of hours ago, but it didn't appear in the web archive, so I'm resending it. If the original does get through, apologies for the duplicate. On 2 Feb 2003, Robert Collins wrote: On Sun, 2003-02-02 at 07:37, Igor Pechtchanski wrote: [snip] Incidentally, there are two backslash() functions defined: one in filemanip.h, and another in concat.h. This is the only function defined in concat.h, and it doesn't seem to be used anywhere anymore. Is concat.h dead? Should it be removed? Ditto for concat.cc? Not sure, lets leave this for now. Rob FYI, I've built setup without concat.h and concat.cc, and it seems to be working fine. I'm attaching a patch that removes all references to concat.* files (most of the size is due to the generated Makefile.in changes). After applying the patch, you'll have to cvs rm concat.*, sorry (I don't know how to do this with patch). Igor ChangeLog: 2003-02-01 Igor Pechtchanski [EMAIL PROTECTED] * String++.cc: Don't include concat.h. * Makefile.am: Remove concat.cc and concat.h references. * Makefile.in: Regenerate. * concat.h: Remove. * concat.cc: Remove. -- http://cs.nyu.edu/~pechtcha/ |\ _,,,---,,_[EMAIL PROTECTED] ZZZzz /,`.-'`'-. ;-;;,_[EMAIL PROTECTED] |,4- ) )-,_. ,\ ( `'-' Igor Pechtchanski '---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow! Oh, boy, virtual memory! Now I'm gonna make myself a really *big* RAMdisk! -- /usr/games/fortune Index: String++.cc === RCS file: /cvs/cygwin-apps/setup/String++.cc,v retrieving revision 2.7 diff -u -p -r2.7 String++.cc --- String++.cc 10 Nov 2002 03:56:05 - 2.7 +++ String++.cc 1 Feb 2003 23:39:27 - @@ -18,7 +18,6 @@ #include String++.h #include string.h #include ctype.h -#include concat.h #include io_stream.h #include iostream #include sstream Index: Makefile.am === RCS file: /cvs/cygwin-apps/setup/Makefile.am,v retrieving revision 2.24 diff -u -p -r2.24 Makefile.am --- Makefile.am 25 Nov 2002 00:41:24 - 2.24 +++ Makefile.am 1 Feb 2003 23:39:26 - @@ -137,8 +137,6 @@ setup_SOURCES = \ compress_bz.h \ compress_gz.cc \ compress_gz.h \ - concat.cc \ - concat.h \ cygpackage.cc \ cygpackage.h \ desktop.cc \ Index: Makefile.in === RCS file: /cvs/cygwin-apps/setup/Makefile.in,v retrieving revision 2.86 diff -u -p -r2.86 Makefile.in --- Makefile.in 25 Nov 2002 00:41:24 - 2.86 +++ Makefile.in 1 Feb 2003 23:39:26 - @@ -276,8 +276,6 @@ setup_SOURCES = \ compress_bz.h \ compress_gz.cc \ compress_gz.h \ - concat.cc \ - concat.h \ cygpackage.cc \ cygpackage.h \ desktop.cc \ @@ -447,32 +445,31 @@ am__setup_SOURCES_DIST = AntiVirus.cc An archive_tar.cc archive_tar.h archive_tar_file.cc autoload.c \ category.cc category.h choose.cc choose.h cistring.cc \ cistring.h compress.cc compress.h compress_bz.cc compress_bz.h \ - compress_gz.cc compress_gz.h concat.cc concat.h cygpackage.cc \ - cygpackage.h desktop.cc desktop.h dialog.cc dialog.h \ - diskfull.cc diskfull.h download.cc download.h Exception.cc \ - Exception.h find.cc find.h FindVisitor.cc FindVisitor.h \ - filemanip.cc filemanip.h fromcwd.cc geturl.cc geturl.h hash.cc \ - hash.h ini.cc ini.h IniDBBuilder.cc IniDBBuilder.h \ - IniDBBuilderPackage.cc IniDBBuilderPackage.h inilex.cc \ - iniparse.cc iniparse.h IniParseFeedback.cc IniParseFeedback.h \ - IniParseFindVisitor.cc IniParseFindVisitor.h install.cc \ - io_stream.cc io_stream.h io_stream_cygfile.cc \ - io_stream_cygfile.h io_stream_file.cc io_stream_file.h \ - io_stream_memory.cc io_stream_memory.h IOStreamProvider.h \ - localdir.cc localdir.h log.cc log.h LogFile.cc LogFile.h \ - LogSingleton.cc LogSingleton.h main.cc md5.c md5.h MD5++.cc \ - MD5++.h mkdir.cc mkdir.h mklink2.cc mklink2.h mount.cc mount.h \ - msg.cc msg.h net.cc net.h netio.cc netio.h nio-ie5.cc nio-ie5.h \ - nio-file.cc nio-file.h nio-ftp.cc nio-ftp.h nio-http.cc \ - nio-http.h package_db.cc package_db.h package_meta.cc \ - package_meta.h package_source.cc package_source.h \ - package_version.cc package_version.h PackageSpecification.cc \ - PackageSpecification.h PackageTrust.h PickCategoryLine.cc \ - PickCategoryLine.h PickLine.cc PickLine.h PickPackageLine.cc \ - PickPackageLine.h PickView.cc PickView.h port.h
RE: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full
No. I like Igors approach better. For yours, we end up with duplicate resources : you should split the 'installation incomplete' and the log to check into two separate messages. Then you have to tangle with temporary buffers to combine them. That should be trivial with the string classes, and if it's not, then that sounds like a call for a method or operator on cistring. Or a Grand Unified String Class, i.e. subsuming cistring into String. So many issues, so little time -- Gary R. Van Sickle Brewer. Patriot.
