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

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

[Ramkumar Ramachandra]

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

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

[Ramkumar Ramachandra]

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

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

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

[Ramkumar Ramachandra]

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

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

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

[Ramkumar Ramachandra]

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

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

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

[Ramkumar Ramachandra]

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

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

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

[Ramkumar Ramachandra]

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

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

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

[Junio C Hamano]

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...

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

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

[Ramkumar Ramachandra]

Hi Junio,

Junio C Hamano writes:
> 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

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

[Jonathan Nieder]

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>
---
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.
-- 

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

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

> +++ 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.

> @@ -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?

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

[Jonathan Nieder]

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?

>  	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.

> @@ -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.

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

[Jonathan Nieder]

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.

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

[Ramkumar Ramachandra]

Hi Jonathan,

Jonathan Nieder writes:
> 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

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

[Ramkumar Ramachandra]

Hi Jonathan,

Jonathan Nieder writes:
> 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.

> > @@ -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

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

[Ramkumar Ramachandra]

Hi Jonathan,

Jonathan Nieder writes:
> 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