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 -0000 2.6
+++ LogFile.cc 1 Feb 2003 17:25:10 -0000
@@ -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 -0000 2.3
+++ LogFile.h 1 Feb 2003 17:25:10 -0000
@@ -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 -0000 2.45
+++ res.rc 1 Feb 2003 17:25:22 -0000
@@ -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_INFO "Setup.exe version %1"
IDS_CYGWIN_SETUP "Cygwin Setup"
IDS_CYGWIN_SETUP_WITH_PROGRESS "%1!d!%% - Cygwin Setup"
IDS_CORRUPT_PACKAGE "Package file %s has a corrupt local copy, please remove
and retry."
+ IDS_MISSING_LOG "log file"
END
#endif // English (U.S.) resources
Index: resource.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/resource.h,v
retrieving revision 2.20
diff -u -p -r2.20 resource.h
--- resource.h 19 Jan 2003 20:31:53 -0000 2.20
+++ resource.h 1 Feb 2003 17:25:22 -0000
@@ -120,6 +120,7 @@
#define IDC_DISABLE_AV 1067
#define IDC_LEAVE_AV 1068
#define IDC_CHOOSE_KEEP 1069
+#define IDS_MISSING_LOG 1070
#define IDC_STATIC -1
// Next default values for new objects
