Hi Dennis,
On Thu, 1 Sep 2016, Dennis Kaarsemaker wrote:
> On wo, 2016-08-31 at 10:56 +0200, Johannes Schindelin wrote:
> > diff --git a/sequencer.c b/sequencer.c
> > index 51c2f76..4c902e5 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -763,7 +763,8 @@ enum todo_command {
> > TODO_SQUASH,
> > TODO_EXEC,
> > TODO_NOOP,
> > - TODO_DROP
> > + TODO_DROP,
> > + TODO_COMMENT
> > };
>
> (picking a random commit that touches this enum)
>
> In a few places you now make comparisons like "< TODO_NOOP", so I think
> it would be good to have a comment near the definition of this enum
> that says that ordering matters and why, so people don't attempt to add
> a new TODO_FOOBAR at the end.
True.
It does not seem that we have a precedent for that. The closest is what I
had in an early iteration of the fsck message IDs, and subsequently things
were refactored so that it is not the order, but a flag, that determines
what the command does.
Not sure how to do this elegantly. Maybe like this?
enum todo_command {
TODO_PICK_COMMANDS = 0,
TODO_PICK = TODO_PICK_COMMANDS,
TODO_SQUASH,
TODO_NON_PICK_COMMANDS,
TODO_EXEC = TODO_NON_PICK_COMMANDS,
TODO_NOOP_COMMANDS,
TODO_NOOP = TODO_NOOP_COMMANDS,
TODO_DROP
TODO_DROP,
TODO_LAST_COMMAND,
TODO_COMMENT = TODO_LAST_COMMAND
};
But that is so god-awful to read.
Still unsure,
Dscho
