All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ben Curtis via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Ben Curtis <nospam@nowsci.com>, Ben Curtis <nospam@nowsci.com>
Subject: [PATCH] commit: replace rebase/sequence booleans with single pick_state enum
Date: Fri, 17 Jan 2020 13:45:05 +0000	[thread overview]
Message-ID: <pull.531.git.1579268705473.gitgitgadget@gmail.com> (raw)

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

             reply	other threads:[~2020-01-17 13:45 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-17 13:45 Ben Curtis via GitGitGadget [this message]
2020-01-17 14:29 ` [PATCH] commit: replace rebase/sequence booleans with single pick_state enum Derrick Stolee
2020-01-17 15:46   ` Ben Curtis
2020-01-17 20:01 ` Phillip Wood
2020-01-18 16:34   ` Ben Curtis
2020-01-20 17:09     ` Phillip Wood
2020-01-20 21:11       ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.531.git.1579268705473.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=nospam@nowsci.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.