Re: [GSOC PATCH v2 1/2] environment: remove the global variable 'merge_log_config'

[Phillip Wood <phillip.wood123@xxxxxxxxx>]

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 <christian.couder@xxxxxxxxx>
> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@xxxxxxxxx>
> Signed-off-by: Ayush Chandekar <ayu.chandekar@xxxxxxxxx>
> ---
>   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,