Hi,
On Tue, 13 Dec 2016, Junio C Hamano wrote:
> Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:
>
> > On Tue, Dec 13, 2016 at 12:38 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> >> Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> >>
> >>> +/*
> >>> + * Note that ordering matters in this enum. Not only must it match the mapping
> >>> + * below, it is also divided into several sections that matter. When adding
> >>> + * new commands, make sure you add it in the right section.
> >>> + */
> >>
> >> Good thinking.
Not my thinking... This was in direct response to a suggestion by Dennis
Kaarsemaker, I cannot take credit for the idea.
> Makes me wish C were a better language, though ;-)
> >
> > Do this:
> >
> > static const char *todo_command_strings[] = {
> > [TODO_PICK] = "pick",
> > [TODO_REVERT] = "revert",
> > [TODO_NOOP] = "noop:,
> > };
> >
> > which makes the array be order-independent. You still need to make
> > sure you fill in all the entries, of course, but it tends to avoid at
> > least one gotcha, and it makes it more obvious how the two are tied
> > together.
>
> Yes, I know. But I do not think the variant of C we stick to is not
> new enough to have that.
Let me try to express this without double negation: our coding guidelines
state very explicitly that we do not use C99 initializers, and it also
explains why: we want to support a broader range of compilers. For
details, see:
https://github.com/git/git/blob/v2.11.0/Documentation/CodingGuidelines#L179-L181
TBH I briefly considered going the same route as I did for the fsck
warnings (taking a lot of heat for the ugliness):
https://github.com/git/git/blob/v2.11.0/fsck.c#L17-L85
It would have looked somewhat like this:
#define FOREACH_TODO(FUNC) \
FUNC(PICK, 'p', "pick") \
FUNC(REVERT, 0, "revert") \
FUNC(EDIT, 'e', "edit") \
...
FUNC(COMMENT, 0, NULL)
#define TODO(id, short, long) TODO_##id,
enum todo_command {
FOREACH_TODO(TODO)
TODO_MAX
};
#undef TODO
#define TODO(id, short, long) { short, long },
static struct {
char c;
const char *str;
} todo_command_info[] = {
FOREACH_TODO(TODO)
{ 0, NULL }
};
#undef TODO
I have to admit that even I prefer the version I provided in my
contribution over the "fsck method" outlined above.
Ciao,
Dscho