git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Mar 23, 2017, 16:07 UTC
Message-ID
<75e2c4fe839602af19811ed3e251eb583ff6c8ad.1490285210.git.johannes.schindelin@gmx.de>
In-Reply-To
<cover.1490285210.git.johannes.schindelin@gmx.de>

It was pointed out during review of the sequencer-i patch series (which taught the sequencer to execute an interactive rebase) that it may be cumbersome to keep extending the signature of the run_git_commit() function whenever a new commit option is needed.

While that concern had merit, back then I was reluctant to change even more than was already asked for (which typically introduces regressions, this late in the review process, which is no fun for nobody).

Now, with fresh eyes, and with an actual need, is a good time to change the strategy from adding individual flag parameters to coalescing them into a single flags parameter.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 48 ++++++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 22 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 8183a83c1fa..ce05d61a2ae 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -602,6 +602,11 @@ N_("you have staged changes in your working tree\n"
 "\n"
 "  git rebase --continue\n");
 
+#define ALLOW_EMPTY (1<<0)
+#define EDIT_MSG    (1<<1)
+#define AMEND_MSG   (1<<2)
+#define CLEANUP_MSG (1<<3)
+
 /*
  * If we are cherry-pick, and if the merge did not result in
  * hand-editing, we will hit this commit and inherit the original
@@ -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)
+			  unsigned int flags)
 {
 	struct child_process cmd = CHILD_PROCESS_INIT;
 	const char *value;
@@ -624,7 +628,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
 	cmd.git_cmd = 1;
 
 	if (is_rebase_i(opts)) {
-		if (!edit) {
+		if (!(flags & EDIT_MSG)) {
 			cmd.stdout_to_stderr = 1;
 			cmd.err = -1;
 		}
@@ -640,7 +644,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
 	argv_array_push(&cmd.args, "commit");
 	argv_array_push(&cmd.args, "-n");
 
-	if (amend)
+	if ((flags & AMEND_MSG))
 		argv_array_push(&cmd.args, "--amend");
 	if (opts->gpg_sign)
 		argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
@@ -648,16 +652,16 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
 		argv_array_push(&cmd.args, "-s");
 	if (defmsg)
 		argv_array_pushl(&cmd.args, "-F", defmsg, NULL);
-	if (cleanup_commit_message)
+	if ((flags & CLEANUP_MSG))
 		argv_array_push(&cmd.args, "--cleanup=strip");
-	if (edit)
+	if ((flags & EDIT_MSG))
 		argv_array_push(&cmd.args, "-e");
-	else if (!cleanup_commit_message &&
+	else if (!(flags & CLEANUP_MSG) &&
 		 !opts->signoff && !opts->record_origin &&
 		 git_config_get_value("commit.cleanup", &value))
 		argv_array_push(&cmd.args, "--cleanup=verbatim");
 
-	if (allow_empty)
+	if ((flags & ALLOW_EMPTY))
 		argv_array_push(&cmd.args, "--allow-empty");
 
 	if (opts->allow_empty_message)
@@ -926,14 +930,14 @@ static void record_in_rewritten(struct object_id *oid,
 static int do_pick_commit(enum todo_command command, struct commit *commit,
 		struct replay_opts *opts, int final_fixup)
 {
-	int edit = opts->edit, cleanup_commit_message = 0;
-	const char *msg_file = edit ? NULL : git_path_merge_msg();
+	unsigned int flags = opts->edit ? EDIT_MSG : 0, allow = 0;
+	const char *msg_file = opts->edit ? NULL : git_path_merge_msg();
 	unsigned char head[20];
 	struct commit *base, *next, *parent;
 	const char *base_label, *next_label;
 	struct commit_message msg = { NULL, NULL, NULL, NULL };
 	struct strbuf msgbuf = STRBUF_INIT;
-	int res, unborn = 0, amend = 0, allow = 0;
+	int res, unborn = 0;
 
 	if (opts->no_commit) {
 		/*
@@ -991,7 +995,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 			opts);
 		if (res || command != TODO_REWORD)
 			goto leave;
-		edit = amend = 1;
+		flags |= EDIT_MSG | AMEND_MSG;
 		msg_file = NULL;
 		goto fast_forward_edit;
 	}
@@ -1046,15 +1050,15 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 	}
 
 	if (command == TODO_REWORD)
-		edit = 1;
+		flags |= EDIT_MSG;
 	else if (is_fixup(command)) {
 		if (update_squash_messages(command, commit, opts))
 			return -1;
-		amend = 1;
+		flags |= AMEND_MSG;
 		if (!final_fixup)
 			msg_file = rebase_path_squash_msg();
 		else if (file_exists(rebase_path_fixup_msg())) {
-			cleanup_commit_message = 1;
+			flags |= CLEANUP_MSG;
 			msg_file = rebase_path_fixup_msg();
 		} else {
 			const char *dest = git_path("SQUASH_MSG");
@@ -1064,7 +1068,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 					     rebase_path_squash_msg(), dest);
 			unlink(git_path("MERGE_MSG"));
 			msg_file = dest;
-			edit = 1;
+			flags |= EDIT_MSG;
 		}
 	}
 
@@ -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;
 	if (!opts->no_commit)
 fast_forward_edit:
-		res = run_git_commit(msg_file, opts, allow, edit, amend,
-				     cleanup_commit_message);
+		res = run_git_commit(msg_file, opts, flags);
 
 	if (!res && final_fixup) {
 		unlink(rebase_path_fixup_msg());
@@ -2154,7 +2158,7 @@ static int continue_single_pick(void)
 
 static int commit_staged_changes(struct replay_opts *opts)
 {
-	int amend = 0;
+	unsigned 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))
 		return error(_("could not commit staged changes."));
 	unlink(rebase_path_amend());
 	return 0;
-- 
2.12.1.windows.1
Previous: Johannes SchindelinNext: Junio C Hamano
Message 20 of 27 in “rebase -i (reword): call the commit-msg hook again”
  1. 0/3 rebase -i (reword): call the commit-msg hook againJohannes Schindelin, Mar 22, 2017
  2. 2/3 sequencer: make commit options more extensibleJohannes Schindelin, Mar 22, 2017
  3. Junio C HamanoMar 22, 2017
  4. Johannes SchindelinMar 23, 2017
  5. Junio C HamanoMar 23, 2017
  6. 1/3 t7504: document regression: reword no longer calls commit-msgJohannes Schindelin, Mar 22, 2017
  7. Sebastian SchuberthMar 22, 2017
  8. Johannes SchindelinMar 22, 2017
  9. Junio C HamanoMar 22, 2017
  10. Johannes SchindelinMar 23, 2017
  11. Sebastian SchuberthMar 23, 2017
  12. Johannes SchindelinMar 23, 2017
  13. Junio C HamanoMar 22, 2017
  14. Johannes SchindelinMar 23, 2017
  15. 3/3 sequencer: allow the commit-msg hooks to run during a `reword`Johannes Schindelin, Mar 22, 2017
  16. Junio C HamanoMar 22, 2017
  17. Johannes SchindelinMar 23, 2017
  18. 0/3 rebase -i (reword): call the commit-msg hook againJohannes Schindelin, Mar 23, 2017
  19. 1/3 t7504: document regression: reword no longer calls commit-msgJohannes Schindelin, Mar 23, 2017
  20. 2/3 sequencer: make commit options more extensibleJohannes Schindelin, Mar 23, 2017
  21. Junio C HamanoMar 24, 2017
  22. Johannes SchindelinMar 25, 2017
  23. Junio C HamanoMar 27, 2017
  24. Junio C HamanoMar 27, 2017
  25. Johannes SchindelinMar 29, 2017
  26. 3/3 sequencer: allow the commit-msg hooks to run during a `reword`Johannes Schindelin, Mar 23, 2017
  27. Junio C HamanoMar 23, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.