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

Hi Junio,

On Tue, Jul 29, 2025 at 10:18 PM Junio C Hamano [off-list ref] wrote:

Ayush Chandekar [off-list ref] writes:

quoted

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