Hi Junio,

On Mon, 22 Dec 2014, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Mon, 22 Dec 2014, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >> 
> >> >> In other words, at some point wouldn't we be better off with
> >> >> something like this
> >> >> 
> >> >> 	struct {
> >> >>         	enum id;
> >> >>                 const char *id_string;
> >> >>                 enum error_level { FSCK_PASS, FSCK_WARN, FSCK_ERROR };
> >> >> 	} possible_fsck_errors[];
> >> >
> >> > I considered that, and Michael Haggerty also suggested that in a private
> >> > mail. However, I find that there is a clear hierarchy in the default
> >> > messages: fatal errors, errors, warnings and infos.
> >> 
> >> I am glad I am not alone ;-)
> >> ...
> > Oh, but please understand that this hierarchy only applies to the default
> > settings. All of these settings can be overridden individually – and the
> > first override will initialize a full array with the default settings.
> 
> But that means that the runtime needs to switch between two code
> with and without override, no?
> 
> > 	if (options->strict_mode)
> > 		return options->strict_mode[msg_id];
> 
> In other words, I think this is misleading and unnecessary
> optimization for the "full array" allocation.  A code that uses an
> array of a struct like the above that Michael and I independently
> suggested would initialize once with or without an override and then
> at the runtime there is no "if the array is there use it"
> conditional.
> 
> I do not know why Michael suggested the same thing, but the reason
> why I prefer that arrangement is because I think it would be easier
> to read and maintain.

Well, I disagree that it would be easier to maintain, because it appears
to me that the clear hierarchy keeps things simple. For example if some
clearly fatal error is clustered with non-fatal ones due to alphabetical
ordering, it is much harder to spot when it is marked as a demoteable
error by mistake.

For example, try to spot the error here:

	...
	F(ALMOST_HAPPY, INFO) \
	F(CANNOT_RECOVER, ERROR) \
	F(COFFEE_IS_EMPTY, WARN) \
	F(JUST_BEING_CHATTY, INFO) \
	F(LIFE_IS_GOOD, INFO) \
	F(MISSING_SOMETHING_VITAL, FATAL_ERROR) \
	F(NEED_TO_SLEEP, WARN) \
	F(SOMETHING_WENT_WRONG, ERROR) \
	...

Personally, I find it very, very hard to spot that CANNOT_RECOVER is
marked as a mere ERROR instead of a FATAL_ERROR. Even if it is nicely
alphabetically ordered.

I will sleep over this, though. Maybe I can come up with a solution that
makes all three of us happy.

Ciao,
Dscho