Re: [PATCH v5 2/5] merge: Make '--log' an integer option for number of shortlog entries
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:49:22
Ramkumar Ramachandra wrote:
Documentation/git-fmt-merge-msg.txt | 10 +++++----- Documentation/merge-options.txt | 6 +++--- builtin/fmt-merge-msg.c | 25 ++++++++++++++----------- builtin/merge.c | 18 ++++++++++-------- 4 files changed, 32 insertions(+), 27 deletions(-)
Is this on top of "next"? I had trouble applying it on top of the update-contrib-example-merge topic.
quoted hunk ↗ jump to hunk
--- a/Documentation/git-fmt-merge-msg.txt +++ b/Documentation/git-fmt-merge-msg.txt@@ -24,10 +24,10 @@ 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. + one-line descriptions from at most <n> actual commits that are + being merged. If omitted, <n> defaults to 20.
The description feels a bit awkward. Maybe: In addition to branch names, populate the log message with one-line descriptions from the actual commits that are being merged. At most <n> commits from each merge parent will be used (20 if <n> is omitted). This overrides the `merge.log` configuration variable.
quoted hunk ↗ jump to hunk
+++ 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[=<n>]:: --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 <n> actual commits that are being + merged. See also linkgit:git-fmt-merge-msg[1].
Maybe s/<n>/at most <n>/.
quoted hunk ↗ jump to hunk
--- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c@@ -7,21 +7,21 @@ #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 shortlog_len;
Before: merge_summary is 1 for --log, 0 for --no-log. After: shortlog_len is 20 for --log, 0 for --no-log, right?
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);
}
if (!found_merge_log && !strcmp("merge.summary", key))
- merge_summary = git_config_bool(key, value);
+ shortlog_len = git_config_bool(key, value);So should this say something like shortlog_len = git_config_bool(key, value) ? DEFAULT_MERGE_LOG_LEN : 0; ?
quoted hunk ↗ jump to hunk
@@ -318,10 +318,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, - "alias for --log (deprecated)", - PARSE_OPT_NOARG | PARSE_OPT_HIDDEN }, + { OPTION_INTEGER, 0, "log", &shortlog_len, "n", + "populate log with <n> entries from shortlog", + PARSE_OPT_OPTARG, NULL, 20 }, + { OPTION_INTEGER, 0, "summary", &shortlog_len, "n", + "alias for --log (deprecated)", + PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN, NULL, 20 },
Whitespace damage.
quoted hunk ↗ jump to hunk
+++ 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;@@ -177,8 +177,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", + "populate log with <n> entries from shortlog",
The emphasis seems wrong: the important thing is still that --log uses a shortlog, not the number of entries. Maybe something like "populate log with (at most <n>) entries from shortlog" but that might make "git merge -h" use more than 80 columns...
quoted hunk ↗ jump to hunk
OPT_BOOLEAN(0, "commit", &option_commit,@@ -505,7 +506,7 @@ 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")) - option_log = git_config_bool(k, v); + shortlog_len = git_config_bool(k, v);
As before (is this missing a " ? DEFAULT_MERGE_LOG_LEN : 0"?). Except as noted above, Reviewed-by: Jonathan Nieder <redacted>