Re: [PATCH] Re: [setup] Inaccurate message: See /setup.log.full

[Igor Pechtchanski <pechtcha at cs dot nyu dot edu>]

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 <pechtcha@cs.nyu.edu>
> >>
> >>         * 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 <pechtcha@cs.nyu.edu>

	* 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/
      |\      _,,,---,,_		pechtcha@cs.nyu.edu
ZZZzz /,`.-'`'    -.  ;-;;,_		igor@watson.ibm.com
     |,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

Attachment: setup-incomplete-message.patch
Description: Text document