diff mbox series

[RFC,1/3] git-merge: add option to format default message using multiple lines

Message ID 20200209131623.5827-2-darktemplar@dark-templar-archives.net (mailing list archive)
State New, archived
Headers show
Series git-merge: update default commit message | expand

Commit Message

i.Dark_Templar Feb. 9, 2020, 1:16 p.m. UTC
If a lot of objects is merged at once, default commit message
could become one very long line, which might be inconvenient to read.
This change implements an option to change default autogenerated message
so it'd take multiple lines, but each line wouldn't be long.

An artificial example.

Original merge commit message:
    Merge branch 'branch_with_some_long_name_1', remote-tracking branch 'clone/remote_branch_with_some_name', tags 'some_tag' and 'some_other_tag'; commit 'ae46a39cead2b42282abce725e90b561c06e94ba'; commit '33d0281e0eeb2a5e9907ebedc230e28c46865092' into merge7

Multiline merge commit message:
    Merge into merge8:
    branch 'branch_with_some_long_name_1'
    remote-tracking branch 'clone/remote_branch_with_some_name'
    tag 'some_tag'
    tag 'some_other_tag'
    commit 'ae46a39cead2b42282abce725e90b561c06e94ba'
    commit '33d0281e0eeb2a5e9907ebedc230e28c46865092'

Signed-off-by: i.Dark_Templar <darktemplar@dark-templar-archives.net>
---
 Documentation/config/fmt-merge-msg.txt |  6 +++
 builtin/fmt-merge-msg.c                | 66 +++++++++++++++++++++++++-
 2 files changed, 70 insertions(+), 2 deletions(-)

Comments

Junio C Hamano Feb. 9, 2020, 5:44 p.m. UTC | #1
"i.Dark_Templar" <darktemplar@dark-templar-archives.net> writes:

> If a lot of objects is merged at once, default commit message
> could become one very long line, which might be inconvenient to read.
> This change implements an option to change default autogenerated message
> so it'd take multiple lines, but each line wouldn't be long.
> 
> An artificial example.
>
> Original merge commit message:
>     Merge branch 'branch_with_some_long_name_1', remote-tracking branch 'clone/remote_branch_with_some_name', tags 'some_tag' and 'some_other_tag'; commit 'ae46a39cead2b42282abce725e90b561c06e94ba'; commit '33d0281e0eeb2a5e9907ebedc230e28c46865092' into merge7
>
> Multiline merge commit message:
>     Merge into merge8:
>     branch 'branch_with_some_long_name_1'
>     remote-tracking branch 'clone/remote_branch_with_some_name'
>     tag 'some_tag'
>     tag 'some_other_tag'
>     commit 'ae46a39cead2b42282abce725e90b561c06e94ba'
>     commit '33d0281e0eeb2a5e9907ebedc230e28c46865092'


How would these commits appear in the "git shorlog" output?  Losing
some information is OK (after all, you are making the 'title' of the
merge commit less crowded to prefer 'simpler' summary in exchange),
but you'd need to strike a good balance what to discard.  Is the
fact that the name of branch that got some unspecified new things
was 'merge8' the most important thing to convey?

If you make the commit 'title' a short one-line summary, you MUST
have a blank line after that line.  Otherwise, the 'where does the
title of this commit object end?' logic will helpfully merge all the
lines in the first paragraph (i.e. up to the first blank line) into
one long line, defeating your effort to make the summary simpler by
losing details.  That is:

    Merge 6 commits into merge8

    branch 'branch_with_some_long_name_1'
    remote-tracking ...
    ...

That's it for the 'mechanics', i.e. a discussion about how a good
design of this 'feature' should look like.

About the feature itself, I am not sure.  Even though I admit I was
the one who invented octupus merges, and it does make the history
look "pretty" in gitk when used judiciously, its practical benefit
over repeated pairwise merges is doubtful.  Besides, it makes
bisection less efficient.  So from that point of view, I am not sure
if we want a feature to encourage creation of more octopus merges.

I haven't read the code yet---I usually don't before figuring out if
the feature and its design makes sense---so I have no comment on the
actual change at this moment.  I may send a separate review message
later.

Thanks.
i.Dark_Templar Feb. 10, 2020, 6:51 p.m. UTC | #2
09.02.2020 20:44, Junio C Hamano пишет:
> "i.Dark_Templar" <darktemplar@dark-templar-archives.net> writes:
> 
>> If a lot of objects is merged at once, default commit message
>> could become one very long line, which might be inconvenient to read.
>> This change implements an option to change default autogenerated message
>> so it'd take multiple lines, but each line wouldn't be long.
>>
>> An artificial example.
>>
>> Original merge commit message:
>>     Merge branch 'branch_with_some_long_name_1', remote-tracking branch 'clone/remote_branch_with_some_name', tags 'some_tag' and 'some_other_tag'; commit 'ae46a39cead2b42282abce725e90b561c06e94ba'; commit '33d0281e0eeb2a5e9907ebedc230e28c46865092' into merge7
>>
>> Multiline merge commit message:
>>     Merge into merge8:
>>     branch 'branch_with_some_long_name_1'
>>     remote-tracking branch 'clone/remote_branch_with_some_name'
>>     tag 'some_tag'
>>     tag 'some_other_tag'
>>     commit 'ae46a39cead2b42282abce725e90b561c06e94ba'
>>     commit '33d0281e0eeb2a5e9907ebedc230e28c46865092'
> 
> 
> How would these commits appear in the "git shorlog" output?  Losing
> some information is OK (after all, you are making the 'title' of the
> merge commit less crowded to prefer 'simpler' summary in exchange),
> but you'd need to strike a good balance what to discard.  Is the
> fact that the name of branch that got some unspecified new things
> was 'merge8' the most important thing to convey?
> 
> If you make the commit 'title' a short one-line summary, you MUST
> have a blank line after that line.  Otherwise, the 'where does the
> title of this commit object end?' logic will helpfully merge all the
> lines in the first paragraph (i.e. up to the first blank line) into
> one long line, defeating your effort to make the summary simpler by
> losing details.  That is:
> 
>     Merge 6 commits into merge8
> 
>     branch 'branch_with_some_long_name_1'
>     remote-tracking ...
>     ...
> 
> That's it for the 'mechanics', i.e. a discussion about how a good
> design of this 'feature' should look like.
> 
> About the feature itself, I am not sure.  Even though I admit I was
> the one who invented octupus merges, and it does make the history
> look "pretty" in gitk when used judiciously, its practical benefit
> over repeated pairwise merges is doubtful.  Besides, it makes
> bisection less efficient.  So from that point of view, I am not sure
> if we want a feature to encourage creation of more octopus merges.
> 
> I haven't read the code yet---I usually don't before figuring out if
> the feature and its design makes sense---so I have no comment on the
> actual change at this moment.  I may send a separate review message
> later.
> 
> Thanks.
> 

Thank you for feedback.

I totally forgot about title in commit message and short log. I think
I'd like to take your suggestion and modify it a bit.
I want to add some text before enumeration of commits into message like
this:

Merge 6 commits into merge8

Following commits are merged:
branch 'branch_with_some_long_name_1'
remote-tracking ...
...

As for octopus merge, there are still some situations where it has some
advantages over series of pairwise merges, although I agree that it
might be uncommon cases. I can name linking multiple previously
unrelated histories as one such case. Bisect could be useless and
history would look better in my opinion.

If this feature would be fine with updated commit message, I'll update
my changes and send updated patch series.
diff mbox series

Patch

diff --git a/Documentation/config/fmt-merge-msg.txt b/Documentation/config/fmt-merge-msg.txt
index c73cfa90b7..8c12540fa7 100644
--- a/Documentation/config/fmt-merge-msg.txt
+++ b/Documentation/config/fmt-merge-msg.txt
@@ -8,3 +8,9 @@  merge.log::
 	most the specified number of one-line descriptions from the
 	actual commits that are being merged.  Defaults to false, and
 	true is a synonym for 20.
+
+merge.multilineMessage::
+	Make default merge commit message multiline. Every merged object
+	will be written using new line. This should ensure that
+	commit message wouldn't become one very long line when
+	there are a lot of merged objects.
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 05a92c59d8..93d44b59d9 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -20,6 +20,7 @@  static const char * const fmt_merge_msg_usage[] = {
 };
 
 static int use_branch_desc;
+static int use_multiline_default_message = 0;
 
 int fmt_merge_msg_config(const char *key, const char *value, void *cb)
 {
@@ -32,6 +33,8 @@  int fmt_merge_msg_config(const char *key, const char *value, void *cb)
 			merge_log_config = DEFAULT_MERGE_LOG_LEN;
 	} else if (!strcmp(key, "merge.branchdesc")) {
 		use_branch_desc = git_config_bool(key, value);
+	} else if (!strcmp(key, "merge.multilinemessage")) {
+		use_multiline_default_message = git_config_bool(key, value);
 	} else {
 		return git_default_config(key, value, cb);
 	}
@@ -467,6 +470,61 @@  static void fmt_merge_msg_title(struct strbuf *out,
 		strbuf_addf(out, " into %s\n", current_branch);
 }
 
+static void fmt_merge_msg_title_multiline(struct strbuf *out,
+				const char *current_branch)
+{
+	int i = 0;
+	int j = 0;
+
+	if (!strcmp("master", current_branch))
+		strbuf_addstr(out, "Merge:\n");
+	else
+		strbuf_addf(out, "Merge into %s:\n", current_branch);
+
+	for (i = 0; i < srcs.nr; i++) {
+		struct src_data *src_data = srcs.items[i].util;
+		int add_origin = 0;
+
+		if (src_data->head_status == 1) {
+			strbuf_addf(out, "%s\n", srcs.items[i].string);
+			continue;
+		}
+
+		add_origin = strcmp(".", srcs.items[i].string);
+
+		if (src_data->head_status == 3) {
+			if (!add_origin)
+				strbuf_addstr(out, "HEAD\n");
+			else
+				strbuf_addf(out, "HEAD of %s\n", srcs.items[i].string);
+		}
+		for (j = 0; j < src_data->branch.nr; j++) {
+			if (!add_origin)
+				strbuf_addf(out, "branch %s\n", src_data->branch.items[j].string);
+			else
+				strbuf_addf(out, "branch %s of %s\n", src_data->branch.items[j].string, srcs.items[i].string);
+		}
+		for (j = 0; j < src_data->r_branch.nr; j++) {
+			if (!add_origin)
+				strbuf_addf(out, "remote-tracking branch %s\n", src_data->r_branch.items[j].string);
+			else
+				strbuf_addf(out, "remote-tracking branch %s of %s\n", src_data->r_branch.items[j].string, srcs.items[i].string);
+		}
+		for (j = 0; j < src_data->tag.nr; j++) {
+			if (!add_origin)
+				strbuf_addf(out, "tag %s\n", src_data->tag.items[j].string);
+			else
+				strbuf_addf(out, "tag %s of %s\n", src_data->tag.items[j].string, srcs.items[i].string);
+		}
+		for (j = 0; j < src_data->generic.nr; j++) {
+			if (!add_origin)
+				strbuf_addf(out, "commit %s\n", src_data->generic.items[j].string);
+			else
+				strbuf_addf(out, "commit %s of %s\n", src_data->generic.items[j].string, srcs.items[i].string);
+		}
+	}
+}
+
 static void fmt_tag_signature(struct strbuf *tagbuf,
 			      struct strbuf *sig,
 			      const char *buf,
@@ -634,8 +692,12 @@  int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 			die("error in line %d: %.*s", i, len, p);
 	}
 
-	if (opts->add_title && srcs.nr)
-		fmt_merge_msg_title(out, current_branch);
+	if (opts->add_title && srcs.nr) {
+		if (!use_multiline_default_message)
+			fmt_merge_msg_title(out, current_branch);
+		else
+			fmt_merge_msg_title_multiline(out, current_branch);
+	}
 
 	if (origins.nr)
 		fmt_merge_msg_sigs(out);