Johannes Schindelin [off-list ref] writes:
Hi Junio,
On Mon, 22 Dec 2014, Junio C Hamano wrote:
quoted
Johannes Schindelin [off-list ref] writes:
quoted
quoted
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.
Thanks.
Hi Junio,
On Mon, 22 Dec 2014, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
On Mon, 22 Dec 2014, Junio C Hamano wrote:
quoted
Johannes Schindelin [off-list ref] writes:
quoted
quoted
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?
quoted
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