Re: [PATCH v5 1/5] fmt_merge_msg: Change fmt_merge_msg API to accept shortlog_len

Subsystems: the rest

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v5 1/5] fmt_merge_msg: Change fmt_merge_msg API to accept shortlog_len

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:49:22

Jonathan Nieder [off-list ref] writes:
The rest looks good.  So except for the commit message,

Reviewed-by: Jonathan Nieder <redacted>
Thanks; all of your comments make sense.  Here is what I'll queue.

-- >8 --
From: Ramkumar Ramachandra <redacted>
Date: Sun, 22 Aug 2010 21:56:34 +0530
Subject: [PATCH] fmt_merge_msg: Change fmt_merge_msg API to accept shortlog_len

Give "shortlog_len" parameter to the fmt_merge_msg(), remove its
"merge_summary" parameter, and remove fmt_merge_msg_shortlog() function.
In the updated API, shortlog_len == 0 means no shortlog is given.

The parameter "merge_title" controls if the title of the merge commit is
autogenerated (it reads something like "Merge branch ..."), and typically
it is set to true when the caller does not give its own message.

Signed-off-by: Ramkumar Ramachandra <redacted>
Helped-and-Reviewed-by: Jonathan Nieder [off-list ref]
Signed-off-by: Junio C Hamano <redacted>
---
 builtin.h               |    7 ++++---
 builtin/fmt-merge-msg.c |   30 ++++++++++++++----------------
 builtin/merge.c         |   14 ++++++--------
 3 files changed, 24 insertions(+), 27 deletions(-)
diff --git a/builtin.h b/builtin.h
index ed6ee26..09b94ea 100644
--- a/builtin.h
+++ b/builtin.h
@@ -7,6 +7,8 @@
 #include "commit.h"
 #include "notes.h"
 
+#define DEFAULT_MERGE_LOG_LEN 20
+
 extern const char git_version_string[];
 extern const char git_usage_string[];
 extern const char git_more_info_string[];
@@ -14,9 +16,8 @@ extern const char git_more_info_string[];
 extern void list_common_cmds_help(void);
 extern const char *help_unknown_cmd(const char *cmd);
 extern void prune_packed_objects(int);
-extern int fmt_merge_msg(int merge_summary, struct strbuf *in,
-	struct strbuf *out);
-extern int fmt_merge_msg_shortlog(struct strbuf *in, struct strbuf *out);
+extern int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
+			 int merge_title, int shortlog_len);
 extern int commit_notes(struct notes_tree *t, const char *msg);
 
 struct notes_rewrite_cfg {
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 937d5a7..42021d3 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -255,9 +255,9 @@ 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,
-	struct strbuf *in, struct strbuf *out) {
-	int limit = 20, i = 0, pos = 0;
+static int do_fmt_merge_msg(int merge_title, struct strbuf *in,
+	struct strbuf *out, int shortlog_len) {
+	int i = 0, pos = 0;
 	unsigned char head_sha1[20];
 	const char *current_branch;
 
@@ -288,7 +288,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 (shortlog_len) {
 		struct commit *head;
 		struct rev_info rev;
 
@@ -303,17 +303,14 @@ 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, shortlog_len, out);
 	}
 	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_shortlog(struct strbuf *in, struct strbuf *out) {
-	return do_fmt_merge_msg(0, 1, in, out);
+int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
+		  int merge_title, int shortlog_len) {
+	return do_fmt_merge_msg(merge_title, in, out, shortlog_len);
 }
 
 int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
@@ -355,12 +352,13 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 
 	if (strbuf_read(&input, fileno(in), 0) < 0)
 		die_errno("could not read input file");
-	if (message) {
+
+	if (message)
 		strbuf_addstr(&output, message);
-		ret = fmt_merge_msg_shortlog(&input, &output);
-	} else {
-		ret = fmt_merge_msg(merge_summary, &input, &output);
-	}
+	ret = fmt_merge_msg(&input, &output,
+			    message ? 0 : 1,
+			    merge_summary ? DEFAULT_MERGE_LOG_LEN : 0);
+
 	if (ret)
 		return ret;
 	write_in_full(STDOUT_FILENO, output.buf, output.len);
diff --git a/builtin/merge.c b/builtin/merge.c
index 2207f79..b2c0984 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -998,14 +998,12 @@ 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)
-			fmt_merge_msg_shortlog(&merge_names, &merge_msg);
-		else if (!have_message)
-			fmt_merge_msg(option_log, &merge_names, &merge_msg);
-
-
-		if (!(have_message && !option_log) && merge_msg.len)
-			strbuf_setlen(&merge_msg, merge_msg.len-1);
+		if (!have_message || option_log) {
+			fmt_merge_msg(&merge_names, &merge_msg, !have_message,
+				      option_log ? DEFAULT_MERGE_LOG_LEN : 0);
+			if (merge_msg.len)
+				strbuf_setlen(&merge_msg, merge_msg.len - 1);
+		}
 	}
 
 	if (head_invalid || !argc)
-- 
1.7.2.2.426.g2251a

[PATCH rr/fmt-merge-msg] merge, fmt_merge_msg --log: default value is DEFAULT_MERGE_LOG_LEN

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:49:22

Most references to the DEFAULT_MERGE_LOG_LEN were changed to use that
symbolic constant, but a few uses of hardcoded "20" remain.

Cc: Ramkumar Ramachandra <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
Noticed this while looking over ab6beee (merge: Make '--log' an
integer option) from pu.  Maybe something like it could be squashed in
in the next round.
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 425900d..bc7c30f 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -323,10 +323,11 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 	struct option options[] = {
 		{ OPTION_INTEGER, 0, "log", &shortlog_len, "n",
 		  "populate log with <n> entries from shortlog",
-		  PARSE_OPT_OPTARG, NULL, 20 },
+		  PARSE_OPT_OPTARG, NULL, DEFAULT_MERGE_LOG_LEN },
 		{ OPTION_INTEGER, 0, "summary", &shortlog_len, "n",
 		  "alias for --log (deprecated)",
-		  PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN, NULL, 20 },
+		  PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN, NULL,
+		  DEFAULT_MERGE_LOG_LEN },
 		OPT_STRING('m', "message", &message, "text",
 			"use <text> as start of message"),
 		OPT_FILENAME('F', "file", &inpath, "file to read from"),
diff --git a/builtin/merge.c b/builtin/merge.c
index bf550a6..9e4733d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -177,7 +177,7 @@ static struct option builtin_merge_options[] = {
 	OPT_BOOLEAN(0, "summary", &show_diffstat, "(synonym to --stat)"),
 	{ OPTION_INTEGER, 0, "log", &shortlog_len, "n",
 	  "add (at most <n>) entries from shortlog to merge commit message",
-	  PARSE_OPT_OPTARG, NULL, 20 },
+	  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,
-- 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help