[GSOC PATCH 0/2] builtin/fmt-merge-msg: remove dependency on global variables and 'the_repository'

[GSOC PATCH 0/2] builtin/fmt-merge-msg: remove dependency on global variables and 'the_repository'

[Ayush Chandekar]

The aim of this patch series is to remove the definition '#define USE_THE_REPOSITORY_VARIABLE'
from "builtin/fmt-merge-msg.c" by removing global variable 'merge_log_config' and the global 
'the_repository'

This patch series contains two patches:

1 - Remove the global varaible 'merge_log_config' and add a function 'adjust_shortlog_len()'
    in fmt-merge-msg.{c,h} to replicate the variable's usage.

2 - Remove the dependency of 'the_repository' in "builtin/fmt-merge-msg.c", allowing the removal
    of the definition '#define USE_THE_REPOSITORY_VARIABLE'. Also add a test to make sure that
    "git fmt-merge-msg -h" can be called with repository being NULL.

Ayush Chandekar (2):
  environment: remove the global variable 'merge_log_config'
  builtin/fmt-merge-msg: stop depending on 'the_repository'

 builtin/fmt-merge-msg.c |  9 ++++-----
 builtin/merge.c         |  4 ++--
 environment.c           |  2 --
 fmt-merge-msg.c         | 30 ++++++++++++++++++++++--------
 fmt-merge-msg.h         |  3 ++-
 t/t1517-outside-repo.sh |  7 +++++++
 6 files changed, 37 insertions(+), 18 deletions(-)

-- 
2.49.0

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

[Ayush Chandekar]

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' and introduce a function
'adjust_shortlog_len()' in fmt-merge-msg.c to handle the 'shortlog_len'
variable.

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@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/fmt-merge-msg.c |  4 ++--
 builtin/merge.c         |  4 ++--
 environment.c           |  2 --
 fmt-merge-msg.c         | 30 ++++++++++++++++++++++--------
 fmt-merge-msg.h         |  3 ++-
 5 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 3b6aac2cf7..fed8163825 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -58,8 +58,8 @@ int cmd_fmt_merge_msg(int argc,
 			     0);
 	if (argc > 0)
 		usage_with_options(fmt_merge_msg_usage, options);
-	if (shortlog_len < 0)
-		shortlog_len = (merge_log_config > 0) ? merge_log_config : 0;
+
+	adjust_shortlog_len(the_repository, &shortlog_len);
 
 	if (inpath && strcmp(inpath, "-")) {
 		in = fopen(inpath, "r");
diff --git a/builtin/merge.c b/builtin/merge.c
index 18b22c0a26..e1cf2a6d63 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1403,8 +1403,8 @@ int cmd_merge(int argc,
 		parse_branch_merge_options(branch_mergeoptions);
 	argc = parse_options(argc, argv, prefix, builtin_merge_options,
 			builtin_merge_usage, 0);
-	if (shortlog_len < 0)
-		shortlog_len = (merge_log_config > 0) ? merge_log_config : 0;
+
+	adjust_shortlog_len(the_repository, &shortlog_len);
 
 	if (verbosity < 0 && show_progress == -1)
 		show_progress = 0;
diff --git a/environment.c b/environment.c
index 7c2480b22e..74c9838b31 100644
--- a/environment.c
+++ b/environment.c
@@ -20,7 +20,6 @@
 #include "repository.h"
 #include "config.h"
 #include "refs.h"
-#include "fmt-merge-msg.h"
 #include "commit.h"
 #include "strvec.h"
 #include "path.h"
@@ -66,7 +65,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..2ceaebb0ce 100644
--- a/fmt-merge-msg.c
+++ b/fmt-merge-msg.c
@@ -26,14 +26,7 @@ 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)
 {
-	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)
-			return error("%s: negative length %s", key, value);
-		if (is_bool && merge_log_config)
-			merge_log_config = DEFAULT_MERGE_LOG_LEN;
-	} else if (!strcmp(key, "merge.branchdesc")) {
+	if (!strcmp(key, "merge.branchdesc")) {
 		use_branch_desc = git_config_bool(key, value);
 	} else if (!strcmp(key, "merge.suppressdest")) {
 		if (!value)
@@ -645,6 +638,27 @@ static void find_merge_parents(struct merge_parents *result,
 	result->nr = j;
 }
 
+void adjust_shortlog_len(struct repository *r, int *shortlog_len)
+{
+	const char *keys[] = { "merge.log", "merge.summary", NULL};
+	
+	if (*shortlog_len >= 0)
+		return;
+
+	for (const char **key = keys; *key; ++key) {
+		int is_bool, value;
+		if (!repo_config_get_bool_or_int(r, *key, &is_bool, &value)) {
+			if (!is_bool && value < 0) {
+				error("%s: negative length %d", *key, value);
+				return;
+			}
+			*shortlog_len = (is_bool && value) ? DEFAULT_MERGE_LOG_LEN : value;
+			return;
+		}
+	}
+
+	*shortlog_len = 0;
+}
 
 int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 		  struct fmt_merge_msg_opts *opts)
diff --git a/fmt-merge-msg.h b/fmt-merge-msg.h
index 73ca3e4465..f54f00d26f 100644
--- a/fmt-merge-msg.h
+++ b/fmt-merge-msg.h
@@ -2,6 +2,7 @@
 #define FMT_MERGE_MSG_H
 
 #include "strbuf.h"
+#include "repository.h"
 
 #define DEFAULT_MERGE_LOG_LEN 20
 
@@ -12,9 +13,9 @@ 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);
+void adjust_shortlog_len(struct repository *r, int *shortlog_len);
 int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 		  struct fmt_merge_msg_opts *);
 
-- 
2.49.0

[GSOC PATCH 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'

[Ayush Chandekar]

Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global
'the_repository'. Replace all the occurrences of 'the_repository' with
'repo', where 'repo' is a pointer to 'struct repository' passed to the
function 'cmd_fmt_merge_msg()' and thus remove the definition '#define
USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that "git
fmt-merge-msg -h" can be called outside a repository.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/fmt-merge-msg.c | 7 +++----
 t/t1517-outside-repo.sh | 7 +++++++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index fed8163825..848498b8e6 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #include "builtin.h"
 #include "config.h"
 #include "fmt-merge-msg.h"
@@ -13,7 +12,7 @@ static const char * const fmt_merge_msg_usage[] = {
 int cmd_fmt_merge_msg(int argc,
 		      const char **argv,
 		      const char *prefix,
-		      struct repository *repo UNUSED)
+		      struct repository *repo)
 {
 	char *inpath = NULL;
 	const char *message = NULL;
@@ -53,13 +52,13 @@ int cmd_fmt_merge_msg(int argc,
 	int ret;
 	struct fmt_merge_msg_opts opts;
 
-	git_config(fmt_merge_msg_config, NULL);
 	argc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,
 			     0);
 	if (argc > 0)
 		usage_with_options(fmt_merge_msg_usage, options);
+	repo_config(repo, fmt_merge_msg_config, NULL);
 
-	adjust_shortlog_len(the_repository, &shortlog_len);
+	adjust_shortlog_len(repo, &shortlog_len);
 
 	if (inpath && strcmp(inpath, "-")) {
 		in = fopen(inpath, "r");
diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
index 8f59b867f2..4b4e645860 100755
--- a/t/t1517-outside-repo.sh
+++ b/t/t1517-outside-repo.sh
@@ -121,4 +121,11 @@ test_expect_success 'prune does not crash with -h' '
 	test_grep "[Uu]sage: git prune " usage
 '
 
+test_expect_success 'fmt-merge-msg does not crash with -h' '
+	test_expect_code 129 git fmt-merge-msg -h >usage &&
+	test_grep "[Uu]sage: git fmt-merge-msg " usage &&
+	test_expect_code 129 nongit git fmt-merge-msg -h >usage &&
+	test_grep "[Uu]sage: git fmt-merge-msg " usage
+'
+
 test_done
-- 
2.49.0

Re: [GSOC PATCH 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global
> 'the_repository'. Replace all the occurrences of 'the_repository' with
> 'repo', where 'repo' is a pointer to 'struct repository' passed to the
> function 'cmd_fmt_merge_msg()' and thus remove the definition '#define
> USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that "git
> fmt-merge-msg -h" can be called outside a repository.

This also moves the call to git_config()/repo_config() after
parse_options().

It generally is a bad idea to read command line options first and
then read the configuration (it is a bug if such a flow causes
values from configuration to overwrite values from command line).
THe current set of options and configuration variables may not
overlap, in which case such a questionable arrangement happen to be
without bug right now, but it would prevent future developers from
adding new options and configuration variables and make them
interact with each other in the most natural way.

In any case, the reason for this change of the order between config
and parse-options is not explained at all in the proposed log
message.

> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> ---
>  builtin/fmt-merge-msg.c | 7 +++----
>  t/t1517-outside-repo.sh | 7 +++++++
>  2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
> index fed8163825..848498b8e6 100644
> --- a/builtin/fmt-merge-msg.c
> +++ b/builtin/fmt-merge-msg.c
> @@ -1,4 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
>  #include "builtin.h"
>  #include "config.h"
>  #include "fmt-merge-msg.h"
> @@ -13,7 +12,7 @@ static const char * const fmt_merge_msg_usage[] = {
>  int cmd_fmt_merge_msg(int argc,
>  		      const char **argv,
>  		      const char *prefix,
> -		      struct repository *repo UNUSED)
> +		      struct repository *repo)
>  {
>  	char *inpath = NULL;
>  	const char *message = NULL;
> @@ -53,13 +52,13 @@ int cmd_fmt_merge_msg(int argc,
>  	int ret;
>  	struct fmt_merge_msg_opts opts;
>  
> -	git_config(fmt_merge_msg_config, NULL);
>  	argc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,
>  			     0);
>  	if (argc > 0)
>  		usage_with_options(fmt_merge_msg_usage, options);
> +	repo_config(repo, fmt_merge_msg_config, NULL);
>  
> -	adjust_shortlog_len(the_repository, &shortlog_len);
> +	adjust_shortlog_len(repo, &shortlog_len);
>  
>  	if (inpath && strcmp(inpath, "-")) {
>  		in = fopen(inpath, "r");
> diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
> index 8f59b867f2..4b4e645860 100755
> --- a/t/t1517-outside-repo.sh
> +++ b/t/t1517-outside-repo.sh
> @@ -121,4 +121,11 @@ test_expect_success 'prune does not crash with -h' '
>  	test_grep "[Uu]sage: git prune " usage
>  '
>  
> +test_expect_success 'fmt-merge-msg does not crash with -h' '
> +	test_expect_code 129 git fmt-merge-msg -h >usage &&
> +	test_grep "[Uu]sage: git fmt-merge-msg " usage &&
> +	test_expect_code 129 nongit git fmt-merge-msg -h >usage &&
> +	test_grep "[Uu]sage: git fmt-merge-msg " usage
> +'
> +
>  test_done

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

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> 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' and introduce a function
> 'adjust_shortlog_len()' in fmt-merge-msg.c to handle the 'shortlog_len'
> variable.
>
> This change is part of an ongoing effort to eliminate global variables,
> improve modularity and help libify the codebase.

And the downsides of this change are...?

One obvious behaviour change I can see can happen when you have an
invalid value set to merge.summary and run the command with command
line override with the "--log" option.  In the current code, the
config callback barfs when it notices an invalid merge.summary
setting, even though it won't be used because the valid value given
via the "--log" option would override it.  In the updated code,
adjust_shortlog_len() would short-circuit and does not even bother
reading from the configuration, so the user will not be notified of
a broken configuration.

It is not immediately obvious if this particular behaviour change is
a regression or an improvement, but it probably deserves to be noted
somewhere to help future developers what our thinking was.

> @@ -26,14 +26,7 @@ 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)
>  {
> -	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)
> -			return error("%s: negative length %s", key, value);
> -		if (is_bool && merge_log_config)
> -			merge_log_config = DEFAULT_MERGE_LOG_LEN;
> -	} else if (!strcmp(key, "merge.branchdesc")) {
> +	if (!strcmp(key, "merge.branchdesc")) {
>  		use_branch_desc = git_config_bool(key, value);
>  	} else if (!strcmp(key, "merge.suppressdest")) {
>  		if (!value)
> @@ -645,6 +638,27 @@ static void find_merge_parents(struct merge_parents *result,
>  	result->nr = j;
>  }
>  
> +void adjust_shortlog_len(struct repository *r, int *shortlog_len)
> +{
> +	const char *keys[] = { "merge.log", "merge.summary", NULL};
> +	
> +	if (*shortlog_len >= 0)
> +		return;
> +
> +	for (const char **key = keys; *key; ++key) {
> +		int is_bool, value;
> +		if (!repo_config_get_bool_or_int(r, *key, &is_bool, &value)) {
> +			if (!is_bool && value < 0) {
> +				error("%s: negative length %d", *key, value);
> +				return;
> +			}
> +			*shortlog_len = (is_bool && value) ? DEFAULT_MERGE_LOG_LEN : value;
> +			return;
> +		}
> +	}
> +
> +	*shortlog_len = 0;
> +}
>  
>  int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
>  		  struct fmt_merge_msg_opts *opts)
> diff --git a/fmt-merge-msg.h b/fmt-merge-msg.h
> index 73ca3e4465..f54f00d26f 100644
> --- a/fmt-merge-msg.h
> +++ b/fmt-merge-msg.h
> @@ -2,6 +2,7 @@
>  #define FMT_MERGE_MSG_H
>  
>  #include "strbuf.h"
> +#include "repository.h"
>  
>  #define DEFAULT_MERGE_LOG_LEN 20
>  
> @@ -12,9 +13,9 @@ 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);
> +void adjust_shortlog_len(struct repository *r, int *shortlog_len);
>  int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
>  		  struct fmt_merge_msg_opts *);

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

[Ayush Chandekar]

Hi Junio,

On Tue, Jul 29, 2025 at 10:18 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Ayush Chandekar <ayu.chandekar@gmail.com> writes:
>
> > 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' and introduce a function
> > 'adjust_shortlog_len()' in fmt-merge-msg.c to handle the 'shortlog_len'
> > variable.
> >
> > This change is part of an ongoing effort to eliminate global variables,
> > improve modularity and help libify the codebase.
>
> And the downsides of this change are...?
>
> One obvious behaviour change I can see can happen when you have an
> invalid value set to merge.summary and run the command with command
> line override with the "--log" option.  In the current code, the
> config callback barfs when it notices an invalid merge.summary
> setting, even though it won't be used because the valid value given
> via the "--log" option would override it.  In the updated code,
> adjust_shortlog_len() would short-circuit and does not even bother
> reading from the configuration, so the user will not be notified of
> a broken configuration.
>
> It is not immediately obvious if this particular behaviour change is
> a regression or an improvement, but it probably deserves to be noted
> somewhere to help future developers what our thinking was.

Oh right, I did not mention this in the commit message. I am not sure
if this behaviour is good or not.

Technically, if the user wants to use the "--log" option, they would
not care about the config. Whereas, if the user wants to use the
config, they would be notified in case of an invalid one.

I will mention this in the commit message, but do you think this
behaviour is fine?

Thanks
Ayush

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

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> I will mention this in the commit message, but do you think this
> behaviour is fine?

I cannot answer that one immediately after saying that I do not know
if this is a regression or an improvement.  

If you still pushed me to answer it immediately, you'd get my
default position, a conservative "any changes in behaviour caused by
an internal code clean-up is bad and is a serious regression" ;-).

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

[Phillip Wood]

Hi Ayush

On 29/07/2025 17:19, Ayush Chandekar wrote:
> 
> @@ -26,14 +26,7 @@ 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)
>   {
> -	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)
> -			return error("%s: negative length %s", key, value);
> -		if (is_bool && merge_log_config)
> -			merge_log_config = DEFAULT_MERGE_LOG_LEN;
> -	} else if (!strcmp(key, "merge.branchdesc")) {

In the old code if both "merge.log" and "merge.summary" are set in the 
config file the last one wins

> +void adjust_shortlog_len(struct repository *r, int *shortlog_len)
> +{
> +	const char *keys[] = { "merge.log", "merge.summary", NULL};
> +	
> +	if (*shortlog_len >= 0)
> +		return;
> +
> +	for (const char **key = keys; *key; ++key) {
> +		int is_bool, value;
> +		if (!repo_config_get_bool_or_int(r, *key, &is_bool, &value)) {
> +			if (!is_bool && value < 0) {
> +				error("%s: negative length %d", *key, value);
> +				return;
> +			}
> +			*shortlog_len = (is_bool && value) ? DEFAULT_MERGE_LOG_LEN : value;
> +			return;

In the new code "merge.log" is always used in preference to 
"merge.summary" even if "merge.summary" appears later in the config 
file. When you have two keys setting the same variable I think the only 
way to preserve the last one wins behavior is to keep using a callback 
that updates the value as the config files are parsed.

Thanks

Phillip

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

[Ayush Chandekar]

Hi Phillip,

On Wed, Jul 30, 2025 at 12:37 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Ayush
>
> On 29/07/2025 17:19, Ayush Chandekar wrote:
> >
> > @@ -26,14 +26,7 @@ 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)
> >   {
> > -     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)
> > -                     return error("%s: negative length %s", key, value);
> > -             if (is_bool && merge_log_config)
> > -                     merge_log_config = DEFAULT_MERGE_LOG_LEN;
> > -     } else if (!strcmp(key, "merge.branchdesc")) {
>
> In the old code if both "merge.log" and "merge.summary" are set in the
> config file the last one wins
>
> > +void adjust_shortlog_len(struct repository *r, int *shortlog_len)
> > +{
> > +     const char *keys[] = { "merge.log", "merge.summary", NULL};
> > +
> > +     if (*shortlog_len >= 0)
> > +             return;
> > +
> > +     for (const char **key = keys; *key; ++key) {
> > +             int is_bool, value;
> > +             if (!repo_config_get_bool_or_int(r, *key, &is_bool, &value)) {
> > +                     if (!is_bool && value < 0) {
> > +                             error("%s: negative length %d", *key, value);
> > +                             return;
> > +                     }
> > +                     *shortlog_len = (is_bool && value) ? DEFAULT_MERGE_LOG_LEN : value;
> > +                     return;
>
> In the new code "merge.log" is always used in preference to
> "merge.summary" even if "merge.summary" appears later in the config
> file. When you have two keys setting the same variable I think the only
> way to preserve the last one wins behavior is to keep using a callback
> that updates the value as the config files are parsed.
>

Sorry for not mentioning this in the commit message.

I had looked at the documentation which says:

Documentation/git-fmt-merge-msg.adoc
merge.summary::
Synonym to `merge.log`; this is deprecated and will be removed in
the future.

So I thought that I should give precedence to "merge.log" as
"merge.summary" is deprecated.

> Thanks
>
> Phillip
>

Thanks
Ayush

Re: [GSOC PATCH 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'

[Ayush Chandekar]

On Tue, Jul 29, 2025 at 10:11 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Ayush Chandekar <ayu.chandekar@gmail.com> writes:
>
> > Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global
> > 'the_repository'. Replace all the occurrences of 'the_repository' with
> > 'repo', where 'repo' is a pointer to 'struct repository' passed to the
> > function 'cmd_fmt_merge_msg()' and thus remove the definition '#define
> > USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that "git
> > fmt-merge-msg -h" can be called outside a repository.
>
> This also moves the call to git_config()/repo_config() after
> parse_options().
>
> It generally is a bad idea to read command line options first and
> then read the configuration (it is a bug if such a flow causes
> values from configuration to overwrite values from command line).
> THe current set of options and configuration variables may not
> overlap, in which case such a questionable arrangement happen to be
> without bug right now, but it would prevent future developers from
> adding new options and configuration variables and make them
> interact with each other in the most natural way.
>

I understand it, but how do we tackle if NULL repository is passed.

> In any case, the reason for this change of the order between config
> and parse-options is not explained at all in the proposed log
> message.
>

 Apologies, I will mention it.

Re: [GSOC PATCH 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

>> It generally is a bad idea to read command line options first and
>> then read the configuration (it is a bug if such a flow causes
>> values from configuration to overwrite values from command line).
>> THe current set of options and configuration variables may not
>> overlap, in which case such a questionable arrangement happen to be
>> without bug right now, but it would prevent future developers from
>> adding new options and configuration variables and make them
>> interact with each other in the most natural way.
>
> I understand it, but how do we tackle if NULL repository is passed.

Perhaps you want to study the problem space and related past changes
before going forward.  The first place to look at is what happens
when you call repo_config(), outside a repository or a working tree
and repository is NULL.

f29f1990 (config: teach repo_config to allow `repo` to be NULL,
2025-03-08) and what it calls "the following commits" may be
illuminating.

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

[Phillip Wood]

Hi Ayush

On 29/07/2025 22:16, Ayush Chandekar wrote:
>  
> On Wed, Jul 30, 2025 at 12:37 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> Hi Ayush
>>
>> On 29/07/2025 17:19, Ayush Chandekar wrote:
>>>
>>> @@ -26,14 +26,7 @@ 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)
>>>    {
>>> -     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)
>>> -                     return error("%s: negative length %s", key, value);
>>> -             if (is_bool && merge_log_config)
>>> -                     merge_log_config = DEFAULT_MERGE_LOG_LEN;
>>> -     } else if (!strcmp(key, "merge.branchdesc")) {
>>
>> In the old code if both "merge.log" and "merge.summary" are set in the
>> config file the last one wins
>>
>>> +void adjust_shortlog_len(struct repository *r, int *shortlog_len)
>>> +{
>>> +     const char *keys[] = { "merge.log", "merge.summary", NULL};
>>> +
>>> +     if (*shortlog_len >= 0)
>>> +             return;
>>> +
>>> +     for (const char **key = keys; *key; ++key) {
>>> +             int is_bool, value;
>>> +             if (!repo_config_get_bool_or_int(r, *key, &is_bool, &value)) {
>>> +                     if (!is_bool && value < 0) {
>>> +                             error("%s: negative length %d", *key, value);
>>> +                             return;
>>> +                     }
>>> +                     *shortlog_len = (is_bool && value) ? DEFAULT_MERGE_LOG_LEN : value;
>>> +                     return;
>>
>> In the new code "merge.log" is always used in preference to
>> "merge.summary" even if "merge.summary" appears later in the config
>> file. When you have two keys setting the same variable I think the only
>> way to preserve the last one wins behavior is to keep using a callback
>> that updates the value as the config files are parsed.
>>
> 
> Sorry for not mentioning this in the commit message.
> 
> I had looked at the documentation which says:
> 
> Documentation/git-fmt-merge-msg.adoc
> merge.summary::
> Synonym to `merge.log`; this is deprecated and will be removed in
> the future.
> 
> So I thought that I should give precedence to "merge.log" as
> "merge.summary" is deprecated.

If it is deprecated we still need to support it until it is removed 
(maybe we should do that in Git 3.0?). We cannot change the behavior 
just because a setting is deprecated.

Thanks

Phillip

>> Thanks
>>
>> Phillip
>>
> 
> Thanks
> Ayush
> 

Re: [GSOC PATCH 0/2] builtin/fmt-merge-msg: remove dependency on global variables and 'the_repository'

[Ayush Chandekar]

Just an update, I'm still working on this patch series.

Thanks,
Ayush

[GSOC PATCH v2 0/2] builtin/fmt-merge-msg: remove dependency on global variables and 'the_repository'

[Ayush Chandekar]

The aim of this patch series is to remove the definition '#define USE_THE_REPOSITORY_VARIABLE'
from "builtin/fmt-merge-msg.c" by removing global variable 'merge_log_config' and the global 
'the_repository'

This patch series contains two patches:

1 - Remove the global varaible 'merge_log_config' 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.

2 - Remove the dependency of 'the_repository' in "builtin/fmt-merge-msg.c", allowing the removal
    of the definition '#define USE_THE_REPOSITORY_VARIABLE'. Also add a test to make sure that
    "git fmt-merge-msg -h" can be called with repository being NULL.

Thanks to Junio and Phillip for reviewing my patch series and Christian for mentoring me!

Ayush Chandekar (2):
  environment: remove the global variable 'merge_log_config'
  builtin/fmt-merge-msg: stop depending on 'the_repository'

 builtin/fmt-merge-msg.c |  6 +++---
 builtin/merge.c         |  3 ++-
 environment.c           |  1 -
 fmt-merge-msg.c         | 10 ++++++----
 fmt-merge-msg.h         |  1 -
 t/t1517-outside-repo.sh |  7 +++++++
 6 files changed, 18 insertions(+), 10 deletions(-)

Range-diff against v1:
1:  c82620a1f5 < -:  ---------- environment: remove the global variable 'merge_log_config'
-:  ---------- > 1:  3aa014ed46 environment: remove the global variable 'merge_log_config'
2:  04d6f682a6 ! 2:  8e55516cda builtin/fmt-merge-msg: stop depending on 'the_repository'
    @@ Commit message
         builtin/fmt-merge-msg: stop depending on 'the_repository'
     
         Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global
    -    'the_repository'. Replace all the occurrences of 'the_repository' with
    -    'repo', where 'repo' is a pointer to 'struct repository' passed to the
    -    function 'cmd_fmt_merge_msg()' and thus remove the definition '#define
    -    USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that "git
    -    fmt-merge-msg -h" can be called outside a repository.
    +    'the_repository'. Remove the 'UNUSED' macro from the 'struct repository'
    +    parameter and replace 'git_config()' with 'repo_config()' so that
    +    configuration is read from the passed repository. Also, add a test to
    +    make sure that "git fmt-merge-msg -h" can be called outside a
    +    repository.
     
         Mentored-by: Christian Couder <christian.couder@gmail.com>
         Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
    @@ builtin/fmt-merge-msg.c: 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);
    ++	repo_config(repo, fmt_merge_msg_config, &merge_log_config);
      	argc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,
      			     0);
      	if (argc > 0)
    - 		usage_with_options(fmt_merge_msg_usage, options);
    -+	repo_config(repo, fmt_merge_msg_config, NULL);
    - 
    --	adjust_shortlog_len(the_repository, &shortlog_len);
    -+	adjust_shortlog_len(repo, &shortlog_len);
    - 
    - 	if (inpath && strcmp(inpath, "-")) {
    - 		in = fopen(inpath, "r");
     
      ## t/t1517-outside-repo.sh ##
     @@ t/t1517-outside-repo.sh: test_expect_success 'prune does not crash with -h' '

-- 
2.49.0

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

[Ayush Chandekar]

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 change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

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

[GSOC PATCH v2 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'

[Ayush Chandekar]

Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global
'the_repository'. Remove the 'UNUSED' macro from the 'struct repository'
parameter and replace 'git_config()' with 'repo_config()' so that
configuration is read from the passed repository. Also, add a test to
make sure that "git fmt-merge-msg -h" can be called outside a
repository.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/fmt-merge-msg.c | 5 ++---
 t/t1517-outside-repo.sh | 7 +++++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 4b24de32fb..cf4273a52c 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #include "builtin.h"
 #include "config.h"
 #include "fmt-merge-msg.h"
@@ -13,7 +12,7 @@ static const char * const fmt_merge_msg_usage[] = {
 int cmd_fmt_merge_msg(int argc,
 		      const char **argv,
 		      const char *prefix,
-		      struct repository *repo UNUSED)
+		      struct repository *repo)
 {
 	char *inpath = NULL;
 	const char *message = NULL;
@@ -54,7 +53,7 @@ int cmd_fmt_merge_msg(int argc,
 	int ret;
 	struct fmt_merge_msg_opts opts;
 
-	git_config(fmt_merge_msg_config, &merge_log_config);
+	repo_config(repo, 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/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
index 8f59b867f2..4b4e645860 100755
--- a/t/t1517-outside-repo.sh
+++ b/t/t1517-outside-repo.sh
@@ -121,4 +121,11 @@ test_expect_success 'prune does not crash with -h' '
 	test_grep "[Uu]sage: git prune " usage
 '
 
+test_expect_success 'fmt-merge-msg does not crash with -h' '
+	test_expect_code 129 git fmt-merge-msg -h >usage &&
+	test_grep "[Uu]sage: git fmt-merge-msg " usage &&
+	test_expect_code 129 nongit git fmt-merge-msg -h >usage &&
+	test_grep "[Uu]sage: git fmt-merge-msg " usage
+'
+
 test_done
-- 
2.49.0

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

[Phillip Wood]

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

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

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> 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

When fmt_merge_msg_config() needs to read more stuff, the callback
parameter may have to be updated, but this will do for now.

Thanks.

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

[Ayush Chandekar]

On Mon, Aug 11, 2025 at 9:43 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
> > 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
>
> When fmt_merge_msg_config() needs to read more stuff, the callback
> parameter may have to be updated, but this will do for now.
>
> Thanks.

Yes, then we can create a struct and pass the struct instead, maybe.

Thanks,
Ayush