format-patch and send-email
cover format-patch and send-email
A couple of patches that have previously been sent to the list
Joe Perches (2):
git-send-email.perl: Don't add header "In-Reply-To:" when --no-chain-reply-to set
format-patch: Add --cover-letter-wrap
Documentation/git-format-patch.txt | 13 +++++++++ builtin-log.c | 46 +++++++++++++++++++++++++++++--- contrib/completion/git-completion.bash | 1 + git-send-email.perl | 3 +- 4 files changed, 58 insertions(+), 5 deletions(-)
PATCH 1/2 git-send-email.perl: Don't add header "In-Reply-To:" when --no-chain-reply-to set
using
git format-patch --thread=shallow -o <foo>
and
git send-email --no-thread --no-chain-reply-to <foo>
duplicates the headers
In-Reply-To:
References:
--- git-send-email.perl | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
git-send-email.perl +2 -1
@@ -1151,7 +1151,8 @@ foreach my $t (@files) { # set up for the next message if ($thread && $message_was_sent && - ($chain_reply_to || !defined $reply_to || length($reply_to) == 0)) { + ($chain_reply_to && + (!defined $reply_to || length($reply_to) == 0))) { $reply_to = $message_id; if (length $references > 0) { $references .= "\n $message_id";
Signature · 1 line
1.6.3.1.10.g659a0.dirty
Re: [PATCH 1/2] git-send-email.perl: Don't add header "In-Reply-To:" when --no-chain-reply-to set
Joe Perches, 01.07.2009:
Quoted 4 lines
using
git format-patch --thread=shallow -o <foo>
and
git send-email --no-thread --no-chain-reply-to <foo>
I guess you meant --thread here.
Quoted 4 lines
duplicates the headers
In-Reply-To:
References:
I noticed the duplicated headers when fixing two bugs some weeks ago. I guess to get rid of the duplicated headers, you have to parse the mail that format-patch produces, since you don't have a clue, what format-patch did. I'm not sure if it's worth it, if the duplicated headers don't harm the email standard. It only complicates the tool.
Quoted 10 lines
Signed-off-by Joe Perches <[email protected]>--- git-send-email.perl | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)git-send-email.perl +0
-0
git-send-email.perl foreach my $t (@files) { · line 1158
Quoted 9 lines
git-send-email.perl +2
-1@@ -1151,7 +1151,8 @@ foreach my $t (@files) { # set up for the next message if ($thread && $message_was_sent && - ($chain_reply_to || !defined $reply_to || length($reply_to) == 0)) { + ($chain_reply_to && + (!defined $reply_to || length($reply_to) == 0))) { $reply_to = $message_id; if (length $references > 0) { $references .= "\n $message_id";
This part of git-send-email seems to be prone for errors, blame it and you will see in the latest commits.
You should run at least the test from the test suite for the particular command you change. Test 49 (threading but no chain-reply-to) from t9001 now fails.
That means, this will fail now (covered by test 49):
git format-patch -o <foo> git send-email --thread --no-chain-reply-to <foo>
and also this (not covered by any test, maybe we should add one):
git format-patch -o <foo> git send-email --thread --chain-reply-to <foo>
Re: [PATCH 1/2] git-send-email.perl: Don't add header "In-Reply-To:" when --no-chain-reply-to set
Quoted 7 lines · Markus Heidelberg
Joe Perches, 01.07.2009:
Quoted 4 lines
using
git format-patch --thread=shallow -o <foo>
and
git send-email --no-thread --no-chain-reply-to <foo>I guess you meant --thread here.
Actually, I did mean --no-thread.
If format-patch does the threading, send-email shouldn't.
Quoted 11 lines
Quoted 4 lines
duplicates the headers
In-Reply-To:
References:You should run at least the test from the test suite for the particular command you change. Test 49 (threading but no chain-reply-to) from t9001 now fails.
That means, this will fail now (covered by test 49):
Quoted 7 lines
git format-patch -o <foo> git send-email --thread --no-chain-reply-to <foo>and also this (not covered by any test, maybe we should add one):
git format-patch -o <foo> git send-email --thread --chain-reply-to <foo>
I didn't know the tests existed, thanks.
I'll investigate a bit more.
cheers, Joe
Re: [PATCH 1/2] git-send-email.perl: Don't add header "In-Reply-To:" when --no-chain-reply-to set
Joe Perches, 01.07.2009:
Quoted 10 lines · Markus Heidelberg
Quoted 7 lines
Joe Perches, 01.07.2009:
Quoted 4 lines
using
git format-patch --thread=shallow -o <foo>
and
git send-email --no-thread --no-chain-reply-to <foo>I guess you meant --thread here.
Actually, I did mean --no-thread.
But --no-thread doesn't duplicate, --thread does.
According to the last line of your patch, you are using an old version 1.6.3.1. After this version two threading bugs were fixed, so you may encounter a wrong behaviour with your old version.
Quoted 1 line
If format-patch does the threading, send-email shouldn't.
If the user wants format-patch to do the threading, he shouldn't want send-email to do it as well.
Markus
Re: [PATCH 1/2] git-send-email.perl: Don't add header "In-Reply-To:" when --no-chain-reply-to set
Quoted 7 lines · Markus Heidelberg
Joe Perches, 01.07.2009:
Quoted 6 lines · Markus Heidelberg
Quoted 5 lines
Joe Perches, 01.07.2009:
Quoted 4 lines
using
git format-patch --thread=shallow -o <foo>
and
git send-email --no-thread --no-chain-reply-to <foo>
Quoted 7 lines
Quoted 2 lines
Quoted 1 line
I guess you meant --thread here.
Actually, I did mean --no-thread.
But --no-thread doesn't duplicate, --thread does.
According to the last line of your patch, you are using an old version 1.6.3.1. After this version two threading bugs were fixed, so you may encounter a wrong behaviour with your old version.
Great, I'm glad the bug was fixed already and apologies for the noise, I don't generally track the git list.
Reading the archives, I see you're responsible, thanks.
cheers, Joe
PATCH 2/2 format-patch: Add --cover-letter-wrap
In reply to Joe Perches--cover-letter does not give control over the column wrap position. This adds --cover-letter-wrap with 3 arguments position as well as indent and additional_indent.
--- Documentation/git-format-patch.txt | 13 +++++++++ builtin-log.c | 46 +++++++++++++++++++++++++++++--- contrib/completion/git-completion.bash | 1 + 3 files changed, 56 insertions(+), 4 deletions(-)
Documentation/git-format-patch.txt +13 -0
@@ -20,6 +20,8 @@ SYNOPSIS [--subject-prefix=Subject-Prefix] [--cc=<email>] [--cover-letter] + [--cover-letter-wrap=width[,indent1[,indent2]]] + [--no-cover-letter-wrap] [<common diff options>] [ <since> | <revision range> ]
@@ -168,6 +170,17 @@ if that is not set. containing the shortlog and the overall diffstat. You can fill in a description in the file before sending it out. +--cover-letter-wrap=<width>[,<indent1>[,<indent2>]]]:: + Linewrap the cover-letter shortlog output by wrapping each line at + `width`. The first line of each entry is indented by `indent1` + spaces, and the second and subsequent lines are indented by + `indent2` spaces. + `width`, `indent1`, and `indent2` default to 72, 2 and 4 respectively. + +--no-cover-letter-wrap:: + Do not linewrap the cover-letter shortlog output. + indent is fixed at 6. + --suffix=.<sfx>:: Instead of using `.patch` as the suffix for generated filenames, use specified suffix. A common alternative is
builtin-log.c +42 -4
70 lines not on this page · raw message
contrib/completion/git-completion.bash +1 -0
@@ -985,6 +985,7 @@ _git_format_patch () --full-index --binary --not --all --cover-letter + --no-cover-letter-wrap --cover-letter-wrap= --no-prefix --src-prefix= --dst-prefix= --inline --suffix= --ignore-if-in-upstream --subject-prefix=
Signature · 1 line
1.6.3.1.10.g659a0.dirty
Re: [PATCH 2/2] format-patch: Add --cover-letter-wrap
Quoted 12 lines · Joe Perches
builtin-log.c +8
-0@@ -792,6 +806,27 @@ static int output_directory_callback(const struct option *opt, const char *arg,return 0 ; } +static int cls_callback (const struct option *opt,const char *arg,int unset) +{ +if (unset) + cls.cover_letter_wrap =0 ; +else { +int i1, i2, i3; +if (!arg) +return 1 ;
builtin-log.c static int output_directory_callback(const struct option *opt, const char *arg, · line 826
Quoted 10 lines
builtin-log.c +10
-0+int arg_count =sscanf (arg,"%d,%d,%d" , &i1, &i2, &i3); +if (arg_count <=0 ) +return 1 ; +if (arg_count >=1 ) + cls.cover_letter_wrappos = i1; +if (arg_count >=2 ) + cls.cover_letter_indent1 = i2; +if (arg_count >=3 ) + cls.cover_letter_indent2 = i3; + }
This bracket is one indent off.
I'm not sure, but can this be simplified to just setting the struct members directly through sscanf? You won't need to have these if's in that case. I think something like --cover-letter-wrap="" would be equivalent to just using the defaults and not an error. Does that sound right?
builtin-log.c int cmd_format_patch(int argc, const char **argv, const char *prefix) · line 915
Quoted 13 lines
builtin-log.c +6
-0+return 0 ; +} +static int thread_callback (const struct option *opt,const char *arg,int unset) {int *thread = (int *)opt->value;@@ -875,6 +910,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) "print patches to standard out"),OPT_BOOLEAN (0 ,"cover-letter" , &cover_letter,"generate a cover letter" ), + { OPTION_CALLBACK, 0, "cover-letter-wrap", &cls, NULL, +"control the cover letter format" , + PARSE_OPT_OPTARG, cls_callback },
Why is this PARSE_OPT_OPTARG? I only see the choice of having arguments or prefixed with a --no. Also, please use PARSE_OPT_LITERAL_ARGHELP and give it the help string you use in the docs (<width>[,<indent1>[,<indent2>]]).