[02/11] worktree: extract copy_filtered_worktree_config()

Commit Message

Derrick Stolee Feb. 20, 2022, 5:54 p.m. UTC

From: Derrick Stolee <[e-mail]>

This logic was introduced by 5325591 (worktree: copy sparse-checkout
patterns and config on add, 2022-02-07), but some feedback came in that
the add_worktree() method was already too complex. It is better to
extract this logic into a helper method to reduce this complexity.

Reported-by: Eric Sunshine <[e-mail]>
Signed-off-by: Derrick Stolee <[e-mail]>
---
 builtin/worktree.c | 81 ++++++++++++++++++++++++----------------------
 1 file changed, 42 insertions(+), 39 deletions(-)

Comments

Junio C Hamano Feb. 20, 2022, 8:26 p.m. UTC | #1

"Derrick Stolee via GitGitGadget" <[e-mail]> writes:

> From: Derrick Stolee <[e-mail]>
>
> This logic was introduced by 5325591 (worktree: copy sparse-checkout
> patterns and config on add, 2022-02-07), but some feedback came in that
> the add_worktree() method was already too complex. It is better to
> extract this logic into a helper method to reduce this complexity.
>
> Reported-by: Eric Sunshine <[e-mail]>
> Signed-off-by: Derrick Stolee <[e-mail]>
> ---
>  builtin/worktree.c | 81 ++++++++++++++++++++++++----------------------
>  1 file changed, 42 insertions(+), 39 deletions(-)

Quite straight-forward.  Looking good.

Thanks.

Patch

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7c272078dc9..2771a6dc793 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -236,6 +236,46 @@  static void check_candidate_path(const char *path,
 		die(_("'%s' is a missing but already registered worktree;\nuse '%s -f' to override, or 'prune' or 'remove' to clear"), path, cmd);
 }
 
+static void copy_filtered_worktree_config(const char *worktree_git_dir)
+{
+	char *from_file = git_pathdup("config.worktree");
+	char *to_file = xstrfmt("%s/config.worktree", worktree_git_dir);
+
+	if (file_exists(from_file)) {
+		struct config_set cs = { { 0 } };
+		const char *core_worktree;
+		int bare;
+
+		if (safe_create_leading_directories(to_file) ||
+			copy_file(to_file, from_file, 0666)) {
+			error(_("failed to copy worktree config from '%s' to '%s'"),
+				from_file, to_file);
+			goto worktree_copy_cleanup;
+		}
+
+		git_configset_init(&cs);
+		git_configset_add_file(&cs, from_file);
+
+		if (!git_configset_get_bool(&cs, "core.bare", &bare) &&
+			bare &&
+			git_config_set_multivar_in_file_gently(
+				to_file, "core.bare", NULL, "true", 0))
+			error(_("failed to unset '%s' in '%s'"),
+				"core.bare", to_file);
+		if (!git_configset_get_value(&cs, "core.worktree", &core_worktree) &&
+			git_config_set_in_file_gently(to_file,
+							"core.worktree", NULL))
+			error(_("failed to unset '%s' in '%s'"),
+				"core.worktree", to_file);
+
+		git_configset_clear(&cs);
+	}
+
+worktree_copy_cleanup:
+	free(from_file);
+	free(to_file);
+}
+
 static int add_worktree(const char *path, const char *refname,
 			const struct add_opts *opts)
 {
@@ -360,45 +400,8 @@  static int add_worktree(const char *path, const char *refname,
 	 * values from the current worktree into the new one, that way the
 	 * new worktree behaves the same as this one.
 	 */
-	if (repository_format_worktree_config) {
-		char *from_file = git_pathdup("config.worktree");
-		char *to_file = xstrfmt("%s/config.worktree",
-					sb_repo.buf);
-
-		if (file_exists(from_file)) {
-			struct config_set cs = { { 0 } };
-			const char *core_worktree;
-			int bare;
-
-			if (safe_create_leading_directories(to_file) ||
-			    copy_file(to_file, from_file, 0666)) {
-				error(_("failed to copy worktree config from '%s' to '%s'"),
-				      from_file, to_file);
-				goto worktree_copy_cleanup;
-			}
-
-			git_configset_init(&cs);
-			git_configset_add_file(&cs, from_file);
-
-			if (!git_configset_get_bool(&cs, "core.bare", &bare) &&
-			    bare &&
-			    git_config_set_multivar_in_file_gently(
-					to_file, "core.bare", NULL, "true", 0))
-				error(_("failed to unset '%s' in '%s'"),
-				      "core.bare", to_file);
-			if (!git_configset_get_value(&cs, "core.worktree", &core_worktree) &&
-			    git_config_set_in_file_gently(to_file,
-							  "core.worktree", NULL))
-				error(_("failed to unset '%s' in '%s'"),
-				      "core.worktree", to_file);
-
-			git_configset_clear(&cs);
-		}
-
-worktree_copy_cleanup:
-		free(from_file);
-		free(to_file);
-	}
+	if (repository_format_worktree_config)
+		copy_filtered_worktree_config(sb_repo.buf);
 
 	strvec_pushf(&child_env, "%s=%s", GIT_DIR_ENVIRONMENT, sb_git.buf);
 	strvec_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);