Subject: Re: [PATCH v2 02/34] sequencer (rebase -i): implement the 'noop' command

From: Junio C Hamano <[email-masked]>
Date: Mon, 19 Dec 2016 10:31:32 -0800

Johannes Schindelin <[email-masked]> writes:

> On Tue, 13 Dec 2016, Junio C Hamano wrote:
>
>> Linus Torvalds <[email-masked]> writes:
>> 
>> > On Tue, Dec 13, 2016 at 12:38 PM, Junio C Hamano <[email-masked]> wrote:
>> >> Johannes Schindelin <[email-masked]> 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.

I now realize that I was unclear about what "thinking" I found good
in my comment.  I do not particularly like defining two parallel
things and having to maintain them in sync.  The "Good thinking"
praise goes to whoever thought that this burdensome fact deserves a
clear comment in front of these two things.

And ...

>> 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.

... solves only one-half of the problem with the language I had.
The order of the entries in this array[] may become more flexible
in the source, but you still have to define enum separately.

I guess if we really want to, we need to resort to something "ugly
but workable" like what you did in fsck.c with FOREACH_MSG_ID(X).
That approach may be the least ugly way if we have to maintain two
or more parallel things in sync.

    ... and then realizes you wrote pretty much the same thing
    ... after writing all of the above ;-)

But it is way overkill for sequencer commands that are only handful.