Subject: Re: [PATCH 2/3] sequencer: make commit options more extensible

From: Junio C Hamano <[email-masked]>
Date: Wed, 22 Mar 2017 11:21:28 -0700

Johannes Schindelin <[email-masked]> writes:

> +#define ALLOW_EMPTY (1<<0)
> +#define EDIT_MSG    (1<<1)
> +#define AMEND_MSG   (1<<2)
> +#define CLEANUP_MSG (1<<3)

These being bits makes it clear that they can be independently set
and unset.

> @@ -615,8 +620,7 @@ N_("you have staged changes in your working tree\n"
>   * author metadata.
>   */
>  static int run_git_commit(const char *defmsg, struct replay_opts *opts,
> -			  int allow_empty, int edit, int amend,
> -			  int cleanup_commit_message)
> +			  int flags)

Use "unsigned" not signed integer for a collection of bits (see
e.g. GSoC microproject ideas page).

> @@ -1123,11 +1127,11 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
>  	if (allow < 0) {
>  		res = allow;
>  		goto leave;
> -	}
> +	} else if (allow)
> +		flags |= ALLOW_EMPTY;

;-)  Much more descriptive than just "allow".

>  static int commit_staged_changes(struct replay_opts *opts)
>  {
> -	int amend = 0;
> +	int flags = ALLOW_EMPTY | EDIT_MSG;
>  
>  	if (has_unstaged_changes(1))
>  		return error(_("cannot rebase: You have unstaged changes."));
> @@ -2184,10 +2188,10 @@ static int commit_staged_changes(struct replay_opts *opts)
>  				       "--continue' again."));
>  
>  		strbuf_release(&rev);
> -		amend = 1;
> +		flags |= AMEND_MSG;
>  	}
>  
> -	if (run_git_commit(rebase_path_message(), opts, 1, 1, amend, 0))
> +	if (run_git_commit(rebase_path_message(), opts, flags))

OK, the initialization of "flags" corresponds to these "1, 1" in the
original.

Overall, much easier to understand (and to extend).  Good
maintaintability clean-up.

Thanks.