Re: [PATCH v3 2/5] fmt-merge-msg: Make the number of log entries in commit message configurable

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 21, 2010, 03:54 UTC
Message-ID: <20100821035403.GA2474@burratino>
In-Reply-To: <1282331702-5115-3-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra wrote:

Show 23 quoted lines

> +++ b/builtin/fmt-merge-msg.c
> @@ -12,16 +12,23 @@ static const char * const fmt_merge_msg_usage[] = {
>  };
>  
>  static int merge_summary;
> +static int log_limit = 0;
>  
>  static int fmt_merge_msg_config(const char *key, const char *value, void *cb)
>  {
>  	static int found_merge_log = 0;
> +	int is_bool = 0;
>  	if (!strcmp("merge.log", key)) {
>  		found_merge_log = 1;
> -		merge_summary = git_config_bool(key, value);
> +		log_limit = git_config_bool_or_int(key, value, &is_bool);
>  	}
>  	if (!found_merge_log && !strcmp("merge.summary", key))
> -		merge_summary = git_config_bool(key, value);
> +		log_limit = git_config_bool_or_int(key, value, &is_bool);
> +
> +	if (is_bool && log_limit)
> +		log_limit = 20;
> +	merge_summary = log_limit ? 1 : 0;

Hmm, this seems to be trying to have it both ways. It would be simpler to either:

	static int merge_summary;
	static int log_limit = 20;

providing independent internal "enabled" and "limit" knobs, so one could use, say,

	[merge]
		log = 2
		log = false
		log = true

with the result being be a log_limit of 2, or

	static int log_limit;

where 0 means disabled, so in that example the result would be a log_limit of 20.

Show 7 quoted lines

> @@ -140,7 +147,7 @@ static void print_joined(const char *singular, const char *plural,
>  }
>  
>  static void shortlog(const char *name, unsigned char *sha1,
> -		struct commit *head, struct rev_info *rev, int limit,
> +		struct commit *head, struct rev_info *rev,
>  		struct strbuf *out)

A part of me wishes we would still pass the limit around (for no good reason), but you are probably right that it is easier to work with the global.

> @@ -257,7 +264,7 @@ static void do_fmt_merge_msg_title(struct strbuf *out,
> 
>  static int do_fmt_merge_msg(int merge_title, int merge_summary,

What happened to the merge_summary argument?