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

Hi Junio,

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

Johannes Schindelin [off-list ref] writes:

quoted

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

I doubt that this is easier to read. There are essentially three
categories we are handling: exec, comments, and everything else. IMO the
current code is the easiest to understand.

Ciao,
Dscho