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

2015-06-24 Thread Paul Tan
Hi Junio,

On Tue, Jun 16, 2015 at 5:03 PM, Paul Tan pyoka...@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 pyoka...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-24 Thread Junio C Hamano
Paul Tan pyoka...@gmail.com writes:

 On Tue, Jun 16, 2015 at 5:03 PM, Paul Tan pyoka...@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 pyoka...@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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-16 Thread 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 pyoka...@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..000
--- a/t/t5100/info0015--no-inbody-headers

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

2015-06-16 Thread 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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html