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

[PATCH] commit: replace rebase/sequence booleans with single pick_state enum

From
BGBen Curtis via GitGitGadget <gitgitgadget@gmail.com>
Date
Jan 17, 2020, 13:45 UTC
Message-ID
<pull.531.git.1579268705473.gitgitgadget@gmail.com>
From: Ben Curtis <nospam@nowsci.com>

In 116a408, the boolean `rebase_in_progress` was introduced by dscho to handle instances when cherry-pick and rebase were occuring at the same time. This created a situation where two independent booleans were being used to define the state of git at a point in time.

Under his recommendation to follow guidance from Junio, specifically `https://public-inbox.org/git/xmqqr234i2q0.fsf@gitster-ct.c.googlers.com/`, it was decided that an `enum` that defines the state of git would be a more effective path forward.

Tasks completed:
  - Remove multiple booleans `rebase_in_progress` and `sequencer_in_use` and
replaced with a single `pick_state` enum that determines if, when
cherry-picking, we are in a rebase, multi-pick, or single-pick state
  - Converted double `if` statement to `if/else if` prioritizing `REBASE` to
continue to disallow cherry pick in rebase.
Signed-off-by: Ben Curtis <nospam@nowsci.com>
---
    commit: replaced rebase/sequence booleans with single pick_state enum
    
    Addresses https://github.com/gitgitgadget/git/issues/426
    
    Previous discussions from @dscho and Junio led to the decision to merge
    two independent booleans into a single enum to track the state of git 
    during a cherry-pick leading to this PR/patch.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-531%2FFmstrat%2Fjs%2Fadvise-rebase-skip-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-531/Fmstrat/js/advise-rebase-skip-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/531
 builtin/commit.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 2beae13620..84f7e69cb1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -125,7 +125,11 @@ static enum commit_msg_cleanup_mode cleanup_mode;
 static const char *cleanup_arg;
 
 static enum commit_whence whence;
-static int sequencer_in_use, rebase_in_progress;
+static enum {
+	SINGLE_PICK,
+	MULTI_PICK,
+	REBASE
+} pick_state;
 static int use_editor = 1, include_status = 1;
 static int have_option_m;
 static struct strbuf message = STRBUF_INIT;
@@ -184,10 +188,12 @@ static void determine_whence(struct wt_status *s)
 		whence = FROM_MERGE;
 	else if (file_exists(git_path_cherry_pick_head(the_repository))) {
 		whence = FROM_CHERRY_PICK;
-		if (file_exists(git_path_seq_dir()))
-			sequencer_in_use = 1;
 		if (file_exists(git_path_rebase_merge_dir()))
-			rebase_in_progress = 1;
+			pick_state = REBASE;
+		else if (file_exists(git_path_seq_dir()))
+			pick_state = MULTI_PICK;
+		else
+			pick_state = SINGLE_PICK;
 	}
 	else
 		whence = FROM_COMMIT;
@@ -459,7 +465,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
 		if (whence == FROM_MERGE)
 			die(_("cannot do a partial commit during a merge."));
 		else if (whence == FROM_CHERRY_PICK) {
-			if (rebase_in_progress && !sequencer_in_use)
+			if (pick_state == REBASE)
 				die(_("cannot do a partial commit during a rebase."));
 			die(_("cannot do a partial commit during a cherry-pick."));
 		}
@@ -958,9 +964,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 			fputs(_(empty_amend_advice), stderr);
 		else if (whence == FROM_CHERRY_PICK) {
 			fputs(_(empty_cherry_pick_advice), stderr);
-			if (sequencer_in_use)
+			if (pick_state == MULTI_PICK)
 				fputs(_(empty_cherry_pick_advice_multi), stderr);
-			else if (rebase_in_progress)
+			else if (pick_state == REBASE)
 				fputs(_(empty_rebase_advice), stderr);
 			else
 				fputs(_(empty_cherry_pick_advice_single), stderr);
@@ -1167,7 +1173,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		if (whence == FROM_MERGE)
 			die(_("You are in the middle of a merge -- cannot amend."));
 		else if (whence == FROM_CHERRY_PICK) {
-			if (rebase_in_progress && !sequencer_in_use)
+			if (pick_state == REBASE)
 				die(_("You are in the middle of a rebase -- cannot amend."));
 			die(_("You are in the middle of a cherry-pick -- cannot amend."));
 		}
@@ -1643,7 +1649,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		if (!reflog_msg)
 			reflog_msg = (whence != FROM_CHERRY_PICK)
 					? "commit"
-					: rebase_in_progress && !sequencer_in_use
+					: pick_state == REBASE
 					? "commit (rebase)"
 					: "commit (cherry-pick)";
 		commit_list_insert(current_head, &parents);

base-commit: 116a408b6ffcb2496ebf10dfce1364a42e8f0b32
-- 
gitgitgadget
Next: Derrick Stolee
Message 1 of 7 in “commit: replace rebase/sequence booleans with single pick_state enum”
  1. commit: replace rebase/sequence booleans with single pick_state enumBen Curtis via GitGitGadget, Jan 17, 2020
  2. Derrick StoleeJan 17, 2020
  3. Ben CurtisJan 17, 2020
  4. Phillip WoodJan 17, 2020
  5. Ben CurtisJan 18, 2020
  6. Phillip WoodJan 20, 2020
  7. Johannes SchindelinJan 20, 2020

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.