Re: [PATCH 2/3] sequencer: make commit options more extensible
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 22, 2017, 18:21 UTC
- Message-ID
- <xmqqa88dkw9j.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <c0c836e1f1739af25ad875419ee6125b53ac6a1a.1490194846.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> 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.
Show 7 quoted lines
> @@ -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).
Show 7 quoted lines
> @@ -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".
Show 17 quoted lines
> 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.