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

Johannes Schindelin [off-list ref] writes:

On Tue, 13 Dec 2016, Junio C Hamano wrote:

quoted

Linus Torvalds [off-list ref] writes:

quoted

On Tue, Dec 13, 2016 at 12:38 PM, Junio C Hamano [off-list ref] wrote:

quoted

Johannes Schindelin [off-list ref] writes:

quoted

+/*
+ * 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 ...

quoted

Makes me wish C were a better language, though ;-)

quoted

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.