Subject: Re: [PATCH v2 27/34] sequencer (rebase -i): differentiate between comments and 'noop'

From: Junio C Hamano <[email-masked]>
Date: Mon, 19 Dec 2016 11:04:08 -0800

Johannes Schindelin <[email-masked]> writes:

> In the upcoming patch, we will support rebase -i's progress
> reporting. The progress skips comments but counts 'noop's.
>
> Signed-off-by: Johannes Schindelin <[email-masked]>
> ---
>  sequencer.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)

> diff --git a/sequencer.c b/sequencer.c
> index 1f314b2743..63f6f25ced 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -770,7 +770,9 @@ enum todo_command {
>  	TODO_EXEC,
>  	/* commands that do nothing but are counted for reporting progress */
>  	TODO_NOOP,
> -	TODO_DROP
> +	TODO_DROP,
> +	/* comments (not counted for reporting progress) */
> +	TODO_COMMENT
>  };
>  
>  static struct {

Makes sense.  I would have done this immediately after introducing
NOOP if I were doing this series, if only because by having the
unchanging last element early in enum {} definition, we can avoid
having to deal with the "last element cannot have comma", but that
is not a big issue.

> @@ -785,12 +787,13 @@ static struct {
>  	{ 's', "squash" },
>  	{ 'x', "exec" },
>  	{ 0,   "noop" },
> -	{ 'd', "drop" }
> +	{ 'd', "drop" },
> +	{ 0,   NULL }
>  };
>  
>  static const char *command_to_string(const enum todo_command command)
>  {
> -	if ((size_t)command < ARRAY_SIZE(todo_command_info))
> +	if (command < TODO_COMMENT)
>  		return todo_command_info[command].str;
>  	die("Unknown command: %d", command);
>  }

The same comment as "instead of comparing with TODO_NOOP, you would
want is_noop()" applies to three instances of comparing with
TODO_COMMENT we can see in this patch, I think.

"is_counted()" perhaps?