wishlist bugreport: make limit configurable for do_fmt_merge_msg (merge.log)

wishlist bugreport: make limit configurable for do_fmt_merge_msg (merge.log)

[Yaroslav Halchenko]

Hi Git Developers,

Shame on me but couldn't figure out if there is any official bug tracker
for git -- previously I just complained here, so keeping the tradition:

merge.log (or merge.summary) enables a really nice feature of including
a list of commits involved in the merge.  Unfortunately it is limited to
20 entries and only includes total number of included commits if that is
larger than 20.

Looking at the source code (if I got it right)

static int do_fmt_merge_msg(int merge_title, int merge_summary,
	struct strbuf *in, struct strbuf *out) {
	int limit = 20, i = 0, pos = 0;
.... no line touches limit ....
			shortlog(origins.items[i].string, origins.items[i].util,
					head, &rev, limit, out);

so, limit of 20 is hardcoded and cannot be altered via configuration.  I
would love to have it configurable, so, if desired, be set to infinity
(configuration wide or as a cmd line parameter for a specific merge).

Thanks in advance
-- 
                                  .-.
=------------------------------   /v\  ----------------------------=
Keep in touch                    // \\     (yoh@|www.)onerussian.com
Yaroslav Halchenko              /(   )\               ICQ#: 60653192
                   Linux User    ^^-^^    [175555]

Re: wishlist bugreport: make limit configurable for do_fmt_merge_msg (merge.log)

[Ramkumar Ramachandra]

Hi Yaroslav,

Yaroslav Halchenko writes:
> merge.log (or merge.summary) enables a really nice feature of including
> a list of commits involved in the merge.  Unfortunately it is limited to
> 20 entries and only includes total number of included commits if that is
> larger than 20.
> 
> Looking at the source code (if I got it right)
> 
> static int do_fmt_merge_msg(int merge_title, int merge_summary,
> 	struct strbuf *in, struct strbuf *out) {
> 	int limit = 20, i = 0, pos = 0;
> .... no line touches limit ....
> 			shortlog(origins.items[i].string, origins.items[i].util,
> 					head, &rev, limit, out);
> 
> so, limit of 20 is hardcoded and cannot be altered via configuration.  I
> would love to have it configurable, so, if desired, be set to infinity
> (configuration wide or as a cmd line parameter for a specific merge).

You're perhpas looking for something like this? Warning: Untested.

-- 8< --
commit 86c34c682345843d9138882a85ba36faf10e0d95
Author: Ramkumar Ramachandra <artagnon@gmail.com>
Date:   Fri Aug 20 12:12:59 2010 +0530

    fmt-merge-msg: Make the number of log entries in shortlog configurable
    
    Introduce a new configuration option called merge.logLimit to limit
    the number of log entries displayed in the shortlog of a merge commit
    configurable. Set the default value to 20.

diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index a76cd4e..30782f6 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -12,6 +12,7 @@ static const char * const fmt_merge_msg_usage[] = {
 };
 
 static int merge_summary;
+static int log_limit = 20;
 
 static int fmt_merge_msg_config(const char *key, const char *value, void *cb)
 {
@@ -22,6 +23,8 @@ static int fmt_merge_msg_config(const char *key, const char *value, void *cb)
 	}
 	if (!found_merge_log && !strcmp("merge.summary", key))
 		merge_summary = git_config_bool(key, value);
+	if (!strcmp("merge.logLimit", key))
+		log_limit = git_config_int(key, value);
 	return 0;
 }
 
@@ -140,7 +143,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)
 {
 	int i, count = 0;
@@ -169,7 +172,7 @@ static void shortlog(const char *name, unsigned char *sha1,
 			continue;
 
 		count++;
-		if (subjects.nr > limit)
+		if (subjects.nr > log_limit)
 			continue;
 
 		format_commit_message(commit, "%s", &sb, &ctx);
@@ -182,13 +185,13 @@ static void shortlog(const char *name, unsigned char *sha1,
 			string_list_append(&subjects, strbuf_detach(&sb, NULL));
 	}
 
-	if (count > limit)
+	if (count > log_limit)
 		strbuf_addf(out, "\n* %s: (%d commits)\n", name, count);
 	else
 		strbuf_addf(out, "\n* %s:\n", name);
 
 	for (i = 0; i < subjects.nr; i++)
-		if (i >= limit)
+		if (i >= log_limit)
 			strbuf_addf(out, "  ...\n");
 		else
 			strbuf_addf(out, "  %s\n", subjects.items[i].string);
@@ -257,7 +260,7 @@ static void do_fmt_merge_msg_title(struct strbuf *out,
 
 static int do_fmt_merge_msg(int merge_title, int merge_summary,
 	struct strbuf *in, struct strbuf *out) {
-	int limit = 20, i = 0, pos = 0;
+	int i = 0, pos = 0;
 	unsigned char head_sha1[20];
 	const char *current_branch;
 
@@ -303,7 +306,7 @@ static int do_fmt_merge_msg(int merge_title, int merge_summary,
 
 		for (i = 0; i < origins.nr; i++)
 			shortlog(origins.items[i].string, origins.items[i].util,
-					head, &rev, limit, out);
+					head, &rev, out);
 	}
 	return 0;
 }

Re: wishlist bugreport: make limit configurable for do_fmt_merge_msg (merge.log)

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

>     fmt-merge-msg: Make the number of log entries in shortlog configurable
>     
>     Introduce a new configuration option called merge.logLimit to limit
>     the number of log entries displayed in the shortlog of a merge commit
>     configurable. Set the default value to 20.

Neat.  Sign-off?

> +++ b/builtin/fmt-merge-msg.c
> @@ -22,6 +23,8 @@ static int fmt_merge_msg_config(const char *key, const char *value, void *cb)
>  	}
>  	if (!found_merge_log && !strcmp("merge.summary", key))
>  		merge_summary = git_config_bool(key, value);
> +	if (!strcmp("merge.logLimit", key))
> +		log_limit = git_config_int(key, value);

Maybe something like the following would be good on top (or maybe not;
you decide).

-- 8< --
Subject: fmt-merge-msg --log-limit to override merge.loglimit configuration

Yes, one can already use "git -c merge.loglimit=n fmt-merge-msg", but
maybe providing an option name makes it more obvious that this can
be overridden on the command-line.

This also provides --log-limit=0 / "[merge] loglimit = 0" to not limit
the number of commits summarized at all, which I would expect to
be the most interesting case.  So you can use
"git fmt-merge-msg --log-limit=0" in your scripts to get a message
like

	Merge branch 'long-topic'

	 * log-topic:
	   commit 1
	   commit 2
[...]
	   commit 1001
	   commit 1002

and not to limit the number of commits summarized at all.

This patch does not propagate the option from "git merge" yet.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Also untested.  I still haven't figured out the context where this
would be most useful, just thought it sounded like a fun idea.

diff --git a/Documentation/git-fmt-merge-msg.txt b/Documentation/git-fmt-merge-msg.txt
index a585dbe..3066392 100644
--- a/Documentation/git-fmt-merge-msg.txt
+++ b/Documentation/git-fmt-merge-msg.txt
@@ -9,8 +9,8 @@ git-fmt-merge-msg - Produce a merge commit message
 SYNOPSIS
 --------
 [verse]
-'git fmt-merge-msg' [--log | --no-log] <$GIT_DIR/FETCH_HEAD
-'git fmt-merge-msg' [--log | --no-log] -F <file>
+'git fmt-merge-msg' [--log | --no-log] [--log-limit=<n>] <$GIT_DIR/FETCH_HEAD
+'git fmt-merge-msg' [--log | --no-log] [--log-limit=<n>] -F <file>
 
 DESCRIPTION
 -----------
@@ -38,6 +38,11 @@ OPTIONS
 	Synonyms to --log and --no-log; these are deprecated and will be
 	removed in the future.
 
+--log-limit <n>::
+	Truncate --log output after <n> lines, instead of 20.  This
+	overrides the merge.loglimit configuration.  If <n> is 0,
+	do not truncate --log output at all.
+
 -F <file>::
 --file <file>::
 	Take the list of merged objects from <file> instead of
@@ -54,6 +59,11 @@ merge.summary::
 	Synonym to `merge.log`; this is deprecated and will be removed in
 	the future.
 
+merge.loglimit::
+	How many commits (at maximum) to print from each merge
+	parent when the `--log` option is used.  The default is 20.
+	Can be overridden by the `--log-limit` option.
+
 SEE ALSO
 --------
 linkgit:git-merge[1]
diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index a403155..fd861b8 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -10,6 +10,11 @@ merge.log::
 	Whether to include summaries of merged commits in newly created
 	merge commit messages. False by default.
 
+merge.loglimit::
+	How many merged commits to summarize in the merge message if
+	`--log` is used.  The default is 20.  See also
+	linkgit:git-fmt-merge-msg[1].
+
 merge.renameLimit::
 	The number of files to consider when performing rename detection
 	during a merge; if not specified, defaults to the value of
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 2c1d15b..e1aefa7 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -7,7 +7,7 @@
 #include "string-list.h"
 
 static const char * const fmt_merge_msg_usage[] = {
-	"git fmt-merge-msg [--log|--no-log] [--file <file>]",
+	"git fmt-merge-msg [--log|--no-log] [--log-limit=<num>] [--file <file>]",
 	NULL
 };
 
@@ -172,7 +172,7 @@ static void shortlog(const char *name, unsigned char *sha1,
 			continue;
 
 		count++;
-		if (subjects.nr > log_limit)
+		if (log_limit && subjects.nr > log_limit)
 			continue;
 
 		format_commit_message(commit, "%s", &sb, &ctx);
@@ -185,13 +185,13 @@ static void shortlog(const char *name, unsigned char *sha1,
 			string_list_append(&subjects, strbuf_detach(&sb, NULL));
 	}
 
-	if (count > log_limit)
+	if (log_limit && count > log_limit)
 		strbuf_addf(out, "\n* %s: (%d commits)\n", name, count);
 	else
 		strbuf_addf(out, "\n* %s:\n", name);
 
 	for (i = 0; i < subjects.nr; i++)
-		if (i >= log_limit)
+		if (log_limit && i >= log_limit)
 			strbuf_addf(out, "  ...\n");
 		else
 			strbuf_addf(out, "  %s\n", subjects.items[i].string);
@@ -327,6 +327,8 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 		{ OPTION_BOOLEAN, 0, "summary", &merge_summary, NULL,
 		  "alias for --log (deprecated)",
 		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
+		OPT_INTEGER(0, "log-limit", &log_limit,
+			"truncate shortlog after <n> lines (0 for no limit)"),
 		OPT_FILENAME('F', "file", &inpath, "file to read from"),
 		OPT_END()
 	};
-- 

Re: wishlist bugreport: make limit configurable for do_fmt_merge_msg (merge.log)

[Ramkumar Ramachandra]

Hi Jonathan,

Jonathan Nieder writes:
> Ramkumar Ramachandra wrote:
> 
> >     fmt-merge-msg: Make the number of log entries in shortlog configurable
> >     
> >     Introduce a new configuration option called merge.logLimit to limit
> >     the number of log entries displayed in the shortlog of a merge commit
> >     configurable. Set the default value to 20.
> 
> Neat.  Sign-off?

Oops. I forgot about the new policy: when a patch isn't ready for
inclusion, I must say that explicitly and sign off so others can base
their work on my patch, right?
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>

> > +++ b/builtin/fmt-merge-msg.c
> > @@ -22,6 +23,8 @@ static int fmt_merge_msg_config(const char *key, const char *value, void *cb)
> >  	}
> >  	if (!found_merge_log && !strcmp("merge.summary", key))
> >  		merge_summary = git_config_bool(key, value);
> > +	if (!strcmp("merge.logLimit", key))
> > +		log_limit = git_config_int(key, value);
> 
> Maybe something like the following would be good on top (or maybe not;
> you decide).

Looks good- since you're interested in this too, I'll include your
patch, test it and post a series to the list.

Thanks.

-- Ram

Re: wishlist bugreport: make limit configurable for do_fmt_merge_msg (merge.log)

[Johannes Sixt]

Am 8/20/2010 10:16, schrieb Jonathan Nieder:
> This also provides --log-limit=0 / "[merge] loglimit = 0" to not limit
> the number of commits summarized at all, which I would expect to
...
>  [verse]
> -'git fmt-merge-msg' [--log | --no-log] <$GIT_DIR/FETCH_HEAD
> -'git fmt-merge-msg' [--log | --no-log] -F <file>
> +'git fmt-merge-msg' [--log | --no-log] [--log-limit=<n>] <$GIT_DIR/FETCH_HEAD
> +'git fmt-merge-msg' [--log | --no-log] [--log-limit=<n>] -F <file>

Do we need --log-limit? Why not just --log=42 and --no-log equals --log=0?

Ditto for the config option:

  merge.log=42
  merge.log=0

and for backwards compatibility:

  merge.log=false  ===  merge.log=0
  merge.log=true   ===  merge.log=20

-- Hannes

Re: wishlist bugreport: make limit configurable for do_fmt_merge_msg (merge.log)

[Ramkumar Ramachandra]

Hi Johannes,

Johannes Sixt writes:
> Am 8/20/2010 10:16, schrieb Jonathan Nieder:
> > This also provides --log-limit=0 / "[merge] loglimit = 0" to not limit
> > the number of commits summarized at all, which I would expect to
> ...
> >  [verse]
> > -'git fmt-merge-msg' [--log | --no-log] <$GIT_DIR/FETCH_HEAD
> > -'git fmt-merge-msg' [--log | --no-log] -F <file>
> > +'git fmt-merge-msg' [--log | --no-log] [--log-limit=<n>] <$GIT_DIR/FETCH_HEAD
> > +'git fmt-merge-msg' [--log | --no-log] [--log-limit=<n>] -F <file>
> 
> Do we need --log-limit? Why not just --log=42 and --no-log equals --log=0?
> 
> Ditto for the config option:
> 
>   merge.log=42
>   merge.log=0

Ah, just when I was about to post the series. Excellent idea! I'll
drop Jonathan's patch and fixup the series to do this in a few
minutes.

> and for backwards compatibility:
> 
>   merge.log=false  ===  merge.log=0
>   merge.log=true   ===  merge.log=20

I'll use git_config_bool_or_int for this -- we've traded off the
ability to say "infinite" though.

-- Ram

Re: wishlist bugreport: make limit configurable for do_fmt_merge_msg (merge.log)

[Johannes Sixt]

Am 8/20/2010 11:19, schrieb Ramkumar Ramachandra:
> I'll use git_config_bool_or_int for this -- we've traded off the
> ability to say "infinite" though.

I don't miss it. But can we still write in the config file

  [merge]
     log

as a short-hand for

  [merge]
     log = true

? If *that* were not possible anymore, it would be a regression.

-- Hannes