[PATCH/RFC] Revert "git am/mailinfo: Don't look at in-body headers when rebasing"

[PATCH/RFC] Revert "git am/mailinfo: Don't look at in-body headers when rebasing"

[Paul Tan]

This reverts commit d25e51596be9271ad833805a3d6f9012dc24ee79, removing
git-mailsplit's --no-inbody-headers option.

While --no-inbody-headers was introduced to prevent commit messages from
being munged by git-mailinfo while rebasing, the need for this option
disappeared since 5e835ca (rebase: do not munge commit log message,
2008-04-16), as git-am bypasses git-mailinfo and gets the commit message
directly from the commit ID in the patch.

git-am is the only user of --no-inbody-headers, and this option is not
documented. As such, it should be removed.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---

Notes:
    The other direction, of course, is to turn --no-inbody-headers into a
    supported, documented option in both git-mailsplit and git-am.
    
    I do also wonder if we should just ensure that git-format-patch does not
    generate a message that start with "From" or "Date".

 builtin/mailinfo.c                   | 12 +-----------
 git-am.sh                            |  9 +--------
 t/t5100-mailinfo.sh                  |  4 ----
 t/t5100/info0015--no-inbody-headers  |  5 -----
 t/t5100/info0016--no-inbody-headers  |  5 -----
 t/t5100/msg0015--no-inbody-headers   |  3 ---
 t/t5100/msg0016--no-inbody-headers   |  4 ----
 t/t5100/patch0015--no-inbody-headers |  8 --------
 t/t5100/patch0016--no-inbody-headers |  8 --------
 9 files changed, 2 insertions(+), 56 deletions(-)
 delete mode 100644 t/t5100/info0015--no-inbody-headers
 delete mode 100644 t/t5100/info0016--no-inbody-headers
 delete mode 100644 t/t5100/msg0015--no-inbody-headers
 delete mode 100644 t/t5100/msg0016--no-inbody-headers
 delete mode 100644 t/t5100/patch0015--no-inbody-headers
 delete mode 100644 t/t5100/patch0016--no-inbody-headers

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 999a525..34ea160 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -26,7 +26,6 @@ static int patch_lines;
 static struct strbuf **p_hdr_data, **s_hdr_data;
 static int use_scissors;
 static int add_message_id;
-static int use_inbody_headers = 1;
 
 #define MAX_HDR_PARSED 10
 #define MAX_BOUNDARIES 5
@@ -795,17 +794,10 @@ static int handle_commit_msg(struct strbuf *line)
 	if (still_looking) {
 		if (!line->len || (line->len == 1 && line->buf[0] == '\n'))
 			return 0;
-	}
-
-	if (use_inbody_headers && still_looking) {
 		still_looking = check_header(line, s_hdr_data, 0);
 		if (still_looking)
 			return 0;
-	} else
-		/* Only trim the first (blank) line of the commit message
-		 * when ignoring in-body headers.
-		 */
-		still_looking = 0;
+	}
 
 	/* normalize the log message to UTF-8. */
 	if (metainfo_charset)
@@ -1062,8 +1054,6 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)
 			use_scissors = 1;
 		else if (!strcmp(argv[1], "--no-scissors"))
 			use_scissors = 0;
-		else if (!strcmp(argv[1], "--no-inbody-headers"))
-			use_inbody_headers = 0;
 		else
 			usage(mailinfo_usage);
 		argc--; argv++;
diff --git a/git-am.sh b/git-am.sh
index 761befb..df403b0 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -372,7 +372,7 @@ split_patches () {
 prec=4
 dotest="$GIT_DIR/rebase-apply"
 sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=
-messageid= resolvemsg= resume= scissors= no_inbody_headers=
+messageid= resolvemsg= resume= scissors=
 git_apply_opt=
 committer_date_is_author_date=
 ignore_date=
@@ -579,7 +579,6 @@ Use \"git am --abort\" to remove it.")"
 	echo "$keep" >"$dotest/keep"
 	echo "$messageid" >"$dotest/messageid"
 	echo "$scissors" >"$dotest/scissors"
-	echo "$no_inbody_headers" >"$dotest/no_inbody_headers"
 	echo "$GIT_QUIET" >"$dotest/quiet"
 	echo 1 >"$dotest/next"
 	if test -n "$rebasing"
@@ -644,12 +643,6 @@ t)
 f)
 	scissors=--no-scissors ;;
 esac
-if test "$(cat "$dotest/no_inbody_headers")" = t
-then
-	no_inbody_headers=--no-inbody-headers
-else
-	no_inbody_headers=
-fi
 if test "$(cat "$dotest/quiet")" = t
 then
 	GIT_QUIET=t
diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index e97cfb2..b2b5be6 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -31,10 +31,6 @@ do
 		then
 			check_mailinfo $mail --scissors
 		fi &&
-		if test -f "$TEST_DIRECTORY"/t5100/msg$mail--no-inbody-headers
-		then
-			check_mailinfo $mail --no-inbody-headers
-		fi &&
 		if test -f "$TEST_DIRECTORY"/t5100/msg$mail--message-id
 		then
 			check_mailinfo $mail --message-id
diff --git a/t/t5100/info0015--no-inbody-headers b/t/t5100/info0015--no-inbody-headers
deleted file mode 100644
index c4d8d77..0000000
--- a/t/t5100/info0015--no-inbody-headers
+++ /dev/null
@@ -1,5 +0,0 @@
-Author: A U Thor
-Email: a.u.thor@example.com
-Subject: check bogus body header (from)
-Date: Fri, 9 Jun 2006 00:44:16 -0700
-
diff --git a/t/t5100/info0016--no-inbody-headers b/t/t5100/info0016--no-inbody-headers
deleted file mode 100644
index f4857d4..0000000
--- a/t/t5100/info0016--no-inbody-headers
+++ /dev/null
@@ -1,5 +0,0 @@
-Author: A U Thor
-Email: a.u.thor@example.com
-Subject: check bogus body header (date)
-Date: Fri, 9 Jun 2006 00:44:16 -0700
-
diff --git a/t/t5100/msg0015--no-inbody-headers b/t/t5100/msg0015--no-inbody-headers
deleted file mode 100644
index be5115b..0000000
--- a/t/t5100/msg0015--no-inbody-headers
+++ /dev/null
@@ -1,3 +0,0 @@
-From: bogosity
-  - a list
-  - of stuff
diff --git a/t/t5100/msg0016--no-inbody-headers b/t/t5100/msg0016--no-inbody-headers
deleted file mode 100644
index 1063f51..0000000
--- a/t/t5100/msg0016--no-inbody-headers
+++ /dev/null
@@ -1,4 +0,0 @@
-Date: bogus
-
-and some content
-
diff --git a/t/t5100/patch0015--no-inbody-headers b/t/t5100/patch0015--no-inbody-headers
deleted file mode 100644
index ad64848..0000000
--- a/t/t5100/patch0015--no-inbody-headers
+++ /dev/null
@@ -1,8 +0,0 @@
----
-diff --git a/foo b/foo
-index e69de29..d95f3ad 100644
---- a/foo
-+++ b/foo
-@@ -0,0 +1 @@
-+content
-
diff --git a/t/t5100/patch0016--no-inbody-headers b/t/t5100/patch0016--no-inbody-headers
deleted file mode 100644
index ad64848..0000000
--- a/t/t5100/patch0016--no-inbody-headers
+++ /dev/null
@@ -1,8 +0,0 @@
----
-diff --git a/foo b/foo
-index e69de29..d95f3ad 100644
---- a/foo
-+++ b/foo
-@@ -0,0 +1 @@
-+content
-
-- 
2.1.4

Re: [PATCH/RFC] Revert "git am/mailinfo: Don't look at in-body headers when rebasing"

[Johannes Schindelin]

Hi Paul,

On 2015-06-16 11:03, Paul Tan wrote:
> This reverts commit d25e51596be9271ad833805a3d6f9012dc24ee79, removing
> git-mailsplit's --no-inbody-headers option.
> 
> While --no-inbody-headers was introduced to prevent commit messages from
> being munged by git-mailinfo while rebasing, the need for this option
> disappeared since 5e835ca (rebase: do not munge commit log message,
> 2008-04-16), as git-am bypasses git-mailinfo and gets the commit message
> directly from the commit ID in the patch.
> 
> git-am is the only user of --no-inbody-headers, and this option is not
> documented. As such, it should be removed.

Makes sense to me.

> Notes:
>     The other direction, of course, is to turn --no-inbody-headers into a
>     supported, documented option in both git-mailsplit and git-am.
>     
>     I do also wonder if we should just ensure that git-format-patch does not
>     generate a message that start with "From" or "Date".

In case we need this option for anything in the future, we can easily resurrect it from the commit history, so removing the option the appropriate course of action, methinks.

Ciao,
Dscho

Re: [PATCH/RFC] Revert "git am/mailinfo: Don't look at in-body headers when rebasing"

[Paul Tan]

Hi Junio,

On Tue, Jun 16, 2015 at 5:03 PM, Paul Tan <pyokagan@gmail.com> wrote:
> This reverts commit d25e51596be9271ad833805a3d6f9012dc24ee79, removing
> git-mailsplit's --no-inbody-headers option.
>
> While --no-inbody-headers was introduced to prevent commit messages from
> being munged by git-mailinfo while rebasing, the need for this option
> disappeared since 5e835ca (rebase: do not munge commit log message,
> 2008-04-16), as git-am bypasses git-mailinfo and gets the commit message
> directly from the commit ID in the patch.
>
> git-am is the only user of --no-inbody-headers, and this option is not
> documented. As such, it should be removed.
>
> Signed-off-by: Paul Tan <pyokagan@gmail.com>

What do you think about applying this patch?

Either way, the no-inbody-headers code in git-am.sh is dead code so I
don't think I will be implementing it in the git-am rewrite.

Thanks,
Paul

Re: [PATCH/RFC] Revert "git am/mailinfo: Don't look at in-body headers when rebasing"

[Junio C Hamano]

Paul Tan <pyokagan@gmail.com> writes:

> On Tue, Jun 16, 2015 at 5:03 PM, Paul Tan <pyokagan@gmail.com> wrote:
>> This reverts commit d25e51596be9271ad833805a3d6f9012dc24ee79, removing
>> git-mailsplit's --no-inbody-headers option.
>>
>> While --no-inbody-headers was introduced to prevent commit messages from
>> being munged by git-mailinfo while rebasing, the need for this option
>> disappeared since 5e835ca (rebase: do not munge commit log message,
>> 2008-04-16), as git-am bypasses git-mailinfo and gets the commit message
>> directly from the commit ID in the patch.
>>
>> git-am is the only user of --no-inbody-headers, and this option is not
>> documented. As such, it should be removed.
>>
>> Signed-off-by: Paul Tan <pyokagan@gmail.com>
>
> What do you think about applying this patch?

Indifferent, even though it would be the right thing to do in the
longer term.

Keeping what we think nobody uses does not hurt us at least in the
short term; removing such a thing hurts us if it turns out that
somebody whose use we didn't know about were actually using it.

And my time is better spent at this point in the cycle on other
things than having to worry about possible fallouts from the removal
that is not urgent.