{"thread":{"id":"24810","subject":"[PATCH v3 0/5] fmt-merge-msg improvements","startedAt":"2010-08-20T19:14:57Z","lastAt":"2010-08-21T05:55:59Z","messageCount":15,"participants":["Ramkumar Ramachandra","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":3,"patchTotal":5},"messages":[{"id":"148596","messageId":"1282331702-5115-1-git-send-email-artagnon@gmail.com","threadId":"24810","inReplyTo":null,"subject":"[PATCH v3 0/5] fmt-merge-msg improvements","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-20T19:14:57Z","receivedAt":"2010-08-20T19:14:57Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nThere's now a new patch in the series; thanks to Jakub for poking me\nhard enough :)\n\n-- Ram\n\nRamkumar Ramachandra (5):\n  parse-options: Allow PARSE_OPT_NOARG in integer arguments\n  fmt-merge-msg: Make the number of log entries in commit message\n    configurable\n  fmt-merge-msg: Update command line options to sync with config\n    options\n  fmt-merge-msg: Remove deprecated --summary option\n  fmt-merge-msg: Update fmt-merge-msg and merge-config documentation\n\n Documentation/git-fmt-merge-msg.txt |   24 +++++++------------\n Documentation/merge-config.txt      |    8 ++++-\n builtin/fmt-merge-msg.c             |   44 ++++++++++++++++------------------\n parse-options.c                     |    3 ++\n 4 files changed, 39 insertions(+), 40 deletions(-)\n\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"148597","messageId":"1282331702-5115-2-git-send-email-artagnon@gmail.com","threadId":"24810","inReplyTo":"1282331702-5115-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v3 1/5] parse-options: Allow PARSE_OPT_NOARG in integer arguments","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-20T19:14:58Z","receivedAt":"2010-08-20T19:14:58Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"When the option parser encounters an OPTION_INTEGER argument,\nPARSE_OPT_NOARG should imply that the default value should be used.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\nCc: Jakub Narebski <jnareb@gmail.com>\n---\n parse-options.c |    3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex e0c3641..7ec9886 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -138,6 +138,9 @@ static int get_value(struct parse_opt_ctx_t *p,\n \t\t\t*(int *)opt->value = 0;\n \t\t\treturn 0;\n \t\t}\n+\t\tif (opt->flags & PARSE_OPT_NOARG)\n+\t\t\t*(int *)opt->value = opt->defval;\n+\t\t\treturn 0;\n \t\tif (opt->flags & PARSE_OPT_OPTARG && !p->opt) {\n \t\t\t*(int *)opt->value = opt->defval;\n \t\t\treturn 0;\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"148598","messageId":"1282331702-5115-3-git-send-email-artagnon@gmail.com","threadId":"24810","inReplyTo":"1282331702-5115-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v3 2/5] fmt-merge-msg: Make the number of log entries in commit message configurable","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-20T19:14:59Z","receivedAt":"2010-08-20T19:14:59Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Make the `merge.log` option either an integer or boolean instead of\njust a boolean. The integer can be used to specify how many merged\ncommits to summarize (at maximum) in the merge message. Let true mean\n20. Default to false.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\nReported-by: Yaroslav Halchenko <debian@onerussian.com>\nThanks-to: Johannes Sixt <j.sixt@viscovery.net>\nThanks-to: Jonathan Nieder <jrnieder@gmail.com>\n---\n builtin/fmt-merge-msg.c |   23 +++++++++++++++--------\n 1 files changed, 15 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex e7e12ee..e967a05 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -12,16 +12,23 @@ static const char * const fmt_merge_msg_usage[] = {\n };\n \n static int merge_summary;\n+static int log_limit = 0;\n \n static int fmt_merge_msg_config(const char *key, const char *value, void *cb)\n {\n \tstatic int found_merge_log = 0;\n+\tint is_bool = 0;\n \tif (!strcmp(\"merge.log\", key)) {\n \t\tfound_merge_log = 1;\n-\t\tmerge_summary = git_config_bool(key, value);\n+\t\tlog_limit = git_config_bool_or_int(key, value, &is_bool);\n \t}\n \tif (!found_merge_log && !strcmp(\"merge.summary\", key))\n-\t\tmerge_summary = git_config_bool(key, value);\n+\t\tlog_limit = git_config_bool_or_int(key, value, &is_bool);\n+\n+\tif (is_bool && log_limit)\n+\t\tlog_limit = 20;\n+\tmerge_summary = log_limit ? 1 : 0;\n+\n \treturn 0;\n }\n \n@@ -140,7 +147,7 @@ static void print_joined(const char *singular, const char *plural,\n }\n \n static void shortlog(const char *name, unsigned char *sha1,\n-\t\tstruct commit *head, struct rev_info *rev, int limit,\n+\t\tstruct commit *head, struct rev_info *rev,\n \t\tstruct strbuf *out)\n {\n \tint i, count = 0;\n@@ -169,7 +176,7 @@ static void shortlog(const char *name, unsigned char *sha1,\n \t\t\tcontinue;\n \n \t\tcount++;\n-\t\tif (subjects.nr > limit)\n+\t\tif (subjects.nr > log_limit)\n \t\t\tcontinue;\n \n \t\tformat_commit_message(commit, \"%s\", &sb, &ctx);\n@@ -182,13 +189,13 @@ static void shortlog(const char *name, unsigned char *sha1,\n \t\t\tstring_list_append(&subjects, strbuf_detach(&sb, NULL));\n \t}\n \n-\tif (count > limit)\n+\tif (count > log_limit)\n \t\tstrbuf_addf(out, \"\\n* %s: (%d commits)\\n\", name, count);\n \telse\n \t\tstrbuf_addf(out, \"\\n* %s:\\n\", name);\n \n \tfor (i = 0; i < subjects.nr; i++)\n-\t\tif (i >= limit)\n+\t\tif (i >= log_limit)\n \t\t\tstrbuf_addf(out, \"  ...\\n\");\n \t\telse\n \t\t\tstrbuf_addf(out, \"  %s\\n\", subjects.items[i].string);\n@@ -257,7 +264,7 @@ static void do_fmt_merge_msg_title(struct strbuf *out,\n \n static int do_fmt_merge_msg(int merge_title, int merge_summary,\n \tstruct strbuf *in, struct strbuf *out) {\n-\tint limit = 20, i = 0, pos = 0;\n+\tint i = 0, pos = 0;\n \tunsigned char head_sha1[20];\n \tconst char *current_branch;\n \n@@ -303,7 +310,7 @@ static int do_fmt_merge_msg(int merge_title, int merge_summary,\n \n \t\tfor (i = 0; i < origins.nr; i++)\n \t\t\tshortlog(origins.items[i].string, origins.items[i].util,\n-\t\t\t\t\thead, &rev, limit, out);\n+\t\t\t\t\thead, &rev, out);\n \t}\n \treturn 0;\n }\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"148599","messageId":"1282331702-5115-4-git-send-email-artagnon@gmail.com","threadId":"24810","inReplyTo":"1282331702-5115-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v3 3/5] fmt-merge-msg: Update command line options to sync with config options","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-20T19:15:00Z","receivedAt":"2010-08-20T19:15:00Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Update the `--log` and `--summary` command line options to be integers\nand have the same effect as the `merge.log` and `merge.summary`\nconfiguration options.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\nCc: Johannes Sixt <j.sixt@viscovery.net>\nCc: Jonathan Nieder <jrnieder@gmail.com>\n---\n builtin/fmt-merge-msg.c |   24 ++++++++++++------------\n 1 files changed, 12 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex e967a05..64c52bd 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -7,11 +7,10 @@\n #include \"string-list.h\"\n \n static const char * const fmt_merge_msg_usage[] = {\n-\t\"git fmt-merge-msg [-m <message>] [--log|--no-log] [--file <file>]\",\n+\t\"git fmt-merge-msg [-m <message>] [--log=[<n>]|--no-log] [--file <file>]\",\n \tNULL\n };\n \n-static int merge_summary;\n static int log_limit = 0;\n \n static int fmt_merge_msg_config(const char *key, const char *value, void *cb)\n@@ -27,7 +26,6 @@ static int fmt_merge_msg_config(const char *key, const char *value, void *cb)\n \n \tif (is_bool && log_limit)\n \t\tlog_limit = 20;\n-\tmerge_summary = log_limit ? 1 : 0;\n \n \treturn 0;\n }\n@@ -262,7 +260,7 @@ static void do_fmt_merge_msg_title(struct strbuf *out,\n \t\tstrbuf_addf(out, \" into %s\\n\", current_branch);\n }\n \n-static int do_fmt_merge_msg(int merge_title, int merge_summary,\n+static int do_fmt_merge_msg(int merge_title, int log_limit,\n \tstruct strbuf *in, struct strbuf *out) {\n \tint i = 0, pos = 0;\n \tunsigned char head_sha1[20];\n@@ -295,7 +293,7 @@ static int do_fmt_merge_msg(int merge_title, int merge_summary,\n \tif (merge_title)\n \t\tdo_fmt_merge_msg_title(out, current_branch);\n \n-\tif (merge_summary) {\n+\tif (log_limit) {\n \t\tstruct commit *head;\n \t\tstruct rev_info rev;\n \n@@ -315,8 +313,8 @@ static int do_fmt_merge_msg(int merge_title, int merge_summary,\n \treturn 0;\n }\n \n-int fmt_merge_msg(int merge_summary, struct strbuf *in, struct strbuf *out) {\n-\treturn do_fmt_merge_msg(1, merge_summary, in, out);\n+int fmt_merge_msg(int log_limit, struct strbuf *in, struct strbuf *out) {\n+\treturn do_fmt_merge_msg(1, log_limit, in, out);\n }\n \n int fmt_merge_msg_shortlog(struct strbuf *in, struct strbuf *out) {\n@@ -328,10 +326,12 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)\n \tconst char *inpath = NULL;\n \tconst char *message = NULL;\n \tstruct option options[] = {\n-\t\tOPT_BOOLEAN(0, \"log\",     &merge_summary, \"populate log with the shortlog\"),\n-\t\t{ OPTION_BOOLEAN, 0, \"summary\", &merge_summary, NULL,\n+\t\t{ OPTION_INTEGER, 0, \"log\", &log_limit, \"n\",\n+\t\t  \"populate log with <n> entries from shortlog\",\n+\t\t  PARSE_OPT_NOARG, NULL, 20 },\n+\t\t{ OPTION_INTEGER, 0, \"summary\", &log_limit, \"n\",\n \t\t  \"alias for --log (deprecated)\",\n-\t\t  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },\n+\t\t  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 20 },\n \t\tOPT_STRING('m', \"message\", &message, \"text\",\n \t\t\t\"use <text> as start of message\"),\n \t\tOPT_FILENAME('F', \"file\", &inpath, \"file to read from\"),\n@@ -347,7 +347,7 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)\n \t\t\t     0);\n \tif (argc > 0)\n \t\tusage_with_options(fmt_merge_msg_usage, options);\n-\tif (message && !merge_summary) {\n+\tif (message && !log_limit) {\n \t\tchar nl = '\\n';\n \t\twrite_in_full(STDOUT_FILENO, message, strlen(message));\n \t\twrite_in_full(STDOUT_FILENO, &nl, 1);\n@@ -366,7 +366,7 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_addstr(&output, message);\n \t\tret = fmt_merge_msg_shortlog(&input, &output);\n \t} else {\n-\t\tret = fmt_merge_msg(merge_summary, &input, &output);\n+\t\tret = fmt_merge_msg(log_limit, &input, &output);\n \t}\n \tif (ret)\n \t\treturn ret;\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"148600","messageId":"1282331702-5115-5-git-send-email-artagnon@gmail.com","threadId":"24810","inReplyTo":"1282331702-5115-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v3 4/5] fmt-merge-msg: Remove deprecated --summary option","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-20T19:15:01Z","receivedAt":"2010-08-20T19:15:01Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Remove the deprecated --summary option that served as a syonym to the\n--log option.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Documentation/git-fmt-merge-msg.txt |    4 ----\n builtin/fmt-merge-msg.c             |   13 ++-----------\n 2 files changed, 2 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/git-fmt-merge-msg.txt b/Documentation/git-fmt-merge-msg.txt\nindex 302f56b..78c8a6d 100644\n--- a/Documentation/git-fmt-merge-msg.txt\n+++ b/Documentation/git-fmt-merge-msg.txt\n@@ -55,10 +55,6 @@ merge.log::\n \tWhether to include summaries of merged commits in newly\n \tmerge commit messages. False by default.\n \n-merge.summary::\n-\tSynonym to `merge.log`; this is deprecated and will be removed in\n-\tthe future.\n-\n SEE ALSO\n --------\n linkgit:git-merge[1]\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 64c52bd..adab21e 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -15,18 +15,12 @@ static int log_limit = 0;\n \n static int fmt_merge_msg_config(const char *key, const char *value, void *cb)\n {\n-\tstatic int found_merge_log = 0;\n \tint is_bool = 0;\n \tif (!strcmp(\"merge.log\", key)) {\n-\t\tfound_merge_log = 1;\n \t\tlog_limit = git_config_bool_or_int(key, value, &is_bool);\n+\t\tif (is_bool && log_limit)\n+\t\t\tlog_limit = 20;\n \t}\n-\tif (!found_merge_log && !strcmp(\"merge.summary\", key))\n-\t\tlog_limit = git_config_bool_or_int(key, value, &is_bool);\n-\n-\tif (is_bool && log_limit)\n-\t\tlog_limit = 20;\n-\n \treturn 0;\n }\n \n@@ -329,9 +323,6 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)\n \t\t{ OPTION_INTEGER, 0, \"log\", &log_limit, \"n\",\n \t\t  \"populate log with <n> entries from shortlog\",\n \t\t  PARSE_OPT_NOARG, NULL, 20 },\n-\t\t{ OPTION_INTEGER, 0, \"summary\", &log_limit, \"n\",\n-\t\t  \"alias for --log (deprecated)\",\n-\t\t  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 20 },\n \t\tOPT_STRING('m', \"message\", &message, \"text\",\n \t\t\t\"use <text> as start of message\"),\n \t\tOPT_FILENAME('F', \"file\", &inpath, \"file to read from\"),\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"148601","messageId":"1282331702-5115-6-git-send-email-artagnon@gmail.com","threadId":"24810","inReplyTo":"1282331702-5115-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v3 5/5] fmt-merge-msg: Update fmt-merge-msg and merge-config documentation","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-20T19:15:02Z","receivedAt":"2010-08-20T19:15:02Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Update the documentation of fmt-merge-msg and merge-config to reflect\nthe fact that `merge.log` can either be a boolean or integer option\nnow, instead of just a boolean.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Documentation/git-fmt-merge-msg.txt |   20 +++++++++-----------\n Documentation/merge-config.txt      |    8 ++++++--\n 2 files changed, 15 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-fmt-merge-msg.txt b/Documentation/git-fmt-merge-msg.txt\nindex 78c8a6d..4149a1f 100644\n--- a/Documentation/git-fmt-merge-msg.txt\n+++ b/Documentation/git-fmt-merge-msg.txt\n@@ -9,8 +9,8 @@ git-fmt-merge-msg - Produce a merge commit message\n SYNOPSIS\n --------\n [verse]\n-'git fmt-merge-msg' [-m <message>] [--log | --no-log] <$GIT_DIR/FETCH_HEAD\n-'git fmt-merge-msg' [-m <message>] [--log | --no-log] -F <file>\n+'git fmt-merge-msg' [-m <message>] [--log[=<n>] | --no-log] < $GIT_DIR/FETCH_HEAD\n+'git fmt-merge-msg' [-m <message>] [--log[=<n>] | --no-log] -F <file>\n \n DESCRIPTION\n -----------\n@@ -24,19 +24,14 @@ automatically invoking 'git merge'.\n OPTIONS\n -------\n \n---log::\n+--log=<n>::\n \tIn addition to branch names, populate the log message with\n-\tone-line descriptions from the actual commits that are being\n-\tmerged.\n-\n --no-log::\n \tDo not list one-line descriptions from the actual commits being\n \tmerged.\n \n---summary::\n---no-summary::\n-\tSynonyms to --log and --no-log; these are deprecated and will be\n-\tremoved in the future.\n+\tone-line descriptions from at most <n> actual commits that are\n+\tbeing merged.\n \n -m <message>::\n --message <message>::\n@@ -53,7 +48,10 @@ CONFIGURATION\n \n merge.log::\n \tWhether to include summaries of merged commits in newly\n-\tmerge commit messages. False by default.\n+\tcreated merge commit messages.  Optionally, an integer can be\n+\tused to specify how many merged commits to summarize (at\n+\tmaxmium) in the merge message. Specifying \"true\" is equivalent\n+\tto specifying 20. Defaults to false.\n \n SEE ALSO\n --------\ndiff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt\nindex b72f533..f63f7cb 100644\n--- a/Documentation/merge-config.txt\n+++ b/Documentation/merge-config.txt\n@@ -7,8 +7,12 @@ merge.conflictstyle::\n \tmarker and the original text before the `=======` marker.\n \n merge.log::\n-\tWhether to include summaries of merged commits in newly created\n-\tmerge commit messages. False by default.\n+\tWhether to include summaries of merged commits in newly\n+\tcreated merge commit messages.  Optionally, an integer can be\n+\tused to specify how many merged commits to summarize (at\n+\tmaxmium) in the merge message. Specifying \"true\" is equivalent\n+\tto specifying 20.  Defaults to false. See also\n+\tlinkgit:git-fmt-merge-msg[1].\n \n merge.renameLimit::\n \tThe number of files to consider when performing rename detection\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"148603","messageId":"7vlj81awtc.fsf@alter.siamese.dyndns.org","threadId":"24810","inReplyTo":"1282331702-5115-2-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH v3 1/5] parse-options: Allow PARSE_OPT_NOARG in integer arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-20T19:49:51Z","receivedAt":"2010-08-20T19:49:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> When the option parser encounters an OPTION_INTEGER argument,\n> PARSE_OPT_NOARG should imply that the default value should be used.\n\nSorry but why?\n\nDoesn't NOARG mean \"Do not take an argument, if you give me an argument\nthat is an error\"?\n\nI would understand if this were OPT_OPTARG, though.\n\nConfused...\n\n>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> Cc: Jakub Narebski <jnareb@gmail.com>\n> ---\n>  parse-options.c |    3 +++\n>  1 files changed, 3 insertions(+), 0 deletions(-)\n>\n> diff --git a/parse-options.c b/parse-options.c\n> index e0c3641..7ec9886 100644\n> --- a/parse-options.c\n> +++ b/parse-options.c\n> @@ -138,6 +138,9 @@ static int get_value(struct parse_opt_ctx_t *p,\n>  \t\t\t*(int *)opt->value = 0;\n>  \t\t\treturn 0;\n>  \t\t}\n> +\t\tif (opt->flags & PARSE_OPT_NOARG)\n> +\t\t\t*(int *)opt->value = opt->defval;\n> +\t\t\treturn 0;\n>  \t\tif (opt->flags & PARSE_OPT_OPTARG && !p->opt) {\n>  \t\t\t*(int *)opt->value = opt->defval;\n>  \t\t\treturn 0;\n> -- \n> 1.7.2.2.409.gdbb11.dirty\n"},{"id":"148605","messageId":"20100820200148.GE6211@kytes","threadId":"24810","inReplyTo":"7vlj81awtc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 1/5] parse-options: Allow PARSE_OPT_NOARG in integer arguments","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-20T20:01:51Z","receivedAt":"2010-08-20T20:01:51Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Junio,\n\nJunio C Hamano writes:\n> Ramkumar Ramachandra <artagnon@gmail.com> writes:\n> \n> > When the option parser encounters an OPTION_INTEGER argument,\n> > PARSE_OPT_NOARG should imply that the default value should be used.\n> \n> Sorry but why?\n> \n> Doesn't NOARG mean \"Do not take an argument, if you give me an argument\n> that is an error\"?\n\nOh, does it mean that? I might have interpreted the description in\n`parse-options.h` too literally: \"says that this option takes no\nargument\". So I'm handling the case when an integer option is\nspecified, but no integer argument is given.\n\n> I would understand if this were OPT_OPTARG, though.\n\nThat case is already handled. The condition (opt->flags & PARSE_OPT_OPTARG &&\n!p->opt) does the same thing.\n\n> Confused...\n\nOkay, let me explain. Let's say I want to have an option that takes an\ninteger argument, say `foo`. To set it to the integer argument 42, I\ncan say `--foo=42`. To set it to its default value, I could earlier\nsay `--foo=`. With this patch I can simply say `--foo`. Makes sense?\n\n-- Ram\n"},{"id":"148621","messageId":"20100821033916.GA2206@burratino","threadId":"24810","inReplyTo":"20100820200148.GE6211@kytes","subject":"Re: [PATCH v3 1/5] parse-options: Allow PARSE_OPT_NOARG in integer arguments","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-21T03:39:16Z","receivedAt":"2010-08-21T03:39:16Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Ram,\n\n[rearranged for convenience]\nRamkumar Ramachandra wrote:\n\n> Let's say I want to have an option that takes an\n> integer argument, say `foo`. To set it to the integer argument 42, I\n> can say `--foo=42`. To set it to its default value, I could earlier\n> say `--foo=`. With this patch I can simply say `--foo`. Makes sense?\n\nI think you want OPTARG (\"optional argument\").\n\nWhat your patch would allow is using OPTION_INTEGER for boolean\noptions, where people use OPT_SET_INT now.  Maybe that would be a good\ncleanup, but I am not convinced it is worth the churn.\n\n> Junio C Hamano writes:\n\n>> Doesn't NOARG mean \"Do not take an argument, if you give me an argument\n>> that is an error\"?\n>\n> Oh, does it mean that?\n\nYes.\n\n-- 8< --\nSubject: parse-options: clarify PARSE_OPT_NOARG description\n\nHere \"takes no argument\" means \"does not take an argument\".  The\nlatter phrasing might make it clearer that PARSE_OPT_NOARG does not\nmake an option with an argument that can optionally be left off.\n\nNoticed-by: Ramkumar Ramachandra <artagnon@gmail.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\ndiff --git a/parse-options.h b/parse-options.h\nindex 7435cdb..d982f0f 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -69,7 +69,7 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset);\n  * `flags`::\n  *   mask of parse_opt_option_flags.\n  *   PARSE_OPT_OPTARG: says that the argument is optional (not for BOOLEANs)\n- *   PARSE_OPT_NOARG: says that this option takes no argument\n+ *   PARSE_OPT_NOARG: says that this option does not take an argument\n  *   PARSE_OPT_NONEG: says that this option cannot be negated\n  *   PARSE_OPT_HIDDEN: this option is skipped in the default usage, and\n  *                     shown only in the full usage.\n-- \n"},{"id":"148622","messageId":"20100821035403.GA2474@burratino","threadId":"24810","inReplyTo":"1282331702-5115-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH v3 2/5] fmt-merge-msg: Make the number of log entries in commit message configurable","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-21T03:54:03Z","receivedAt":"2010-08-21T03:54:03Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> +++ b/builtin/fmt-merge-msg.c\n> @@ -12,16 +12,23 @@ static const char * const fmt_merge_msg_usage[] = {\n>  };\n>  \n>  static int merge_summary;\n> +static int log_limit = 0;\n>  \n>  static int fmt_merge_msg_config(const char *key, const char *value, void *cb)\n>  {\n>  \tstatic int found_merge_log = 0;\n> +\tint is_bool = 0;\n>  \tif (!strcmp(\"merge.log\", key)) {\n>  \t\tfound_merge_log = 1;\n> -\t\tmerge_summary = git_config_bool(key, value);\n> +\t\tlog_limit = git_config_bool_or_int(key, value, &is_bool);\n>  \t}\n>  \tif (!found_merge_log && !strcmp(\"merge.summary\", key))\n> -\t\tmerge_summary = git_config_bool(key, value);\n> +\t\tlog_limit = git_config_bool_or_int(key, value, &is_bool);\n> +\n> +\tif (is_bool && log_limit)\n> +\t\tlog_limit = 20;\n> +\tmerge_summary = log_limit ? 1 : 0;\n\nHmm, this seems to be trying to have it both ways.  It would be simpler to\neither:\n\n\tstatic int merge_summary;\n\tstatic int log_limit = 20;\n\nproviding independent internal \"enabled\" and \"limit\" knobs, so one could use,\nsay,\n\n\t[merge]\n\t\tlog = 2\n\t\tlog = false\n\t\tlog = true\n\nwith the result being be a log_limit of 2, or\n\n\tstatic int log_limit;\n\nwhere 0 means disabled, so in that example the result would be a log_limit\nof 20.\n\n> @@ -140,7 +147,7 @@ static void print_joined(const char *singular, const char *plural,\n>  }\n>  \n>  static void shortlog(const char *name, unsigned char *sha1,\n> -\t\tstruct commit *head, struct rev_info *rev, int limit,\n> +\t\tstruct commit *head, struct rev_info *rev,\n>  \t\tstruct strbuf *out)\n\nA part of me wishes we would still pass the limit around (for no good\nreason), but you are probably right that it is easier to work with the\nglobal.\n\n> @@ -257,7 +264,7 @@ static void do_fmt_merge_msg_title(struct strbuf *out,\n> \n>  static int do_fmt_merge_msg(int merge_title, int merge_summary,\n\nWhat happened to the merge_summary argument?\n"},{"id":"148623","messageId":"20100821040203.GA2636@burratino","threadId":"24810","inReplyTo":"1282331702-5115-4-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH v3 3/5] fmt-merge-msg: Update command line options to sync with config options","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-21T04:02:03Z","receivedAt":"2010-08-21T04:02:03Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> +++ b/builtin/fmt-merge-msg.c\n> @@ -7,11 +7,10 @@\n>  #include \"string-list.h\"\n>  \n>  static const char * const fmt_merge_msg_usage[] = {\n> -\t\"git fmt-merge-msg [-m <message>] [--log|--no-log] [--file <file>]\",\n> +\t\"git fmt-merge-msg [-m <message>] [--log=[<n>]|--no-log] [--file <file>]\",\n\n[--log[=<n>]], no?\n\n>  \tNULL\n>  };\n>  \n> -static int merge_summary;\n>  static int log_limit = 0;\n\nAh, so you elminate the boolean arg here. :)\n\nNit: the usual style in git is to let statics and globals that should\nbe 0 be implicitly initialized.\n\n> @@ -328,10 +326,12 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)\n>  \tconst char *inpath = NULL;\n>  \tconst char *message = NULL;\n>  \tstruct option options[] = {\n> -\t\tOPT_BOOLEAN(0, \"log\",     &merge_summary, \"populate log with the shortlog\"),\n> -\t\t{ OPTION_BOOLEAN, 0, \"summary\", &merge_summary, NULL,\n> +\t\t{ OPTION_INTEGER, 0, \"log\", &log_limit, \"n\",\n> +\t\t  \"populate log with <n> entries from shortlog\",\n> +\t\t  PARSE_OPT_NOARG, NULL, 20 },\n> +\t\t{ OPTION_INTEGER, 0, \"summary\", &log_limit, \"n\",\n>  \t\t  \"alias for --log (deprecated)\",\n> -\t\t  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },\n> +\t\t  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 20 },\n\nOPTARG, I think.\n"},{"id":"148624","messageId":"20100821040421.GB2636@burratino","threadId":"24810","inReplyTo":"1282331702-5115-5-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH v3 4/5] fmt-merge-msg: Remove deprecated --summary option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-21T04:04:21Z","receivedAt":"2010-08-21T04:04:21Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Remove the deprecated --summary option\n\nWe stopped advertising it in v1.7.1-rc0~3^2, right?  That would make\nthis v1.8 or later material, I think.\n"},{"id":"148638","messageId":"20100821054903.GG6211@kytes","threadId":"24810","inReplyTo":"20100821040421.GB2636@burratino","subject":"Re: [PATCH v3 4/5] fmt-merge-msg: Remove deprecated --summary option","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-21T05:49:05Z","receivedAt":"2010-08-21T05:49:05Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder writes:\n> Ramkumar Ramachandra wrote:\n> \n> > Remove the deprecated --summary option\n> \n> We stopped advertising it in v1.7.1-rc0~3^2, right?  That would make\n> this v1.8 or later material, I think.\n\nOh. I should drop this patch while re-rolling?\n\n-- Ram\n"},{"id":"148639","messageId":"20100821055152.GH6211@kytes","threadId":"24810","inReplyTo":"20100821040203.GA2636@burratino","subject":"Re: [PATCH v3 3/5] fmt-merge-msg: Update command line options to sync with config options","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-21T05:51:54Z","receivedAt":"2010-08-21T05:51:54Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder writes:\n> Ramkumar Ramachandra wrote:\n> \n> > +++ b/builtin/fmt-merge-msg.c\n> > @@ -7,11 +7,10 @@\n> >  #include \"string-list.h\"\n> >  \n> >  static const char * const fmt_merge_msg_usage[] = {\n> > -\t\"git fmt-merge-msg [-m <message>] [--log|--no-log] [--file <file>]\",\n> > +\t\"git fmt-merge-msg [-m <message>] [--log=[<n>]|--no-log] [--file <file>]\",\n> \n> [--log[=<n>]], no?\n\nRight, my bad.\n\n> > @@ -328,10 +326,12 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)\n> >  \tconst char *inpath = NULL;\n> >  \tconst char *message = NULL;\n> >  \tstruct option options[] = {\n> > -\t\tOPT_BOOLEAN(0, \"log\",     &merge_summary, \"populate log with the shortlog\"),\n> > -\t\t{ OPTION_BOOLEAN, 0, \"summary\", &merge_summary, NULL,\n> > +\t\t{ OPTION_INTEGER, 0, \"log\", &log_limit, \"n\",\n> > +\t\t  \"populate log with <n> entries from shortlog\",\n> > +\t\t  PARSE_OPT_NOARG, NULL, 20 },\n> > +\t\t{ OPTION_INTEGER, 0, \"summary\", &log_limit, \"n\",\n> >  \t\t  \"alias for --log (deprecated)\",\n> > -\t\t  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },\n> > +\t\t  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 20 },\n> \n> OPTARG, I think.\n\nOPTARG works. Sorry for the NOARG confusion.\n\n-- Ram\n"},{"id":"148640","messageId":"20100821055556.GI6211@kytes","threadId":"24810","inReplyTo":"20100821035403.GA2474@burratino","subject":"Re: [PATCH v3 2/5] fmt-merge-msg: Make the number of log entries in commit message configurable","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-21T05:55:59Z","receivedAt":"2010-08-21T05:55:59Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder writes:\n> Hmm, this seems to be trying to have it both ways.  It would be simpler to\n> either:\n> \n> \tstatic int merge_summary;\n> \tstatic int log_limit = 20;\n> \n> providing independent internal \"enabled\" and \"limit\" knobs, so one could use,\n> say,\n> \n> \t[merge]\n> \t\tlog = 2\n> \t\tlog = false\n> \t\tlog = true\n> \n> with the result being be a log_limit of 2, or\n> \n> \tstatic int log_limit;\n> \n> where 0 means disabled, so in that example the result would be a log_limit\n> of 20.\n> \n[...]\n> What happened to the merge_summary argument?\n\nThis patch is roughly equivalent to my original log_limit patch. Yes,\nI'm trying to have it both ways in this patch :) merge_summary is set\nby both the command-line option and the config option, while log_limit\nis set by the config option. As you noticed, the next patch removes\nmerge_summary.\n\n-- Ram\n"}]}