Re: [PATCH v2 02/34] sequencer (rebase -i): implement the 'noop' command
From: Junio C Hamano <hidden>
Date: 2016-12-13 20:43:26
Johannes Schindelin [off-list ref] 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. Makes me wish C were a better language, though ;-)
enum todo_command {
+ /* commands that handle commits */
TODO_PICK = 0,
- TODO_REVERT
+ TODO_REVERT,
+ /* commands that do nothing but are counted for reporting progress */
+ TODO_NOOP
};
static const char *todo_command_strings[] = {
"pick",
- "revert"
+ "revert",
+ "noop"
};quoted hunk
@@ -1292,7 +1316,12 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) struct todo_item *item = todo_list->items + todo_list->current; if (save_todo(todo_list, opts)) return -1; - res = do_pick_commit(item->command, item->commit, opts); + if (item->command <= TODO_REVERT) + res = do_pick_commit(item->command, item->commit, + opts); + else if (item->command != TODO_NOOP) + return error(_("unknown command %d"), item->command);
I wonder if making this a switch() statement is easier to read in the longer run. The only thing at this point we are gaining by "not only mapping and enum must match, the orders matter" is so that this codepath can do the same thing for PICK and REVERT, but these two would become more and more minority as we learn more words.
todo_list->current++; if (res) return res;