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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes Schindelin <johannes.schindelin@xxxxxx> 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"
>  };

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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]