From: Phillip Wood Date: Wed, 30 Jul 2025 08:53:51 GMT Subject: Re: [GSOC PATCH 1/2] environment: remove the global variable 'merge_log_config' Message-ID: <1f07119e-64b6-453f-ae83-64a5fb486188@gmail.com> In-Reply-To: Hi Ayush On 29/07/2025 22:16, Ayush Chandekar wrote: > > On Wed, Jul 30, 2025 at 12:37 AM Phillip Wood 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 >