From: Phillip Wood Date: Mon, 11 Aug 2025 14:42:48 GMT Subject: Re: [GSOC PATCH v2 1/2] environment: remove the global variable 'merge_log_config' Message-ID: <076c19ae-58fc-4823-9679-1d5fe6e46211@gmail.com> In-Reply-To: <3aa014ed46d14e31ea0c2f6b7631e7e4cbbd3943.1754868681.git.ayu.chandekar@gmail.com> Hi Ayush On 11/08/2025 00:45, Ayush Chandekar wrote: > The global variable 'merge_log_config', set via the "merge.log" or > "merge.summary" settings, is only used in 'cmd_fmt_merge_msg()' and > 'cmd_merge()' to adjust the 'shortlog_len' variable. > > Remove 'merge_log_config' globally and localize it in > 'cmd_fmt_merge_msg()' and 'cmd_merge()'. Set its value by passing it in > 'fmt_merge_msg_config()' by passing its pointer to the function via the > callback parameter. This looks like a good solution Thanks Phillip > This change is part of an ongoing effort to eliminate global variables, > improve modularity and help libify the codebase. > > Mentored-by: Christian Couder > Mentored-by: Ghanshyam Thakkar > Signed-off-by: Ayush Chandekar > --- > builtin/fmt-merge-msg.c | 3 ++- > builtin/merge.c | 3 ++- > environment.c | 1 - > fmt-merge-msg.c | 10 ++++++---- > fmt-merge-msg.h | 1 - > 5 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c > index 3b6aac2cf7..4b24de32fb 100644 > --- a/builtin/fmt-merge-msg.c > +++ b/builtin/fmt-merge-msg.c > @@ -19,6 +19,7 @@ int cmd_fmt_merge_msg(int argc, > const char *message = NULL; > char *into_name = NULL; > int shortlog_len = -1; > + int merge_log_config = -1; > struct option options[] = { > { > .type = OPTION_INTEGER, > @@ -53,7 +54,7 @@ int cmd_fmt_merge_msg(int argc, > int ret; > struct fmt_merge_msg_opts opts; > > - git_config(fmt_merge_msg_config, NULL); > + git_config(fmt_merge_msg_config, &merge_log_config); > argc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage, > 0); > if (argc > 0) > diff --git a/builtin/merge.c b/builtin/merge.c > index 18b22c0a26..c2089b5e6f 100644 > --- a/builtin/merge.c > +++ b/builtin/merge.c > @@ -1374,6 +1374,7 @@ int cmd_merge(int argc, > struct commit_list *remoteheads = NULL, *p; > void *branch_to_free; > int orig_argc = argc; > + int merge_log_config = -1; > > show_usage_with_options_if_asked(argc, argv, > builtin_merge_usage, builtin_merge_options); > @@ -1392,7 +1393,7 @@ int cmd_merge(int argc, > skip_prefix(branch, "refs/heads/", &branch); > > init_diff_ui_defaults(); > - git_config(git_merge_config, NULL); > + git_config(git_merge_config, &merge_log_config); > > if (!branch || is_null_oid(&head_oid)) > head_commit = NULL; > diff --git a/environment.c b/environment.c > index 7c2480b22e..6751aa5683 100644 > --- a/environment.c > +++ b/environment.c > @@ -66,7 +66,6 @@ int grafts_keep_true_parents; > int core_apply_sparse_checkout; > int core_sparse_checkout_cone; > int sparse_expect_files_outside_of_patterns; > -int merge_log_config = -1; > int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */ > unsigned long pack_size_limit_cfg; > int max_allowed_tree_depth = > diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c > index 40174efa3d..c9085edc40 100644 > --- a/fmt-merge-msg.c > +++ b/fmt-merge-msg.c > @@ -26,13 +26,15 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP; > int fmt_merge_msg_config(const char *key, const char *value, > const struct config_context *ctx, void *cb) > { > + int *merge_log_config = cb; > + > if (!strcmp(key, "merge.log") || !strcmp(key, "merge.summary")) { > int is_bool; > - merge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool); > - if (!is_bool && merge_log_config < 0) > + *merge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool); > + if (!is_bool && *merge_log_config < 0) > return error("%s: negative length %s", key, value); > - if (is_bool && merge_log_config) > - merge_log_config = DEFAULT_MERGE_LOG_LEN; > + if (is_bool && *merge_log_config) > + *merge_log_config = DEFAULT_MERGE_LOG_LEN; > } else if (!strcmp(key, "merge.branchdesc")) { > use_branch_desc = git_config_bool(key, value); > } else if (!strcmp(key, "merge.suppressdest")) { > diff --git a/fmt-merge-msg.h b/fmt-merge-msg.h > index 73ca3e4465..c066d83761 100644 > --- a/fmt-merge-msg.h > +++ b/fmt-merge-msg.h > @@ -12,7 +12,6 @@ struct fmt_merge_msg_opts { > const char *into_name; > }; > > -extern int merge_log_config; > int fmt_merge_msg_config(const char *key, const char *value, > const struct config_context *ctx, void *cb); > int fmt_merge_msg(struct strbuf *in, struct strbuf *out,