threads / patch / 24810

v3, 5 partsfmt-merge-msg improvements

Subject: [PATCH v3 0/5] fmt-merge-msg improvements

## tl;dr

15 messages between Aug 20, 2010 and Aug 21, 2010. Diffs are folded; open one to read it.

replies: 14people: 3as markdown or json

Ramkumar Ramachandra· Aug 20, 2010, 19:14 UTC · lore
Hi,

There's now a new patch in the series; thanks to Jakub for poking me hard enough :)

-- Ram
Ramkumar Ramachandra (5):
  parse-options: Allow PARSE_OPT_NOARG in integer arguments
  fmt-merge-msg: Make the number of log entries in commit message
    configurable
  fmt-merge-msg: Update command line options to sync with config
    options
  fmt-merge-msg: Remove deprecated --summary option
  fmt-merge-msg: Update fmt-merge-msg and merge-config documentation
 Documentation/git-fmt-merge-msg.txt |   24 +++++++------------
 Documentation/merge-config.txt      |    8 ++++-
 builtin/fmt-merge-msg.c             |   44 ++++++++++++++++------------------
 parse-options.c                     |    3 ++
 4 files changed, 39 insertions(+), 40 deletions(-)
-- 
1.7.2.2.409.gdbb11.dirty
Ramkumar Ramachandra· Aug 20, 2010, 19:14 UTC · re: Ramkumar Ramachandra · lore

[PATCH v3 1/5] parse-options: Allow PARSE_OPT_NOARG in integer arguments

When the option parser encounters an OPTION_INTEGER argument, PARSE_OPT_NOARG should imply that the default value should be used.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Cc: Jakub Narebski <jnareb@gmail.com>
---
 parse-options.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
Show changes to parse-options.c +3 −0
diff --git a/parse-options.c b/parse-options.c
index e0c3641..7ec9886 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -138,6 +138,9 @@ static int get_value(struct parse_opt_ctx_t *p,
 			*(int *)opt->value = 0;
 			return 0;
 		}
+		if (opt->flags & PARSE_OPT_NOARG)
+			*(int *)opt->value = opt->defval;
+			return 0;
 		if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
 			*(int *)opt->value = opt->defval;
 			return 0;

-- 
1.7.2.2.409.gdbb11.dirty
Junio C Hamano· Aug 20, 2010, 19:49 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH v3 1/5] parse-options: Allow PARSE_OPT_NOARG in integer arguments

Ramkumar Ramachandra <artagnon@gmail.com> writes:
> When the option parser encounters an OPTION_INTEGER argument,
> PARSE_OPT_NOARG should imply that the default value should be used.
Sorry but why?

Doesn't NOARG mean "Do not take an argument, if you give me an argument that is an error"?

I would understand if this were OPT_OPTARG, though.
Confused...
Show 23 quoted lines
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> Cc: Jakub Narebski <jnareb@gmail.com>
> ---
>  parse-options.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index e0c3641..7ec9886 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -138,6 +138,9 @@ static int get_value(struct parse_opt_ctx_t *p,
>  			*(int *)opt->value = 0;
>  			return 0;
>  		}
> +		if (opt->flags & PARSE_OPT_NOARG)
> +			*(int *)opt->value = opt->defval;
> +			return 0;
>  		if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
>  			*(int *)opt->value = opt->defval;
>  			return 0;
> -- 
> 1.7.2.2.409.gdbb11.dirty
Ramkumar Ramachandra· Aug 20, 2010, 20:01 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 1/5] parse-options: Allow PARSE_OPT_NOARG in integer arguments

Hi Junio,
Junio C Hamano writes:
Show 9 quoted lines
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
> 
> > When the option parser encounters an OPTION_INTEGER argument,
> > PARSE_OPT_NOARG should imply that the default value should be used.
> 
> Sorry but why?
> 
> Doesn't NOARG mean "Do not take an argument, if you give me an argument
> that is an error"?

Oh, does it mean that? I might have interpreted the description in `parse-options.h` too literally: "says that this option takes no argument". So I'm handling the case when an integer option is specified, but no integer argument is given.

> I would understand if this were OPT_OPTARG, though.

That case is already handled. The condition (opt->flags & PARSE_OPT_OPTARG && !p->opt) does the same thing.

> Confused...

Okay, let me explain. Let's say I want to have an option that takes an integer argument, say `foo`. To set it to the integer argument 42, I can say `--foo=42`. To set it to its default value, I could earlier say `--foo=`. With this patch I can simply say `--foo`. Makes sense?

-- Ram
Jonathan Nieder· Aug 21, 2010, 03:39 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH v3 1/5] parse-options: Allow PARSE_OPT_NOARG in integer arguments

Hi Ram,

[rearranged for convenience] Ramkumar Ramachandra wrote:

> Let's say I want to have an option that takes an
> integer argument, say `foo`. To set it to the integer argument 42, I
> can say `--foo=42`. To set it to its default value, I could earlier
> say `--foo=`. With this patch I can simply say `--foo`. Makes sense?
I think you want OPTARG ("optional argument").

What your patch would allow is using OPTION_INTEGER for boolean options, where people use OPT_SET_INT now. Maybe that would be a good cleanup, but I am not convinced it is worth the churn.

> Junio C Hamano writes:
>> Doesn't NOARG mean "Do not take an argument, if you give me an argument
>> that is an error"?
>
> Oh, does it mean that?
Yes.
-- 8< --
Subject: parse-options: clarify PARSE_OPT_NOARG description

Here "takes no argument" means "does not take an argument". The latter phrasing might make it clearer that PARSE_OPT_NOARG does not make an option with an argument that can optionally be left off.

Noticed-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Show changes to parse-options.h +1 −1
diff --git a/parse-options.h b/parse-options.h
index 7435cdb..d982f0f 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -69,7 +69,7 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
  * `flags`::
  *   mask of parse_opt_option_flags.
  *   PARSE_OPT_OPTARG: says that the argument is optional (not for BOOLEANs)
- *   PARSE_OPT_NOARG: says that this option takes no argument
+ *   PARSE_OPT_NOARG: says that this option does not take an argument
  *   PARSE_OPT_NONEG: says that this option cannot be negated
  *   PARSE_OPT_HIDDEN: this option is skipped in the default usage, and
  *                     shown only in the full usage.
Ramkumar Ramachandra· Aug 20, 2010, 19:14 UTC · re: Ramkumar Ramachandra · lore

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

Make the `merge.log` option either an integer or boolean instead of
just a boolean. The integer can be used to specify how many merged
commits to summarize (at maximum) in the merge message. Let true mean
20. Default to false.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Reported-by: Yaroslav Halchenko <debian@onerussian.com>
Thanks-to: Johannes Sixt <j.sixt@viscovery.net>
Thanks-to: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/fmt-merge-msg.c |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)
Show changes to builtin/fmt-merge-msg.c +15 −8
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index e7e12ee..e967a05 100644
--- a/builtin/fmt-merge-msg.c
+++ 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;
+
 	return 0;
 }
 
@@ -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)
 {
 	int i, count = 0;
@@ -169,7 +176,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 +189,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 +264,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 +310,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;
 }

-- 
1.7.2.2.409.gdbb11.dirty
Jonathan Nieder· Aug 21, 2010, 03:54 UTC · re: Ramkumar Ramachandra · lore

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

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?
Ramkumar Ramachandra· Aug 21, 2010, 05:55 UTC · re: Jonathan Nieder · lore

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

Hi Jonathan,
Jonathan Nieder writes:
Show 21 quoted lines
> 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.
> 
[...]
> What happened to the merge_summary argument?

This patch is roughly equivalent to my original log_limit patch. Yes, I'm trying to have it both ways in this patch :) merge_summary is set by both the command-line option and the config option, while log_limit is set by the config option. As you noticed, the next patch removes merge_summary.

-- Ram
Ramkumar Ramachandra· Aug 20, 2010, 19:15 UTC · re: Ramkumar Ramachandra · lore

[PATCH v3 3/5] fmt-merge-msg: Update command line options to sync with config options

Update the `--log` and `--summary` command line options to be integers and have the same effect as the `merge.log` and `merge.summary` configuration options.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Cc: Johannes Sixt <j.sixt@viscovery.net>
Cc: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/fmt-merge-msg.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)
Show changes to builtin/fmt-merge-msg.c +12 −12
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index e967a05..64c52bd 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -7,11 +7,10 @@
 #include "string-list.h"
 
 static const char * const fmt_merge_msg_usage[] = {
-	"git fmt-merge-msg [-m <message>] [--log|--no-log] [--file <file>]",
+	"git fmt-merge-msg [-m <message>] [--log=[<n>]|--no-log] [--file <file>]",
 	NULL
 };
 
-static int merge_summary;
 static int log_limit = 0;
 
 static int fmt_merge_msg_config(const char *key, const char *value, void *cb)
@@ -27,7 +26,6 @@ static int fmt_merge_msg_config(const char *key, const char *value, void *cb)
 
 	if (is_bool && log_limit)
 		log_limit = 20;
-	merge_summary = log_limit ? 1 : 0;
 
 	return 0;
 }
@@ -262,7 +260,7 @@ static void do_fmt_merge_msg_title(struct strbuf *out,
 		strbuf_addf(out, " into %s\n", current_branch);
 }
 
-static int do_fmt_merge_msg(int merge_title, int merge_summary,
+static int do_fmt_merge_msg(int merge_title, int log_limit,
 	struct strbuf *in, struct strbuf *out) {
 	int i = 0, pos = 0;
 	unsigned char head_sha1[20];
@@ -295,7 +293,7 @@ static int do_fmt_merge_msg(int merge_title, int merge_summary,
 	if (merge_title)
 		do_fmt_merge_msg_title(out, current_branch);
 
-	if (merge_summary) {
+	if (log_limit) {
 		struct commit *head;
 		struct rev_info rev;
 
@@ -315,8 +313,8 @@ static int do_fmt_merge_msg(int merge_title, int merge_summary,
 	return 0;
 }
 
-int fmt_merge_msg(int merge_summary, struct strbuf *in, struct strbuf *out) {
-	return do_fmt_merge_msg(1, merge_summary, in, out);
+int fmt_merge_msg(int log_limit, struct strbuf *in, struct strbuf *out) {
+	return do_fmt_merge_msg(1, log_limit, in, out);
 }
 
 int fmt_merge_msg_shortlog(struct strbuf *in, struct strbuf *out) {
@@ -328,10 +326,12 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 	const char *inpath = NULL;
 	const char *message = NULL;
 	struct option options[] = {
-		OPT_BOOLEAN(0, "log",     &merge_summary, "populate log with the shortlog"),
-		{ OPTION_BOOLEAN, 0, "summary", &merge_summary, NULL,
+		{ OPTION_INTEGER, 0, "log", &log_limit, "n",
+		  "populate log with <n> entries from shortlog",
+		  PARSE_OPT_NOARG, NULL, 20 },
+		{ OPTION_INTEGER, 0, "summary", &log_limit, "n",
 		  "alias for --log (deprecated)",
-		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
+		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 20 },
 		OPT_STRING('m', "message", &message, "text",
 			"use <text> as start of message"),
 		OPT_FILENAME('F', "file", &inpath, "file to read from"),
@@ -347,7 +347,7 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 			     0);
 	if (argc > 0)
 		usage_with_options(fmt_merge_msg_usage, options);
-	if (message && !merge_summary) {
+	if (message && !log_limit) {
 		char nl = '\n';
 		write_in_full(STDOUT_FILENO, message, strlen(message));
 		write_in_full(STDOUT_FILENO, &nl, 1);
@@ -366,7 +366,7 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 		strbuf_addstr(&output, message);
 		ret = fmt_merge_msg_shortlog(&input, &output);
 	} else {
-		ret = fmt_merge_msg(merge_summary, &input, &output);
+		ret = fmt_merge_msg(log_limit, &input, &output);
 	}
 	if (ret)
 		return ret;

-- 
1.7.2.2.409.gdbb11.dirty
Jonathan Nieder· Aug 21, 2010, 04:02 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH v3 3/5] fmt-merge-msg: Update command line options to sync with config options

Ramkumar Ramachandra wrote:
Show 7 quoted lines
> +++ b/builtin/fmt-merge-msg.c
> @@ -7,11 +7,10 @@
>  #include "string-list.h"
>  
>  static const char * const fmt_merge_msg_usage[] = {
> -	"git fmt-merge-msg [-m <message>] [--log|--no-log] [--file <file>]",
> +	"git fmt-merge-msg [-m <message>] [--log=[<n>]|--no-log] [--file <file>]",
[--log[=<n>]], no?
Show 5 quoted lines
>  	NULL
>  };
>  
> -static int merge_summary;
>  static int log_limit = 0;
Ah, so you elminate the boolean arg here. :)
Nit: the usual style in git is to let statics and globals that should
be 0 be implicitly initialized.
Show 13 quoted lines
> @@ -328,10 +326,12 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
>  	const char *inpath = NULL;
>  	const char *message = NULL;
>  	struct option options[] = {
> -		OPT_BOOLEAN(0, "log",     &merge_summary, "populate log with the shortlog"),
> -		{ OPTION_BOOLEAN, 0, "summary", &merge_summary, NULL,
> +		{ OPTION_INTEGER, 0, "log", &log_limit, "n",
> +		  "populate log with <n> entries from shortlog",
> +		  PARSE_OPT_NOARG, NULL, 20 },
> +		{ OPTION_INTEGER, 0, "summary", &log_limit, "n",
>  		  "alias for --log (deprecated)",
> -		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
> +		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 20 },
OPTARG, I think.
Ramkumar Ramachandra· Aug 21, 2010, 05:51 UTC · re: Jonathan Nieder · lore

Re: [PATCH v3 3/5] fmt-merge-msg: Update command line options to sync with config options

Hi Jonathan,
Jonathan Nieder writes:
Show 11 quoted lines
> Ramkumar Ramachandra wrote:
> 
> > +++ b/builtin/fmt-merge-msg.c
> > @@ -7,11 +7,10 @@
> >  #include "string-list.h"
> >  
> >  static const char * const fmt_merge_msg_usage[] = {
> > -	"git fmt-merge-msg [-m <message>] [--log|--no-log] [--file <file>]",
> > +	"git fmt-merge-msg [-m <message>] [--log=[<n>]|--no-log] [--file <file>]",
> 
> [--log[=<n>]], no?
Right, my bad.
Show 15 quoted lines
> > @@ -328,10 +326,12 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
> >  	const char *inpath = NULL;
> >  	const char *message = NULL;
> >  	struct option options[] = {
> > -		OPT_BOOLEAN(0, "log",     &merge_summary, "populate log with the shortlog"),
> > -		{ OPTION_BOOLEAN, 0, "summary", &merge_summary, NULL,
> > +		{ OPTION_INTEGER, 0, "log", &log_limit, "n",
> > +		  "populate log with <n> entries from shortlog",
> > +		  PARSE_OPT_NOARG, NULL, 20 },
> > +		{ OPTION_INTEGER, 0, "summary", &log_limit, "n",
> >  		  "alias for --log (deprecated)",
> > -		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
> > +		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 20 },
> 
> OPTARG, I think.
OPTARG works. Sorry for the NOARG confusion.
-- Ram
Ramkumar Ramachandra· Aug 20, 2010, 19:15 UTC · re: Ramkumar Ramachandra · lore

[PATCH v3 4/5] fmt-merge-msg: Remove deprecated --summary option

Remove the deprecated --summary option that served as a syonym to the --log option.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-fmt-merge-msg.txt |    4 ----
 builtin/fmt-merge-msg.c             |   13 ++-----------
 2 files changed, 2 insertions(+), 15 deletions(-)
Show changes to 2 files +2 −15

Documentation/git-fmt-merge-msg.txt, builtin/fmt-merge-msg.c

diff --git a/Documentation/git-fmt-merge-msg.txt b/Documentation/git-fmt-merge-msg.txt
index 302f56b..78c8a6d 100644
--- a/Documentation/git-fmt-merge-msg.txt
+++ b/Documentation/git-fmt-merge-msg.txt
@@ -55,10 +55,6 @@ merge.log::
 	Whether to include summaries of merged commits in newly
 	merge commit messages. False by default.
 
-merge.summary::
-	Synonym to `merge.log`; this is deprecated and will be removed in
-	the future.
-
 SEE ALSO
 --------
 linkgit:git-merge[1]
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 64c52bd..adab21e 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -15,18 +15,12 @@ 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;
 		log_limit = git_config_bool_or_int(key, value, &is_bool);
+		if (is_bool && log_limit)
+			log_limit = 20;
 	}
-	if (!found_merge_log && !strcmp("merge.summary", key))
-		log_limit = git_config_bool_or_int(key, value, &is_bool);
-
-	if (is_bool && log_limit)
-		log_limit = 20;
-
 	return 0;
 }
 
@@ -329,9 +323,6 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 		{ OPTION_INTEGER, 0, "log", &log_limit, "n",
 		  "populate log with <n> entries from shortlog",
 		  PARSE_OPT_NOARG, NULL, 20 },
-		{ OPTION_INTEGER, 0, "summary", &log_limit, "n",
-		  "alias for --log (deprecated)",
-		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 20 },
 		OPT_STRING('m', "message", &message, "text",
 			"use <text> as start of message"),
 		OPT_FILENAME('F', "file", &inpath, "file to read from"),

-- 
1.7.2.2.409.gdbb11.dirty
Jonathan Nieder· Aug 21, 2010, 04:04 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH v3 4/5] fmt-merge-msg: Remove deprecated --summary option

Ramkumar Ramachandra wrote:
> Remove the deprecated --summary option

We stopped advertising it in v1.7.1-rc0~3^2, right? That would make this v1.8 or later material, I think.

Ramkumar Ramachandra· Aug 21, 2010, 05:49 UTC · re: Jonathan Nieder · lore

Re: [PATCH v3 4/5] fmt-merge-msg: Remove deprecated --summary option

Hi Jonathan,
Jonathan Nieder writes:
Show 6 quoted lines
> Ramkumar Ramachandra wrote:
> 
> > Remove the deprecated --summary option
> 
> We stopped advertising it in v1.7.1-rc0~3^2, right?  That would make
> this v1.8 or later material, I think.
Oh. I should drop this patch while re-rolling?
-- Ram
Ramkumar Ramachandra· Aug 20, 2010, 19:15 UTC · re: Ramkumar Ramachandra · lore

[PATCH v3 5/5] fmt-merge-msg: Update fmt-merge-msg and merge-config documentation

Update the documentation of fmt-merge-msg and merge-config to reflect the fact that `merge.log` can either be a boolean or integer option now, instead of just a boolean.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-fmt-merge-msg.txt |   20 +++++++++-----------
 Documentation/merge-config.txt      |    8 ++++++--
 2 files changed, 15 insertions(+), 13 deletions(-)
Show changes to 2 files +15 −10

Documentation/git-fmt-merge-msg.txt, Documentation/merge-config.txt

diff --git a/Documentation/git-fmt-merge-msg.txt b/Documentation/git-fmt-merge-msg.txt
index 78c8a6d..4149a1f 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' [-m <message>] [--log | --no-log] <$GIT_DIR/FETCH_HEAD
-'git fmt-merge-msg' [-m <message>] [--log | --no-log] -F <file>
+'git fmt-merge-msg' [-m <message>] [--log[=<n>] | --no-log] < $GIT_DIR/FETCH_HEAD
+'git fmt-merge-msg' [-m <message>] [--log[=<n>] | --no-log] -F <file>
 
 DESCRIPTION
 -----------
@@ -24,19 +24,14 @@ automatically invoking 'git merge'.
 OPTIONS
 -------
 
---log::
+--log=<n>::
 	In addition to branch names, populate the log message with
-	one-line descriptions from the actual commits that are being
-	merged.
-
 --no-log::
 	Do not list one-line descriptions from the actual commits being
 	merged.
 
---summary::
---no-summary::
-	Synonyms to --log and --no-log; these are deprecated and will be
-	removed in the future.
+	one-line descriptions from at most <n> actual commits that are
+	being merged.
 
 -m <message>::
 --message <message>::
@@ -53,7 +48,10 @@ CONFIGURATION
 
 merge.log::
 	Whether to include summaries of merged commits in newly
-	merge commit messages. False by default.
+	created merge commit messages.  Optionally, an integer can be
+	used to specify how many merged commits to summarize (at
+	maxmium) in the merge message. Specifying "true" is equivalent
+	to specifying 20. Defaults to false.
 
 SEE ALSO
 --------
diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index b72f533..f63f7cb 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -7,8 +7,12 @@ merge.conflictstyle::
 	marker and the original text before the `=======` marker.
 
 merge.log::
-	Whether to include summaries of merged commits in newly created
-	merge commit messages. False by default.
+	Whether to include summaries of merged commits in newly
+	created merge commit messages.  Optionally, an integer can be
+	used to specify how many merged commits to summarize (at
+	maxmium) in the merge message. Specifying "true" is equivalent
+	to specifying 20.  Defaults to false. See also
+	linkgit:git-fmt-merge-msg[1].
 
 merge.renameLimit::
 	The number of files to consider when performing rename detection

-- 
1.7.2.2.409.gdbb11.dirty

← back to recent threads