diff mbox series

[v2,01/14] rebase: factor out checkout for up to date branch

Message ID 0e84d00572ed9a99d68ec856b779f550b2a934de.1638975482.git.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit ae42fa4c03ef0ffe36468f8918b3d404a277da50
Headers show
Series rebase: reset_head() related fixes and improvements | expand

Commit Message

Phillip Wood Dec. 8, 2021, 2:57 p.m. UTC
From: Phillip Wood <phillip.wood@dunelm.org.uk>

This code is heavily indented and it will be convenient later in the
series to have it in its own function.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 builtin/rebase.c | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

Comments

Junio C Hamano Dec. 9, 2021, 9:04 p.m. UTC | #1
"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> This code is heavily indented and it will be convenient later in the
> series to have it in its own function.

Looks good in the sense that it is straight code movement without
changing any behaviour that won't hurt.

Without a clear overall direction given in the cover letter, however,
it is hard to judge if "being convenient later in the series" is a
good thing in the first place, though.

> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  builtin/rebase.c | 33 +++++++++++++++++++--------------
>  1 file changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 34b4744e5f3..f5c37b7d4a5 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -812,6 +812,23 @@ static int rebase_config(const char *var, const char *value, void *data)
>  	return git_default_config(var, value, data);
>  }
>  
> +static int checkout_up_to_date(struct rebase_options *options)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +	int ret = 0;
> +
> +	strbuf_addf(&buf, "%s: checkout %s",
> +		    getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
> +		    options->switch_to);
> +	if (reset_head(the_repository, &options->orig_head, "checkout",
> +		       options->head_name, RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
> +		       NULL, buf.buf, DEFAULT_REFLOG_ACTION) < 0)
> +		ret = error(_("could not switch to %s"), options->switch_to);
> +	strbuf_release(&buf);
> +
> +	return ret;
> +}
> +
>  /*
>   * Determines whether the commits in from..to are linear, i.e. contain
>   * no merge commits. This function *expects* `from` to be an ancestor of
> @@ -1673,21 +1690,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  		if (!(options.flags & REBASE_FORCE)) {
>  			/* Lazily switch to the target branch if needed... */
>  			if (options.switch_to) {
> -				strbuf_reset(&buf);
> -				strbuf_addf(&buf, "%s: checkout %s",
> -					    getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
> -					    options.switch_to);
> -				if (reset_head(the_repository,
> -					       &options.orig_head, "checkout",
> -					       options.head_name,
> -					       RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
> -					       NULL, buf.buf,
> -					       DEFAULT_REFLOG_ACTION) < 0) {
> -					ret = error(_("could not switch to "
> -							"%s"),
> -						      options.switch_to);
> +				ret = checkout_up_to_date(&options);
> +				if (ret)
>  					goto cleanup;
> -				}
>  			}
>  
>  			if (!(options.flags & REBASE_NO_QUIET))
diff mbox series

Patch

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 34b4744e5f3..f5c37b7d4a5 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -812,6 +812,23 @@  static int rebase_config(const char *var, const char *value, void *data)
 	return git_default_config(var, value, data);
 }
 
+static int checkout_up_to_date(struct rebase_options *options)
+{
+	struct strbuf buf = STRBUF_INIT;
+	int ret = 0;
+
+	strbuf_addf(&buf, "%s: checkout %s",
+		    getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
+		    options->switch_to);
+	if (reset_head(the_repository, &options->orig_head, "checkout",
+		       options->head_name, RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
+		       NULL, buf.buf, DEFAULT_REFLOG_ACTION) < 0)
+		ret = error(_("could not switch to %s"), options->switch_to);
+	strbuf_release(&buf);
+
+	return ret;
+}
+
 /*
  * Determines whether the commits in from..to are linear, i.e. contain
  * no merge commits. This function *expects* `from` to be an ancestor of
@@ -1673,21 +1690,9 @@  int cmd_rebase(int argc, const char **argv, const char *prefix)
 		if (!(options.flags & REBASE_FORCE)) {
 			/* Lazily switch to the target branch if needed... */
 			if (options.switch_to) {
-				strbuf_reset(&buf);
-				strbuf_addf(&buf, "%s: checkout %s",
-					    getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
-					    options.switch_to);
-				if (reset_head(the_repository,
-					       &options.orig_head, "checkout",
-					       options.head_name,
-					       RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
-					       NULL, buf.buf,
-					       DEFAULT_REFLOG_ACTION) < 0) {
-					ret = error(_("could not switch to "
-							"%s"),
-						      options.switch_to);
+				ret = checkout_up_to_date(&options);
+				if (ret)
 					goto cleanup;
-				}
 			}
 
 			if (!(options.flags & REBASE_NO_QUIET))