# [PATCH 0/3] rr/fmt-merge-msg replacements 12 messages from 2010-08-28 to 2010-09-01. Participants: Ramkumar Ramachandra, Jonathan Nieder, Junio C Hamano. Thread: https://gitlist.dev/t/24892 ## Ramkumar Ramachandra, 2010-08-28 10:35 Subject: [PATCH 0/3] rr/fmt-merge-msg replacements Message-ID: <1282991734-3368-1-git-send-email-artagnon@gmail.com> URL: https://gitlist.dev/e/1282991734-3368-1-git-send-email-artagnon%40gmail.com ``` I found a problem while writing the test for -1: the first patch is a correction for that; sorry I didn't detect it earlier. The second and third patches are rewrites of the tests I submitted earlier- the rest of the file needs to be cleaned up to match this style. Thanks Jonathan. Ramkumar Ramachandra (3): fmt-merge-msg: Disallow '--log', 'merge.log' < 0 t6200-fmt-merge-msg: Exercise 'merge.log' to configure shortlog length t6200-fmt-merge-msg: Exercise '--log' to configure shortlog length builtin/fmt-merge-msg.c | 4 +++- builtin/merge.c | 2 +- t/t6200-fmt-merge-msg.sh | 123 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 2 deletions(-) -- 1.7.2.2.409.gdbb11.dirty ``` ## Ramkumar Ramachandra, 2010-08-28 10:35 Subject: [PATCH] fmt-merge-msg: Disallow '--log', 'merge.log' < 0 Message-ID: <1282991734-3368-2-git-send-email-artagnon@gmail.com> URL: https://gitlist.dev/e/1282991734-3368-2-git-send-email-artagnon%40gmail.com In-Reply-To: <1282991734-3368-1-git-send-email-artagnon@gmail.com> ``` Explicitly check for the case when '--log' or 'merge.log' is passed a negative integer argument. Handle the case by making it look as if such an argument was never parsed. Signed-off-by: Ramkumar Ramachandra --- builtin/fmt-merge-msg.c | 4 +++- builtin/merge.c | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index d3b8297..0684179 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -18,7 +18,7 @@ static int fmt_merge_msg_config(const char *key, const char *value, void *cb) if (!strcmp(key, "merge.log") || !strcmp(key, "merge.summary")) { int is_bool; shortlog_len = git_config_bool_or_int(key, value, &is_bool); - if (is_bool && shortlog_len) + if ((is_bool && shortlog_len) || shortlog_len < 0) shortlog_len = DEFAULT_MERGE_LOG_LEN; } return 0; @@ -345,6 +345,8 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix) write_in_full(STDOUT_FILENO, &nl, 1); return 0; } + if (shortlog_len < 0) + shortlog_len = DEFAULT_MERGE_LOG_LEN; if (inpath && strcmp(inpath, "-")) { in = fopen(inpath, "r"); diff --git a/builtin/merge.c b/builtin/merge.c index 55dc571..affa197 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -510,7 +510,7 @@ static int git_merge_config(const char *k, const char *v, void *cb) else if (!strcmp(k, "merge.log") || !strcmp(k, "merge.summary")) { int is_bool; shortlog_len = git_config_bool_or_int(k, v, &is_bool); - if (is_bool && shortlog_len) + if ((is_bool && shortlog_len) || shortlog_len < 0) shortlog_len = DEFAULT_MERGE_LOG_LEN; return 0; } else if (!strcmp(k, "merge.renormalize")) -- 1.7.2.2.409.gdbb11.dirty ``` ## Ramkumar Ramachandra, 2010-08-28 10:35 Subject: [PATCH 2/3] t6200-fmt-merge-msg: Exercise 'merge.log' to configure shortlog length Message-ID: <1282991734-3368-3-git-send-email-artagnon@gmail.com> URL: https://gitlist.dev/e/1282991734-3368-3-git-send-email-artagnon%40gmail.com In-Reply-To: <1282991734-3368-1-git-send-email-artagnon@gmail.com> ``` Add a test to exercise the 'merge.log' configuration option of 'git fmt-merge-msg'. It controls the number of shortlog entries to display in merge commit messages. Signed-off-by: Ramkumar Ramachandra Thanks-to: Jonathan Nieder --- t/t6200-fmt-merge-msg.sh | 64 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 64 insertions(+), 0 deletions(-) diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh index 71f6cad..528fdf9 100755 --- a/t/t6200-fmt-merge-msg.sh +++ b/t/t6200-fmt-merge-msg.sh @@ -129,6 +129,70 @@ test_expect_success '[merge] summary/log configuration' ' test_cmp expected actual2 ' +test_expect_success 'setup: clear [merge] configuration' ' + test_might_fail git config --unset-all merge.log && + test_might_fail git config --unset-all merge.summary +' + +test_expect_success 'setup FETCH_HEAD' ' + git checkout master && + test_tick && + git fetch . left +' + +test_expect_success 'merge.log=3 limits shortlog length' ' + cat >expected <<-EOF && + Merge branch ${apos}left${apos} + + * left: (5 commits) + Left #5 + Left #4 + Left #3 + ... + EOF + + git -c merge.log=3 fmt-merge-msg <.git/FETCH_HEAD >actual && + test_cmp expected actual +' + +test_expect_success 'merge.log=5 shows all 5 commits' ' + cat >expected <<-EOF && + Merge branch ${apos}left${apos} + + * left: + Left #5 + Left #4 + Left #3 + Common #2 + Common #1 + EOF + + git -c merge.log=5 fmt-merge-msg <.git/FETCH_HEAD >actual && + test_cmp expected actual +' + +test_expect_success 'merge.log=0 disables shortlog' ' + echo "Merge branch ${apos}left${apos}" >expected + git -c merge.log=0 fmt-merge-msg <.git/FETCH_HEAD >actual && + test_cmp expected actual +' + +test_expect_success 'merge.log=-1 does something sane' ' + cat >expected <<-EOF && + Merge branch ${apos}left${apos} + + * left: + Left #5 + Left #4 + Left #3 + Common #2 + Common #1 + EOF + + git -c merge.log=-1 fmt-merge-msg <.git/FETCH_HEAD >actual && + test_cmp expected actual +' + test_expect_success 'fmt-merge-msg -m' ' echo "Sync with left" >expected && cat >expected.log <<-EOF && -- 1.7.2.2.409.gdbb11.dirty ``` ## Ramkumar Ramachandra, 2010-08-28 10:35 Subject: [PATCH 3/3] t6200-fmt-merge-msg: Exercise '--log' to configure shortlog length Message-ID: <1282991734-3368-4-git-send-email-artagnon@gmail.com> URL: https://gitlist.dev/e/1282991734-3368-4-git-send-email-artagnon%40gmail.com In-Reply-To: <1282991734-3368-1-git-send-email-artagnon@gmail.com> ``` Add a test to exercise the '--log' command-line option of 'git fmt-merge-msg'. It controls the number of shortlog entries to display in merge commit messages. Signed-off-by: Ramkumar Ramachandra Thanks-to: Jonathan Nieder --- t/t6200-fmt-merge-msg.sh | 59 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 59 insertions(+), 0 deletions(-) diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh index 528fdf9..395c23e 100755 --- a/t/t6200-fmt-merge-msg.sh +++ b/t/t6200-fmt-merge-msg.sh @@ -193,6 +193,65 @@ test_expect_success 'merge.log=-1 does something sane' ' test_cmp expected actual ' +test_expect_success '--log=3 limits shortlog length' ' + cat >expected <<-EOF && + Merge branch ${apos}left${apos} + + * left: (5 commits) + Left #5 + Left #4 + Left #3 + ... + EOF + + git fmt-merge-msg --log=3 <.git/FETCH_HEAD >actual && + test_cmp expected actual +' + +test_expect_success '--log=5 shows all 5 commits' ' + cat >expected <<-EOF && + Merge branch ${apos}left${apos} + + * left: + Left #5 + Left #4 + Left #3 + Common #2 + Common #1 + EOF + + git fmt-merge-msg --log=5 <.git/FETCH_HEAD >actual && + test_cmp expected actual +' + +test_expect_success '--no-log disables shortlog' ' + echo "Merge branch ${apos}left${apos}" >expected && + git fmt-merge-msg --no-log <.git/FETCH_HEAD >actual && + test_cmp expected actual +' + +test_expect_success '--log=0 disables shortlog' ' + echo "Merge branch ${apos}left${apos}" >expected && + git fmt-merge-msg --no-log <.git/FETCH_HEAD >actual && + test_cmp expected actual +' + +test_expect_success '--log=-1 does something sane' ' + cat >expected <<-EOF && + Merge branch ${apos}left${apos} + + * left: + Left #5 + Left #4 + Left #3 + Common #2 + Common #1 + EOF + + git fmt-merge-msg --log=-1 <.git/FETCH_HEAD >actual && + test_cmp expected actual +' + test_expect_success 'fmt-merge-msg -m' ' echo "Sync with left" >expected && cat >expected.log <<-EOF && -- 1.7.2.2.409.gdbb11.dirty ``` ## Ramkumar Ramachandra, 2010-08-31 03:53 Subject: [PATCH 1/2] merge: Make '--log' an integer option for number of shortlog entries Message-ID: <1283226800-28980-1-git-send-email-artagnon@gmail.com> URL: https://gitlist.dev/e/1283226800-28980-1-git-send-email-artagnon%40gmail.com In-Reply-To: <1282991734-3368-1-git-send-email-artagnon@gmail.com> ``` Change the command-line '--log' option from a boolean option to an integer option, and parse the optional integer provided on the command-line into the 'shortlog_len' variable. Also update the documentation accordingly. Signed-off-by: Ramkumar Ramachandra Reported-by: Yaroslav Halchenko Thanks-to: Jonathan Nieder Thanks-to: Johannes Sixt Signed-off-by: Junio C Hamano --- Replacement for b5cdf2a. Documentation/git-fmt-merge-msg.txt | 10 ++++++---- Documentation/merge-options.txt | 6 +++--- builtin/fmt-merge-msg.c | 28 ++++++++++++++++++---------- builtin/merge.c | 17 ++++++++++------- 4 files changed, 37 insertions(+), 24 deletions(-) diff --git a/Documentation/git-fmt-merge-msg.txt b/Documentation/git-fmt-merge-msg.txt index 302f56b..f04a9ff 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 ] [--log | --no-log] <$GIT_DIR/FETCH_HEAD -'git fmt-merge-msg' [-m ] [--log | --no-log] -F +'git fmt-merge-msg' [-m ] [--log[=] | --no-log] <$GIT_DIR/FETCH_HEAD +'git fmt-merge-msg' [-m ] [--log[=] | --no-log] -F DESCRIPTION ----------- @@ -24,10 +24,12 @@ automatically invoking 'git merge'. OPTIONS ------- ---log:: +--log[=]:: In addition to branch names, populate the log message with one-line descriptions from the actual commits that are being - merged. + merged. At most commits from each merge parent will be + used (20 if is omitted). This overrides the `merge.log` + configuration variable. --no-log:: Do not list one-line descriptions from the actual commits being diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index 722d704..e33e0f8 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -16,11 +16,11 @@ inspect and further tweak the merge result before committing. With --no-ff Generate a merge commit even if the merge resolved as a fast-forward. ---log:: +--log[=]:: --no-log:: In addition to branch names, populate the log message with - one-line descriptions from the actual commits that are being - merged. + one-line descriptions from at most actual commits that are being + merged. See also linkgit:git-fmt-merge-msg[1]. + With --no-log do not list one-line descriptions from the actual commits being merged. diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index 42021d3..0997c26 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -7,21 +7,24 @@ #include "string-list.h" static const char * const fmt_merge_msg_usage[] = { - "git fmt-merge-msg [-m ] [--log|--no-log] [--file ]", + "git fmt-merge-msg [-m ] [--log[=]|--no-log] [--file ]", NULL }; -static int merge_summary; +static int shortlog_len; static int fmt_merge_msg_config(const char *key, const char *value, void *cb) { static int found_merge_log = 0; if (!strcmp("merge.log", key)) { found_merge_log = 1; - merge_summary = git_config_bool(key, value); + shortlog_len = git_config_bool(key, value) ? DEFAULT_MERGE_LOG_LEN : 0; + return 0; + } + if (!found_merge_log && !strcmp("merge.summary", key)) { + shortlog_len = git_config_bool(key, value) ? DEFAULT_MERGE_LOG_LEN : 0; + return 0; } - if (!found_merge_log && !strcmp("merge.summary", key)) - merge_summary = git_config_bool(key, value); return 0; } @@ -318,10 +321,13 @@ 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", &shortlog_len, "n", + "populate log with entries from shortlog", + PARSE_OPT_OPTARG, NULL, DEFAULT_MERGE_LOG_LEN }, + { OPTION_INTEGER, 0, "summary", &shortlog_len, "n", "alias for --log (deprecated)", - PARSE_OPT_NOARG | PARSE_OPT_HIDDEN }, + PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN, NULL, + DEFAULT_MERGE_LOG_LEN }, OPT_STRING('m', "message", &message, "text", "use as start of message"), OPT_FILENAME('F', "file", &inpath, "file to read from"), @@ -337,12 +343,14 @@ 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 && !shortlog_len) { char nl = '\n'; write_in_full(STDOUT_FILENO, message, strlen(message)); write_in_full(STDOUT_FILENO, &nl, 1); return 0; } + if (shortlog_len < 0) + shortlog_len = DEFAULT_MERGE_LOG_LEN; if (inpath && strcmp(inpath, "-")) { in = fopen(inpath, "r"); @@ -357,7 +365,7 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix) strbuf_addstr(&output, message); ret = fmt_merge_msg(&input, &output, message ? 0 : 1, - merge_summary ? DEFAULT_MERGE_LOG_LEN : 0); + shortlog_len); if (ret) return ret; diff --git a/builtin/merge.c b/builtin/merge.c index b2c0984..9e4733d 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -42,7 +42,7 @@ static const char * const builtin_merge_usage[] = { NULL }; -static int show_diffstat = 1, option_log, squash; +static int show_diffstat = 1, shortlog_len, squash; static int option_commit = 1, allow_fast_forward = 1; static int fast_forward_only; static int allow_trivial = 1, have_message; @@ -175,8 +175,9 @@ static struct option builtin_merge_options[] = { OPT_BOOLEAN(0, "stat", &show_diffstat, "show a diffstat at the end of the merge"), OPT_BOOLEAN(0, "summary", &show_diffstat, "(synonym to --stat)"), - OPT_BOOLEAN(0, "log", &option_log, - "add list of one-line log to merge commit message"), + { OPTION_INTEGER, 0, "log", &shortlog_len, "n", + "add (at most ) entries from shortlog to merge commit message", + PARSE_OPT_OPTARG, NULL, DEFAULT_MERGE_LOG_LEN }, OPT_BOOLEAN(0, "squash", &squash, "create a single commit instead of doing a merge"), OPT_BOOLEAN(0, "commit", &option_commit, @@ -501,8 +502,10 @@ static int git_merge_config(const char *k, const char *v, void *cb) return git_config_string(&pull_twohead, k, v); else if (!strcmp(k, "pull.octopus")) return git_config_string(&pull_octopus, k, v); - else if (!strcmp(k, "merge.log") || !strcmp(k, "merge.summary")) - option_log = git_config_bool(k, v); + else if (!strcmp(k, "merge.log") || !strcmp(k, "merge.summary")) { + shortlog_len = git_config_bool(k, v) ? DEFAULT_MERGE_LOG_LEN : 0; + return 0; + } return git_diff_ui_config(k, v, cb); } @@ -998,9 +1001,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix) for (i = 0; i < argc; i++) merge_name(argv[i], &merge_names); - if (!have_message || option_log) { + if (!have_message || shortlog_len) { fmt_merge_msg(&merge_names, &merge_msg, !have_message, - option_log ? DEFAULT_MERGE_LOG_LEN : 0); + shortlog_len); if (merge_msg.len) strbuf_setlen(&merge_msg, merge_msg.len - 1); } -- 1.7.2.2.409.gdbb11.dirty ``` ## Ramkumar Ramachandra, 2010-08-31 03:53 Subject: [PATCH 2/2] merge: Make 'merge.log' an integer or boolean option Message-ID: <1283226800-28980-2-git-send-email-artagnon@gmail.com> URL: https://gitlist.dev/e/1283226800-28980-2-git-send-email-artagnon%40gmail.com In-Reply-To: <1282991734-3368-1-git-send-email-artagnon@gmail.com> ``` Make 'merge.log' an integer or boolean option to set the number of shortlog entries to display in the merge commit. Note that it defaults to false, and that true means a default value of 20. Also update corresponding documentation. Signed-off-by: Ramkumar Ramachandra Thanks-to: Jonathan Nieder Thanks-to: Johannes Sixt Signed-off-by: Junio C Hamano --- Replacement for afdcbdc. Documentation/git-fmt-merge-msg.txt | 6 ++++-- Documentation/merge-config.txt | 6 ++++-- builtin/fmt-merge-msg.c | 14 +++++--------- builtin/merge.c | 5 ++++- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/Documentation/git-fmt-merge-msg.txt b/Documentation/git-fmt-merge-msg.txt index f04a9ff..40dba8c 100644 --- a/Documentation/git-fmt-merge-msg.txt +++ b/Documentation/git-fmt-merge-msg.txt @@ -54,8 +54,10 @@ CONFIGURATION ------------- merge.log:: - Whether to include summaries of merged commits in newly - merge commit messages. False by default. + In addition to branch names, populate the log message with at + most the specified number of one-line descriptions from the + actual commits that are being merged. Defaults to false, and + true is a synoym for 20. merge.summary:: Synonym to `merge.log`; this is deprecated and will be removed in diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt index a403155..acbe1e1 100644 --- a/Documentation/merge-config.txt +++ b/Documentation/merge-config.txt @@ -7,8 +7,10 @@ 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. + In addition to branch names, populate the log message with at + most the specified number of one-line descriptions from the + actual commits that are being merged. Defaults to false, and + true is a synoym for 20. merge.renameLimit:: The number of files to consider when performing rename detection diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index 0997c26..975ef6a 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -15,15 +15,11 @@ static int shortlog_len; static int fmt_merge_msg_config(const char *key, const char *value, void *cb) { - static int found_merge_log = 0; - if (!strcmp("merge.log", key)) { - found_merge_log = 1; - shortlog_len = git_config_bool(key, value) ? DEFAULT_MERGE_LOG_LEN : 0; - return 0; - } - if (!found_merge_log && !strcmp("merge.summary", key)) { - shortlog_len = git_config_bool(key, value) ? DEFAULT_MERGE_LOG_LEN : 0; - return 0; + if (!strcmp(key, "merge.log") || !strcmp(key, "merge.summary")) { + int is_bool; + shortlog_len = git_config_bool_or_int(key, value, &is_bool); + if (is_bool && shortlog_len || shortlog_len < 0) + shortlog_len = DEFAULT_MERGE_LOG_LEN; } return 0; } diff --git a/builtin/merge.c b/builtin/merge.c index 9e4733d..57ea8d7 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -503,7 +503,10 @@ static int git_merge_config(const char *k, const char *v, void *cb) else if (!strcmp(k, "pull.octopus")) return git_config_string(&pull_octopus, k, v); else if (!strcmp(k, "merge.log") || !strcmp(k, "merge.summary")) { - shortlog_len = git_config_bool(k, v) ? DEFAULT_MERGE_LOG_LEN : 0; + int is_bool; + shortlog_len = git_config_bool_or_int(k, v, &is_bool); + if (is_bool && shortlog_len || shortlog_len < 0) + shortlog_len = DEFAULT_MERGE_LOG_LEN; return 0; } return git_diff_ui_config(k, v, cb); -- 1.7.2.2.409.gdbb11.dirty ``` ## Jonathan Nieder, 2010-08-31 14:32 Subject: Re: [PATCH 1/2] merge: Make '--log' an integer option for number of shortlog entries Message-ID: <20100831143236.GC2315@burratino> URL: https://gitlist.dev/e/20100831143236.GC2315%40burratino In-Reply-To: <1283226800-28980-1-git-send-email-artagnon@gmail.com> ``` Ramkumar Ramachandra wrote: > Change the command-line '--log' option from a boolean option to an > integer option What does "git merge --log=-1" do? It looks like you have made "git fmt-merge-msg --log=-1" equivalent to --log=20, but it might be better to error out or use INT_MAX or something (especially because of plans to make -1 mean "infinity" later). Thanks for your perseverance. Aside from those two nits, the patch looks good. ``` ## Junio C Hamano, 2010-08-31 18:17 Subject: Re: [PATCH 1/2] merge: Make '--log' an integer option for number of shortlog entries Message-ID: <7vzkw2abpz.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vzkw2abpz.fsf%40alter.siamese.dyndns.org In-Reply-To: <20100831143236.GC2315@burratino> ``` Jonathan Nieder writes: > Ramkumar Ramachandra wrote: > >> Change the command-line '--log' option from a boolean option to an >> integer option > > What does "git merge --log=-1" do? > > It looks like you have made "git fmt-merge-msg --log=-1" equivalent to > --log=20, but it might be better to error out or use INT_MAX or > something (especially because of plans to make -1 mean "infinity" > later). Hmm, do we plan on that? It sounds a little bit insane. Erroring it out would be Ok. How about doing this on top? builtin/fmt-merge-msg.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index 0997c26..7c6e226 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -322,7 +322,7 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix) const char *message = NULL; struct option options[] = { { OPTION_INTEGER, 0, "log", &shortlog_len, "n", - "populate log with entries from shortlog", + "populate log with at most entries from shortlog", PARSE_OPT_OPTARG, NULL, DEFAULT_MERGE_LOG_LEN }, { OPTION_INTEGER, 0, "summary", &shortlog_len, "n", "alias for --log (deprecated)", @@ -350,7 +350,7 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix) return 0; } if (shortlog_len < 0) - shortlog_len = DEFAULT_MERGE_LOG_LEN; + die("Negative --log=%d???", shortlog_len); if (inpath && strcmp(inpath, "-")) { in = fopen(inpath, "r"); ``` ## Jonathan Nieder, 2010-08-31 18:21 Subject: Re: [PATCH 1/2] merge: Make '--log' an integer option for number of shortlog entries Message-ID: <20100831182120.GR2315@burratino> URL: https://gitlist.dev/e/20100831182120.GR2315%40burratino In-Reply-To: <7vzkw2abpz.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano wrote: > Hmm, do we plan on that? It sounds a little bit insane. I sent an RFC patch with -1 as internal representation for "git grep -Cinfinity". Infinity was the length Yaroslav was most interested in for merge.log (not sure why, really). Regardless, it's true that that would probably not be a detail to expose. > How about doing this on top? [...] > --- a/builtin/fmt-merge-msg.c > +++ b/builtin/fmt-merge-msg.c > @@ -322,7 +322,7 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix) > const char *message = NULL; > struct option options[] = { > { OPTION_INTEGER, 0, "log", &shortlog_len, "n", > - "populate log with entries from shortlog", > + "populate log with at most entries from shortlog", > PARSE_OPT_OPTARG, NULL, DEFAULT_MERGE_LOG_LEN }, > { OPTION_INTEGER, 0, "summary", &shortlog_len, "n", > "alias for --log (deprecated)", > @@ -350,7 +350,7 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix) > return 0; > } > if (shortlog_len < 0) > - shortlog_len = DEFAULT_MERGE_LOG_LEN; > + die("Negative --log=%d???", shortlog_len); Looks good to me (modulo punctuation :)). ``` ## Ramkumar Ramachandra, 2010-08-31 19:13 Subject: Re: [PATCH 1/2] merge: Make '--log' an integer option for number of shortlog entries Message-ID: <20100831191324.GA27381@kytes> URL: https://gitlist.dev/e/20100831191324.GA27381%40kytes In-Reply-To: <7vzkw2abpz.fsf@alter.siamese.dyndns.org> ``` Hi, Junio C Hamano writes: > Hmm, do we plan on that? It sounds a little bit insane. Erroring it out > would be Ok. How about doing this on top? > @@ -350,7 +350,7 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix) > return 0; > } > if (shortlog_len < 0) > - shortlog_len = DEFAULT_MERGE_LOG_LEN; > + die("Negative --log=%d???", shortlog_len); > > if (inpath && strcmp(inpath, "-")) { > in = fopen(inpath, "r"); It's a little asymmetric- the program won't error out when 'merge.log' is set to -1. Then again, it's probably nicer to simply pretend that we didn't see that invalid configuration. Also, to avoid breaking the '--log=-1' test, either drop the test altogether or use something like this: Signed-off-by: Ramkumar Ramachandra diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh index 395c23e..3bfdebc 100755 --- a/t/t6200-fmt-merge-msg.sh +++ b/t/t6200-fmt-merge-msg.sh @@ -236,7 +236,7 @@ test_expect_success '--log=0 disables shortlog' ' test_cmp expected actual ' -test_expect_success '--log=-1 does something sane' ' +test_expect_code 128 '--log=-1 errors out' ' cat >expected <<-EOF && Merge branch ${apos}left${apos} @@ -248,8 +248,7 @@ test_expect_success '--log=-1 does something sane' ' Common #1 EOF - git fmt-merge-msg --log=-1 <.git/FETCH_HEAD >actual && - test_cmp expected actual + git fmt-merge-msg --log=-1 <.git/FETCH_HEAD >actual ' test_expect_success 'fmt-merge-msg -m' ' ``` ## Junio C Hamano, 2010-08-31 19:53 Subject: Re: [PATCH 1/2] merge: Make '--log' an integer option for number of shortlog entries Message-ID: <7vfwxua79a.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vfwxua79a.fsf%40alter.siamese.dyndns.org In-Reply-To: <20100831191324.GA27381@kytes> ``` Ramkumar Ramachandra writes: > It's a little asymmetric- the program won't error out when 'merge.log' > is set to -1. And what is the reason you do not want to diagnose that as an error? ``` ## Ramkumar Ramachandra, 2010-09-01 04:22 Subject: Re: [PATCH 1/2] merge: Make '--log' an integer option for number of shortlog entries Message-ID: <20100901042247.GB26496@kytes> URL: https://gitlist.dev/e/20100901042247.GB26496%40kytes In-Reply-To: <7vfwxua79a.fsf@alter.siamese.dyndns.org> ``` Hi Junio, Junio C Hamano writes: > Ramkumar Ramachandra writes: > > > It's a little asymmetric- the program won't error out when 'merge.log' > > is set to -1. > > And what is the reason you do not want to diagnose that as an error? Oh, I'm just saying I'm not sure what the behavior should be. Okay- if erroring out is the desired result, I'll fixup and re-roll in a bit. -- Ram ```