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

DISCUSSION Yaroslav Halchenko 4 participants 7 messages Jun 15, 2016 Documentation

Share & tools

Yaroslav Halchenko

author

2016-06-15 22:49 UTC #0 lore raw

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

Signature · 5 lines

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

Ramkumar Ramachandra

2016-06-15 22:49 UTC #1 lore raw

Hi Yaroslav,

Quoted 14 lines · Yaroslav Halchenko

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);

Quoted 3 lines

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 <[email protected]>
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.

builtin/fmt-merge-msg.c +9 -6

index a76cd4e..30782f6 100644
@@ -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;
 }

Jonathan Nieder

2016-06-15 22:49 UTC #2 lore raw

Quoted 5 lines · Ramkumar Ramachandra

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?

Review static int fmt_merge_msg_config(const char *key, const char *value, void *cb) · line 27

Quoted 7 lines

+++ b/builtin/fmt-merge-msg.c

(quoted diff) +2 -0

@@ -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 <[email protected]>

---
Also untested. I still haven't figured out the context where this would be most useful, just thought it sounded like a fun idea.

Documentation/git-fmt-merge-msg.txt +12 -2

index a585dbe..3066392 100644
@@ -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]

Documentation/merge-config.txt +5 -0

index a403155..fd861b8 100644
@@ -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

builtin/fmt-merge-msg.c +6 -4

index 2c1d15b..e1aefa7 100644
@@ -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()
 	};

Ramkumar Ramachandra

2016-06-15 22:49 UTC #3 lore raw

Hi Jonathan,

Quoted 9 lines · Jonathan Nieder

Quoted 5 lines · Ramkumar Ramachandra
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 <[email protected]>

Quoted 10 lines

Review static int fmt_merge_msg_config(const char *key, const char *value, void *cb) · line 27
Quoted 7 lines
+++ b/builtin/fmt-merge-msg.c
(quoted diff) +2 -0
@@ -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

Johannes Sixt

2016-06-15 22:49 UTC #4 lore raw

In reply to Jonathan Nieder

Am 8/20/2010 10:16, schrieb Jonathan Nieder:

Quoted 2 lines

This also provides --log-limit=0 / "[merge] loglimit = 0" to not limit the number of commits summarized at all, which I would expect to

...

Quoted 5 lines

[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

Ramkumar Ramachandra

2016-06-15 22:49 UTC #5 lore raw

Hi Johannes,

Quoted 10 lines · Johannes Sixt

Am 8/20/2010 10:16, schrieb Jonathan Nieder:
Quoted 2 lines
This also provides --log-limit=0 / "[merge] loglimit = 0" to not limit the number of commits summarized at all, which I would expect to
...
Quoted 5 lines
[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>

Quoted 6 lines

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.

Quoted 4 lines

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

Johannes Sixt

2016-06-15 22:49 UTC #6 lore raw

Am 8/20/2010 11:19, schrieb Ramkumar Ramachandra:

Quoted 2 lines

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