threads / patch / 22682

patchAdd 'git format-patch --to=' option and 'format.to' configuration variable.

Subject: [PATCH] Add 'git format-patch --to=' option and 'format.to' configuration variable.

## tl;dr

7 messages between Feb 16, 2010 and Feb 18, 2010. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Steven Drake· Feb 16, 2010, 23:39 UTC · lore

Has the same functionality as the '--cc' option and 'format.cc' configuration variable but for the "To:" email header. Half of the code to support this was already there.

With email the To: header usually more important than the Cc: header.
Signed-off-by: Steven Drake <sdrake@xnet.co.nz>
---
 Documentation/git-format-patch.txt |   11 ++++++++---
 builtin-log.c                      |   16 ++++++++++++++++
 2 files changed, 24 insertions(+), 3 deletions(-)
Show changes to 2 files +24 −3

Documentation/git-format-patch.txt, builtin-log.c

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index f1fd0df..e936d71 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -18,7 +18,7 @@ SYNOPSIS
 		   [--in-reply-to=Message-Id] [--suffix=.<sfx>]
 		   [--ignore-if-in-upstream]
 		   [--subject-prefix=Subject-Prefix]
-		   [--cc=<email>]
+		   [--to=<email>] [--cc=<email>]
 		   [--cover-letter]
 		   [<common diff options>]
 		   [ <since> | <revision range> ]
@@ -162,6 +162,10 @@ will want to ensure that threading is disabled for `git send-email`.
 	allows for useful naming of a patch series, and can be
 	combined with the `--numbered` option.
 
+--to=<email>::
+	Add a `To:` header to the email headers. This is in addition
+	to any configured headers, and may be used multiple times.
+
 --cc=<email>::
 	Add a `Cc:` header to the email headers. This is in addition
 	to any configured headers, and may be used multiple times.
@@ -202,8 +206,8 @@ CONFIGURATION
 -------------
 You can specify extra mail header lines to be added to each message,
 defaults for the subject prefix and file suffix, number patches when
-outputting more than one patch, add "Cc:" headers, configure attachments,
-and sign off patches with configuration variables.
+outputting more than one patch, add "To" or "Cc:" headers, configure
+attachments, and sign off patches with configuration variables.
 
 ------------
 [format]
@@ -211,6 +215,7 @@ and sign off patches with configuration variables.
 	subjectprefix = CHANGE
 	suffix = .txt
 	numbered = auto
+	to = <email>
 	cc = <email>
 	attach [ = mime-boundary-string ]
 	signoff = true
diff --git a/builtin-log.c b/builtin-log.c
index 41b6df4..89f8d60 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -501,6 +501,13 @@ static int git_format_config(const char *var, const char *value, void *cb)
 	}
 	if (!strcmp(var, "format.suffix"))
 		return git_config_string(&fmt_patch_suffix, var, value);
+	if (!strcmp(var, "format.to")) {
+		if (!value)
+			return config_error_nonbool(var);
+		ALLOC_GROW(extra_to, extra_to_nr + 1, extra_to_alloc);
+		extra_to[extra_to_nr++] = xstrdup(value);
+		return 0;
+	}
 	if (!strcmp(var, "format.cc")) {
 		if (!value)
 			return config_error_nonbool(var);
@@ -872,6 +879,13 @@ static int header_callback(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+static int to_callback(const struct option *opt, const char *arg, int unset)
+{
+	ALLOC_GROW(extra_to, extra_to_nr + 1, extra_to_alloc);
+	extra_to[extra_to_nr++] = xstrdup(arg);
+	return 0;
+}
+
 static int cc_callback(const struct option *opt, const char *arg, int unset)
 {
 	ALLOC_GROW(extra_cc, extra_cc_nr + 1, extra_cc_alloc);
@@ -936,6 +950,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		{ OPTION_CALLBACK, 0, "add-header", NULL, "header",
 			    "add email header", PARSE_OPT_NONEG,
 			    header_callback },
+		{ OPTION_CALLBACK, 0, "to", NULL, "email", "add To: header",
+			    PARSE_OPT_NONEG, to_callback },
 		{ OPTION_CALLBACK, 0, "cc", NULL, "email", "add Cc: header",
 			    PARSE_OPT_NONEG, cc_callback },
 		OPT_STRING(0, "in-reply-to", &in_reply_to, "message-id",
-- 
1.6.6
Junio C Hamano· Feb 17, 2010, 01:13 UTC · re: Steven Drake · lore

Re: [PATCH] Add 'git format-patch --to=' option and 'format.to' configuration variable.

Steven Drake <sdrake@xnet.co.nz> writes:
> Has the same functionality as the '--cc' option and 'format.cc'
> configuration variable but for the "To:" email header.  Half of the code to
> support this was already there.

I don't _mind_ adding such a config (I didn't check the patch text, though), but one thing I don't understand is what kind of workflow do people who use format.cc, format.to, --cc and --to are using.

My impression has always been that you drive your _MUA_ to set these values, and MUAs are either (1) not so cooperating to read these headers from format-patch output and use them to decide where to send mails from, or (2) is a git-send-email which is customizable already per repository so you do not have to do anything funky when running format-patch.

How are you sending your output from format-patch, and how does having To: and Cc: header pregenerated by format-patch help you?

Steven Drake· Feb 17, 2010, 03:11 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add 'git format-patch --to=' option and 'format.to' configuration variable.

On Tue, 16 Feb 2010, Junio C Hamano wrote:
> I don't _mind_ adding such a config (I didn't check the patch text,
> though), but one thing I don't understand is what kind of workflow do
> people who use format.cc, format.to, --cc and --to are using.
In the commit that add '--cc', Daniel Barkalow wrote:
> When you have particular reviewers you want to sent particular series
> to, it's nice to be able to generate the whole series with them as
> additional recipients, without configuring them into your general
> headers or adding them by hand afterwards.
In the commit that added 'format.cc' Miklos Vajna wrote:
> Some projects prefer to always CC patches to a given mailing list. In
> these cases, it's handy to configure that address once.
I think it's weired to have an option & config variable for Cc and not To.
> My impression has always been that you drive your _MUA_ to set these
> values, and MUAs are either (1) not so cooperating to read these headers
> from format-patch output and use them to decide where to send mails from,

Any MUA that does not cooperate with reading the To header is going to give trouble with any header (e.g. Subject, Date).

> or (2) is a git-send-email which is customizable already per repository so
> you do not have to do anything funky when running format-patch.
Haven't got around to using git-send-email yet!
> [...] how does having To:
> and Cc: header pregenerated by format-patch help you?
Mostly having to remember/lookup the email address.
-- 
Steven
Junio C Hamano· Feb 17, 2010, 03:47 UTC · re: Steven Drake · lore

Re: [PATCH] Add 'git format-patch --to=' option and 'format.to' configuration variable.

Steven Drake <sdrake@xnet.co.nz> writes:
> I think it's weired to have an option & config variable for Cc and not To.

This line in your message I agree with 100%, and I already said I do _not_ mind adding format.to or --to. But a sad thing is, this one line is the only one I agree with in your message.

All the other lines including what your quoted in your message do not answer my question at all. Neither description by Daniel nor by Miklos explains how giving format.cc or --cc given to format-patch helps the user in the bigger picture. Is a user of this feature expected to always use send-email? If so, how is that different from giving these options to send-email instead?

> Any MUA that does not cooperate with reading the To header is going to
> give trouble with any header (e.g. Subject, Date).

Yes, --cc/--to/format.cc/format.are not about helping users of these uncooperating MUAs, and that is perfectly fine. "It is bad that the feature does not to help them" is not what I am saying. I am only trying to find what these features are designed to help. If the set of MUAs that are helped by this feature is larger than "git send-email" by an iota, I'd be happy.

>> or (2) is a git-send-email which is customizable already per repository so
>> you do not have to do anything funky when running format-patch.
> Haven't got around to using git-send-email yet!

Then how would having To: and Cc: help in the format-patch output help you at all? I tried to ask you (apparently in a garbled grammar, sorry) what your workflow of sending the format-patch output to the outside world is.

The reason why I am asking is _not_ because I want to reject this patch. I want to be able to explain to other people why it is beneficial to be able to specify to/cc setting to format-patch, and using it in what way in a larger picture it would help the user.

I.e. saying "by setting format.to, your output will have these To: header" is not good enough, if we do not make it clear why having the "To/Cc" in the output helps users and in what way.

I want to hear "because the output from format-patch has these To: header, your life gets easier *this way*, if the MUA you use to send out patches is ________. You can use command ______ of the MUA to read the whole thing including the headers, and you do not have to type nor remember the addresses; this is something you cannot do without using this feature if your favorite MUA is _______."

If that MUA is "git send-email", then I would explain to my users "don't worry about these format-patch 'features'; if you are a user of "git send-email", then give them to that command instead." Feature duplication isn't a bad thing per-se, but I want to know about it.

Steven Drake· Feb 17, 2010, 07:33 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add 'git format-patch --to=' option and 'format.to' configuration variable.

On Tue, 16 Feb 2010, Junio C Hamano wrote:
> I tried to ask you (apparently in a garbled grammar, sorry) what
> your workflow of sending the format-patch output to the outside world is.
 
$ ./git-format-patch --stdout master~5..master~1 >>~/mail/postponed-msgs
$ alpine -I cp
Only problem with doing that is that alpine changes the Date: and
 Message-Id: header (the later being a pain if I used --threaded).

I don't know about other MUA (or how Daniel or Miklos work) but someone could use a command like:

$ git-format-patch --stdout master~5..master~1 | sendmail -bm -t

But in all honesty I don't think it matters whether the headers are set by format-patch or send-email and I would understand if you wanted to deprecate '--cc' from format-patch and 'format.cc' (maybe keep it for backwards compat).

-- 
Steven
Junio C Hamano· Feb 17, 2010, 08:14 UTC · re: Steven Drake · lore

Re: [PATCH] Add 'git format-patch --to=' option and 'format.to' configuration variable.

Steven Drake <sdrake@xnet.co.nz> writes:
Show 9 quoted lines
> On Tue, 16 Feb 2010, Junio C Hamano wrote:
>> I tried to ask you (apparently in a garbled grammar, sorry) what
>> your workflow of sending the format-patch output to the outside world is.
>  
> $ ./git-format-patch --stdout master~5..master~1 >>~/mail/postponed-msgs
> $ alpine -I cp
>
> Only problem with doing that is that alpine changes the Date: and
>  Message-Id: header (the later being a pain if I used --threaded).

Finally. That is the kind of "how somebody would work with format-patch output" I was looking for. Thanks.

> ... I would understand if you wanted to deprecate '--cc' from
> format-patch and 'format.cc' (maybe keep it for backwards compat).
No, you took me wrong.  Deprecation wasn't what I was after.

I wanted to see ways to _positively_ explain what format.{cc,to} are good for. With them, we can eventually have a few examples in EXAMPLES section of the manual page to help users (hint, hint...).

Peter Krefting· Feb 18, 2010, 10:15 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add 'git format-patch --to=' option and 'format.to' configuration variable.

Junio C Hamano:
> How are you sending your output from format-patch, and how does having To: 
> and Cc: header pregenerated by format-patch help you?
The patches I have sent to the list have all been sent using
   git format-patch something
   $EDITOR *.patch # remove mbox header, and add To:
   for file in *.patch ; do sendmail -t < $file ; done

as every attempt I have done to send them through Alpine or other MUAs have failed horribly.

I haven't minded having to edit the patch files manually (since I have also visually inspected the patch files in the process), but being able to add the To: line for the list automatically for each of the patches lessens the risk of misspelling or missing it completely when fixing the patches up.

-- 
\\// Peter - http://www.softwolves.pp.se/

← back to recent threads