[RFC] series headers

[RFC] series headers

[Daniel Barkalow]

I'd like to be able to get format-patch to produce a [PATCH 0/N] message, 
with a message that's actually in my repository, plus various goodies 
generated either from diffing the ends of the series or by running through 
the log an extra time to pick up summary information.

The second best idea I have for this currently is to have a commit at the 
end of the series which specifies which has the same tree as its parent 
and has a message with a line "Since <sha1 of the first commit of the 
series>" and has the text. This goes at the end of the series, because it 
describes the state with all of the changes made, which is only a good 
description of a commit at the end of the series, not a commit at the 
start of the series. Making it [PATCH 0/N] is just because it belongs 
first in presentation, regardless of whether the other commits are 
presented with recent commits first or last.

The better idea I just had was to have format-patch notice if the "until" 
side is a tag object instead of a commit, and generate a [0/N] with the 
tag message.

As far as implementing this... would it be sane to make struct 
rev_info.commit_format a callback, so that the code to generate an email 
message can be somewhere that's easy to use to generate an email that 
isn't for a commit in the log? I don't *think* git's quite fast enough for 
the indirect jump to a callback instead of an if tree for an enum will 
actually hurt us.

	-Daniel
*This .sig left intentionally blank*

Re: [RFC] series headers

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> I'd like to be able to get format-patch to produce a [PATCH 0/N] message, 
> with a message that's actually in my repository, plus various goodies 
> generated either from diffing the ends of the series or by running through 
> the log an extra time to pick up summary information.

So far, so good.

> As far as implementing this... would it be sane to make struct 
> rev_info.commit_format a callback, so that the code to generate an email 
> message can be somewhere that's easy to use to generate an email that 
> isn't for a commit in the log? I don't *think* git's quite fast enough for 
> the indirect jump to a callback instead of an if tree for an enum will 
> actually hurt us.

I suspect that temptation to touch rev_info.commit_format arises
purely because you are thinking about making 0/N a (perhaps
fake) commit.  I do not see a point in that.

What is the workflow?

 $ work work work, commit commit commit, reorder and perfect
 $ git tag end-of-series
 ... in $EDITOR, edit the [0/N] message
 $ git format-patch origin..end-of-series
 ... which notices end-of-series, and perhaps internally runs
 ... $ git-shortlog origin..end-of-series
 ... $ git diff --stat --summary origin..end-of-series
 ... $ git cat-file tag end-of-series
 ... to create 0/N which it did not do so far in
 ... 0000-cover-letter.txt
 $ git-send-email 0*.txt

Would it be so much better than this workflow which would
probably not need to touch much of the 'commit formatting" code
that is used for [1/N]..[N/N] messages?

 $ work work work, commit commit commit, reorder and perfect
 $ git format-patch --with-cover origin..HEAD
 ... which notices --with-cover, and perhaps does
 ... $ git-shortlog origin..HEAD
 ... $ git diff --stat --summary origin..HEAD
 ... $ echo "*** BLURB HERE ***"
 ... to create 0/N which it did not do so far in
 ... 0000-cover-letter.txt
 $ $EDITOR 0000-cover-letter.txt
 $ git-send-email 0*.txt

Re: [RFC] series headers

[Theodore Tso]

On Mon, Jul 09, 2007 at 11:57:21PM -0700, Junio C Hamano wrote:
> I suspect that temptation to touch rev_info.commit_format arises
> purely because you are thinking about making 0/N a (perhaps
> fake) commit.  I do not see a point in that.
> 
> What is the workflow?

In general a patch series goes through multiple cycles of
improvements, where people send it out for review/comment, and then it
gets fixed up, etc. etc.   So you don't want to just do this:

>  $ work work work, commit commit commit, reorder and perfect
>  $ git format-patch --with-cover origin..HEAD
>  ... which notices --with-cover, and perhaps does
>  ... $ git-shortlog origin..HEAD
>  ... $ git diff --stat --summary origin..HEAD
>  ... $ echo "*** BLURB HERE ***"
>  ... to create 0/N which it did not do so far in
>  ... 0000-cover-letter.txt
>  $ $EDITOR 0000-cover-letter.txt
>  $ git-send-email 0*.txt

Because you'll be sending out the 0000-cover-letter.txt multiple
times, refining it (and the patches) as you go along.

Some people will use quilt for this process; others will use guilt or
stg in a seprate branch.  The way I would probably solve it is by
making the Patch 0/N cover letter be a empty commit with no changes,
and simply storing the comment in the commit log.  Yes, that means you
have to go back and edit the 0/N message after you've finished doing
your patch series, but the general work flow will require you to go
back and clean up patch 3/27 given some comment that Cristoph Hellwig
made, so you'll be going back and forth using some tool like guilt or
stg anyway.  So going back to edit the 0/N cover-letter is not a big deal.

The one advantage of putting the 0/N cover-letter at the end is that
it makes it easier to drop it if the patch series ends up being pushed
via git, but again, given that the patch series is going to be
reworked multiple times during its life cycle, reworking it one last
time to drop the cover letter doesn't seem like a major issue --- and
if it is ends up getting applied by the maintainer via e-mail and
git-am, the cover letter will get dropped by the maintainer simply not
including it in his mailbox of patches to apply.

	  	     	    	       	       - Ted

[PATCH] Teach the --cover-letter option to format-patch

[Johannes Schindelin]

You can now generate a template for the cover letter by calling

	git format-patch --cover-letter <...>

It will fill in the shortlog and diffstat for you.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Mon, 9 Jul 2007, Junio C Hamano wrote:

	> Daniel Barkalow <barkalow@iabervon.org> writes:
	> 
	> > I'd like to be able to get format-patch to produce a 
	> > [PATCH 0/N] message, with a message that's actually in my 
	> > repository, plus various goodies generated either from diffing 
	> > the ends of the series or by running through the log an extra 
	> > time to pick up summary information.
	> 
	>  $ work work work, commit commit commit, reorder and perfect
	>  $ git format-patch --with-cover origin..HEAD
	>  ... which notices --with-cover, and perhaps does
	>  ... $ git-shortlog origin..HEAD
	>  ... $ git diff --stat --summary origin..HEAD
	>  ... $ echo "*** BLURB HERE ***"
	>  ... to create 0/N which it did not do so far in
	>  ... 0000-cover-letter.txt
	>  $ $EDITOR 0000-cover-letter.txt
	>  $ git-send-email 0*.txt

	Voila.

	BTW format-patch does not complain when there are skipped merges. 
	It surprised me, since you do not stand a chance to pipe that to 
	git-am successfully...

 Documentation/git-format-patch.txt                 |    6 +
 builtin-log.c                                      |  202 ++++++++++++++------
 log-tree.c                                         |    2 +-
 log-tree.h                                         |    1 +
 t/t4013-diff-various.sh                            |    1 +
 ...tch_--stdout_--cover-letter_-n_initial..master^ |  101 ++++++++++
 6 files changed, 254 insertions(+), 59 deletions(-)
 create mode 100644 t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 6cbcf93..aaae083 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -16,6 +16,7 @@ SYNOPSIS
                    [--in-reply-to=Message-Id] [--suffix=.<sfx>]
                    [--ignore-if-in-upstream]
                    [--subject-prefix=Subject-Prefix]
+		   [--cover-letter]
                    <since>[..<until>]
 
 DESCRIPTION
@@ -126,6 +127,11 @@ want a filename like `0001-description-of-my-change.patch`, and
 the first letter does not have to be a dot.  Leaving it empty would
 not add any suffix.
 
+--cover-letter::
+	Generate a cover letter template.  You still have to fill in
+	a description, but the shortlog and the diffstat will be
+	generated for you.
+
 CONFIGURATION
 -------------
 You can specify extra mail header lines to be added to each
diff --git a/builtin-log.c b/builtin-log.c
index 13bae31..e2b5e64 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -14,6 +14,7 @@
 #include "reflog-walk.h"
 #include "patch-ids.h"
 #include "refs.h"
+#include "run-command.h"
 
 static int default_show_root = 1;
 static const char *fmt_patch_subject_prefix = "PATCH";
@@ -303,74 +304,77 @@ static int git_format_config(const char *var, const char *value)
 }
 
 
+static const char *get_oneline_for_filename(struct commit *commit,
+		int keep_subject)
+{
+	static char filename[PATH_MAX];
+	char *sol;
+	int len = 0;
+
+	sol = strstr(commit->buffer, "\n\n");
+	if (!sol)
+		filename[0] = '\0';
+	else {
+		int j, space = 0;
+
+		sol += 2;
+		/* strip [PATCH] or [PATCH blabla] */
+		if (!keep_subject && !prefixcmp(sol, "[PATCH")) {
+			char *eos = strchr(sol + 6, ']');
+			if (eos) {
+				while (isspace(*eos))
+					eos++;
+				sol = eos;
+			}
+		}
+
+		for (j = 0; len < sizeof(filename) &&
+				sol[j] && sol[j] != '\n'; j++) {
+			if (istitlechar(sol[j])) {
+				if (space) {
+					filename[len++] = '-';
+					space = 0;
+				}
+				filename[len++] = sol[j];
+				if (sol[j] == '.')
+					while (sol[j + 1] == '.')
+						j++;
+			} else
+				space = 1;
+		}
+		while (filename[len - 1] == '.'
+		       || filename[len - 1] == '-')
+			len--;
+		filename[len] = '\0';
+	}
+	return filename;
+}
+
 static FILE *realstdout = NULL;
 static const char *output_directory = NULL;
 
-static int reopen_stdout(struct commit *commit, int nr, int keep_subject,
-			 int numbered_files)
+static int reopen_stdout(const char *oneline, int nr, int total)
 {
 	char filename[PATH_MAX];
-	char *sol;
-	int len = 0;
 	int suffix_len = strlen(fmt_patch_suffix) + 1;
+	int len = 0;
 
 	if (output_directory) {
-		if (strlen(output_directory) >=
+		len = snprintf(filename, sizeof(filename), "%s",
+				output_directory);
+		if (len >=
 		    sizeof(filename) - FORMAT_PATCH_NAME_MAX - suffix_len)
 			return error("name of output directory is too long");
-		strlcpy(filename, output_directory, sizeof(filename) - suffix_len);
-		len = strlen(filename);
 		if (filename[len - 1] != '/')
 			filename[len++] = '/';
 	}
 
-	if (numbered_files) {
-		sprintf(filename + len, "%d", nr);
-		len = strlen(filename);
-
-	} else {
-		sprintf(filename + len, "%04d", nr);
-		len = strlen(filename);
-
-		sol = strstr(commit->buffer, "\n\n");
-		if (sol) {
-			int j, space = 1;
-
-			sol += 2;
-			/* strip [PATCH] or [PATCH blabla] */
-			if (!keep_subject && !prefixcmp(sol, "[PATCH")) {
-				char *eos = strchr(sol + 6, ']');
-				if (eos) {
-					while (isspace(*eos))
-						eos++;
-					sol = eos;
-				}
-			}
-
-			for (j = 0;
-			     j < FORMAT_PATCH_NAME_MAX - suffix_len - 5 &&
-				     len < sizeof(filename) - suffix_len &&
-				     sol[j] && sol[j] != '\n';
-			     j++) {
-				if (istitlechar(sol[j])) {
-					if (space) {
-						filename[len++] = '-';
-						space = 0;
-					}
-					filename[len++] = sol[j];
-					if (sol[j] == '.')
-						while (sol[j + 1] == '.')
-							j++;
-				} else
-					space = 1;
-			}
-			while (filename[len - 1] == '.'
-			       || filename[len - 1] == '-')
-				len--;
-			filename[len] = 0;
-		}
-		if (len + suffix_len >= sizeof(filename))
-			return error("Patch pathname too long");
+	if (!filename)
+		len += sprintf(filename + len, "%d", nr);
+	else {
+		len += sprintf(filename + len, "%04d-", nr);
+		len += snprintf(filename + len, sizeof(filename) - len - 1
+				- suffix_len, "%s", oneline);
 		strcpy(filename + len, fmt_patch_suffix);
 	}
 
@@ -438,6 +442,70 @@ static void gen_message_id(char *dest, unsigned int length, char *base)
 		 (int)(email_end - email_start - 1), email_start + 1);
 }
 
+static void make_cover_letter(struct rev_info *rev,
+		int use_stdout, int numbered, int numbered_files,
+		struct commit *origin, struct commit *head)
+{
+	const char *committer, *sep;
+	const char *origin_sha1, *head_sha1;
+	const char *argv[7];
+
+	if (rev->commit_format != CMIT_FMT_EMAIL)
+		die("Cover letter needs email format");
+
+	if (!use_stdout && reopen_stdout(numbered_files ?
+				NULL : "cover-letter", 0, rev->total))
+		return;
+
+	origin_sha1 = sha1_to_hex(origin ? origin->object.sha1 : null_sha1);
+	head_sha1 = sha1_to_hex(head->object.sha1);
+
+	printf("From %s Mon Sep 17 00:00:00 2001\n", head_sha1);
+	if (rev->message_id)
+		printf("Message-Id: <%s>\n", rev->message_id);
+	if (rev->ref_message_id)
+		printf("In-Reply-To: <%s>\nReferences: <%s>\n",
+				rev->ref_message_id, rev->ref_message_id);
+	committer = git_committer_info(0);
+	sep = strchr(committer, '>');
+	if (sep)
+		sep++;
+	else
+		sep = committer + strlen(committer);
+	printf("From: %.*s\n", sep - committer, committer);
+	printf("Date:%s\n", sep);
+	if (rev->total)
+		printf("Subject: [%s %0*d/%d] *** SUBJECT HERE ***\n",
+				rev->subject_prefix,
+				digits_in_number(rev->total), 0, rev->total);
+	else
+		printf("Subject: *** SUBJECT HERE ***\n");
+	printf("\n\n");
+
+	printf("*** BLURB HERE ***\n\n");
+
+	argv[0] = "shortlog";
+	argv[1] = head_sha1;
+	argv[2] = "--not";
+	argv[3] = origin_sha1;
+	argv[4] = NULL;
+	fflush(stdout);
+	run_command_v_opt(argv, RUN_GIT_CMD);
+
+	argv[0] = "diff";
+	argv[1] = "--stat";
+	argv[2] = "--summary";
+	argv[3] = head_sha1;
+	argv[4] = "--not";
+	argv[5] = origin_sha1;
+	argv[6] = NULL;
+	fflush(stdout);
+	run_command_v_opt(argv, RUN_GIT_CMD);
+
+	fflush(stdout);
+	printf("\n");
+}
+
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
 {
 	struct commit *commit;
@@ -452,6 +520,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	int subject_prefix = 0;
 	int ignore_if_in_upstream = 0;
 	int thread = 0;
+	int cover_letter = 0;
+	struct commit *origin = NULL, *head = NULL;
 	const char *in_reply_to = NULL;
 	struct patch_ids ids;
 	char *add_signoff = NULL;
@@ -550,6 +620,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			rev.subject_prefix = argv[i] + 17;
 		} else if (!prefixcmp(argv[i], "--suffix="))
 			fmt_patch_suffix = argv[i] + 9;
+		else if (!strcmp(argv[i], "--cover-letter"))
+			cover_letter = 1;
 		else
 			argv[j++] = argv[i];
 	}
@@ -594,6 +666,17 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		 * get_revision() would return only the specified count.
 		 */
 	}
+	if (cover_letter) {
+		/* remember the range */
+		int i;
+		for (i = 0; i < rev.pending.nr; i++) {
+			struct object *o = rev.pending.objects[i].item;
+			if (o->flags & UNINTERESTING)
+				origin = (struct commit *)o;
+			else
+				head = (struct commit *)o;
+		}
+	}
 
 	if (ignore_if_in_upstream)
 		get_patch_ids(&rev, &ids, prefix);
@@ -618,6 +701,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	total = nr;
 	if (numbered)
 		rev.total = total + start_number - 1;
+	if (cover_letter)
+		make_cover_letter(&rev, use_stdout, numbered, numbered_files,
+				origin, head);
 	rev.add_signoff = add_signoff;
 	rev.ref_message_id = in_reply_to;
 	while (0 <= --nr) {
@@ -636,10 +722,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 				       sha1_to_hex(commit->object.sha1));
 			rev.message_id = message_id;
 		}
-		if (!use_stdout)
-			if (reopen_stdout(commit, rev.nr, keep_subject,
-					  numbered_files))
-				die("Failed to create output files");
+		if (!use_stdout && reopen_stdout(numbered_files ? NULL :
+				get_oneline_for_filename(commit, keep_subject),
+				rev.nr, rev.total))
+			die("Failed to create output files");
 		shown = log_tree_commit(&rev, commit);
 		free(commit->buffer);
 		commit->buffer = NULL;
diff --git a/log-tree.c b/log-tree.c
index 8624d5a..5c20c74 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -130,7 +130,7 @@ static unsigned long append_signoff(char **buf_p, unsigned long *buf_sz_p,
 	return at;
 }
 
-static unsigned int digits_in_number(unsigned int number)
+unsigned int digits_in_number(unsigned int number)
 {
 	unsigned int i = 10, result = 1;
 	while (i <= number) {
diff --git a/log-tree.h b/log-tree.h
index e82b56a..fbab51f 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -12,5 +12,6 @@ int log_tree_diff_flush(struct rev_info *);
 int log_tree_commit(struct rev_info *, struct commit *);
 int log_tree_opt_parse(struct rev_info *, const char **, int);
 void show_log(struct rev_info *opt, const char *sep);
+unsigned int digits_in_number(unsigned int number);
 
 #endif
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 9eec754..6b4d1c5 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -245,6 +245,7 @@ format-patch --inline --stdout initial..master
 format-patch --inline --stdout --subject-prefix=TESTCASE initial..master
 config format.subjectprefix DIFFERENT_PREFIX
 format-patch --inline --stdout initial..master^^
+format-patch --stdout --cover-letter -n initial..master^
 
 diff --abbrev initial..side
 diff -r initial..side
diff --git a/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
new file mode 100644
index 0000000..b66a568
--- /dev/null
+++ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
@@ -0,0 +1,101 @@
+$ git format-patch --stdout --cover-letter -n initial..master^
+From 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 Mon Sep 17 00:00:00 2001
+From: C O Mitter <committer@example.com>
+Date: 1151280300 +0000
+Subject: [DIFFERENT_PREFIX 0/2] *** SUBJECT HERE ***
+
+
+*** BLURB HERE ***
+
+A U Thor (2):
+      Second
+      Third
+
+ dir/sub |    4 ++++
+ file0   |    3 +++
+ file1   |    3 +++
+ file2   |    3 ---
+ 4 files changed, 10 insertions(+), 3 deletions(-)
+ create mode 100644 file1
+ delete mode 100644 file2
+
+From 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 Mon Sep 17 00:00:00 2001
+From: A U Thor <author@example.com>
+Date: Mon, 26 Jun 2006 00:01:00 +0000
+Subject: [DIFFERENT_PREFIX 1/2] Second
+
+This is the second commit.
+---
+ dir/sub |    2 ++
+ file0   |    3 +++
+ file2   |    3 ---
+ 3 files changed, 5 insertions(+), 3 deletions(-)
+ delete mode 100644 file2
+
+diff --git a/dir/sub b/dir/sub
+index 35d242b..8422d40 100644
+--- a/dir/sub
++++ b/dir/sub
+@@ -1,2 +1,4 @@
+ A
+ B
++C
++D
+diff --git a/file0 b/file0
+index 01e79c3..b414108 100644
+--- a/file0
++++ b/file0
+@@ -1,3 +1,6 @@
+ 1
+ 2
+ 3
++4
++5
++6
+diff --git a/file2 b/file2
+deleted file mode 100644
+index 01e79c3..0000000
+--- a/file2
++++ /dev/null
+@@ -1,3 +0,0 @@
+-1
+-2
+-3
+-- 
+g-i-t--v-e-r-s-i-o-n
+
+
+From 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 Mon Sep 17 00:00:00 2001
+From: A U Thor <author@example.com>
+Date: Mon, 26 Jun 2006 00:02:00 +0000
+Subject: [DIFFERENT_PREFIX 2/2] Third
+
+---
+ dir/sub |    2 ++
+ file1   |    3 +++
+ 2 files changed, 5 insertions(+), 0 deletions(-)
+ create mode 100644 file1
+
+diff --git a/dir/sub b/dir/sub
+index 8422d40..cead32e 100644
+--- a/dir/sub
++++ b/dir/sub
+@@ -2,3 +2,5 @@ A
+ B
+ C
+ D
++E
++F
+diff --git a/file1 b/file1
+new file mode 100644
+index 0000000..b1e6722
+--- /dev/null
++++ b/file1
+@@ -0,0 +1,3 @@
++A
++B
++C
+-- 
+g-i-t--v-e-r-s-i-o-n
+
+$
-- 
1.5.3.rc0.2782.g192a9-dirty

Re: [RFC] series headers

[Johannes Schindelin]

Hi,

On Tue, 10 Jul 2007, Theodore Tso wrote:

> In general a patch series goes through multiple cycles of
> improvements, where people send it out for review/comment, and then it
> gets fixed up, etc. etc.   So you don't want to just do this:
> 
> On Mon, Jul 09, 2007 at 11:57:21PM -0700, Junio C Hamano wrote:
> 
> >  $ work work work, commit commit commit, reorder and perfect
> >  $ git format-patch --with-cover origin..HEAD
> >  ... which notices --with-cover, and perhaps does
> >  ... $ git-shortlog origin..HEAD
> >  ... $ git diff --stat --summary origin..HEAD
> >  ... $ echo "*** BLURB HERE ***"
> >  ... to create 0/N which it did not do so far in
> >  ... 0000-cover-letter.txt
> >  $ $EDITOR 0000-cover-letter.txt
> >  $ git-send-email 0*.txt
> 
> Because you'll be sending out the 0000-cover-letter.txt multiple
> times, refining it (and the patches) as you go along.

And what is so wrong with

[insert before format-patch] $EDITOR my-cover-letter.txt

[replace $EDITOR 0000-cover-letter.txt] $EDITOR my-cover-letter.txt 
	0000-cover-letter.patch

and paste the changed text?

Of course, you can still hand roll something with a commit.  Probably a 
squash merge of your patch series.

Ciao,
Dscho

Re: [RFC] series headers

[Theodore Tso]

On Tue, Jul 10, 2007 at 02:56:48PM +0100, Johannes Schindelin wrote:
> And what is so wrong with
> 
> [insert before format-patch] $EDITOR my-cover-letter.txt
> 
> [replace $EDITOR 0000-cover-letter.txt] $EDITOR my-cover-letter.txt 
> 	0000-cover-letter.patch
> 
> and paste the changed text?

Nothing is *wrong* with it per-se, but if you have multiple things
that you are working on at the same-time, you might not want to keep
0000-cover-letter.patch in your working directory.  

The simplest place to put it is in .git/patches/<branch> while I'm
working with it using guilt (or the equivalent place in stgit), and if
you include it in the series file then it becomes easier to hack it so
that git-format-patches will create the cover letter for you.  (And
ideally, you would be able to use guilt to edit the cover letter using
"guilt header -e cover-letter" but that feature hasn't been added yet;
should be simple, though.)

Basically, the issue is of handling the cover letter as a first-class
object, as opposed to something whic his manually handled via
cut-and-paste.  A lot of the tools are actually there already if you
are using tools like guilt and stgit.  So it's basically making a
particular workflow a bit more optimized, that's all.  

There certainly is nothing *wrong* with manual maintenance of the
cover-letter file plus cut-and-paste when you want to update it.  Just
as there's nothing *wrong* with using git-write-tree instead of
git-commit.  Just a question of which is more convenient and less
error prone.  :-)

						- Ted

Re: [RFC] series headers

[Johannes Schindelin]

Hi,

On Tue, 10 Jul 2007, Theodore Tso wrote:

> On Tue, Jul 10, 2007 at 02:56:48PM +0100, Johannes Schindelin wrote:
> > And what is so wrong with
> > 
> > [insert before format-patch] $EDITOR my-cover-letter.txt
> > 
> > [replace $EDITOR 0000-cover-letter.txt] $EDITOR my-cover-letter.txt 
> > 	0000-cover-letter.patch
> > 
> > and paste the changed text?
> 
> Nothing is *wrong* with it per-se, but if you have multiple things that 
> you are working on at the same-time, you might not want to keep 
> 0000-cover-letter.patch in your working directory.

Ah, but then maybe you want to be able to say

	git format-patch --cover-letter=<my-cover-letter> [...]

Where your cover-letter file looks something like this:

	My patch series which will rule the world

	Here is the body, and I explain what the world should look like,
	and that they should accept me as the chosen one president of the
	united geekheads of this world.

IOW the file contents to produce what is produced with my patch would look 
like this:

	*** SUBJECT HERE ***

	*** BLURB HERE ***
Ciao,
Dscho

Re: [PATCH] Teach the --cover-letter option to format-patch

[Daniel Barkalow]

I think this is missing the ultra-important "extra_headers" stuff, which 
is what makes my messages actually reach the right people. That's why I'd 
like the code shared for generating headers (except for Subject) for a 
rev_info between the code that does it for patch messages and the code for 
the cover letter. I think it's also missing making [PATCH 1/N] a reply to 
it if the series is set up as replies.

I like the design, in any case. I want the blurb actually stored in the 
objects directory somehow, so that I don't have to trawl through my sent 
email for it when I send the series again for some reason, but that's a 
relatively straightforward extension to your code. (Read an object with a 
hash and stick the text in instead of the ***...*** parts.)

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH] Teach the --cover-letter option to format-patch

[Johannes Schindelin]

Hi,

On Tue, 10 Jul 2007, Daniel Barkalow wrote:

> I think this is missing the ultra-important "extra_headers" stuff, which 
> is what makes my messages actually reach the right people. That's why 
> I'd like the code shared for generating headers (except for Subject) for 
> a rev_info between the code that does it for patch messages and the code 
> for the cover letter. I think it's also missing making [PATCH 1/N] a 
> reply to it if the series is set up as replies.

Ah yes. Both issues should be relatively easy to integrate into my patch.

> I like the design, in any case. I want the blurb actually stored in the 
> objects directory somehow, so that I don't have to trawl through my sent 
> email for it when I send the series again for some reason, but that's a 
> relatively straightforward extension to your code. (Read an object with 
> a hash and stick the text in instead of the ***...*** parts.)

I do not understand. But then, my patch should be a good starting point. 
Go wild.

Ciao,
Dscho

Re: [PATCH] Teach the --cover-letter option to format-patch

[Peter Oberndorfer]

On Tuesday 10 July 2007 19:20, Johannes Schindelin wrote:
> Hi,
> 
> On Tue, 10 Jul 2007, Daniel Barkalow wrote:
> 
> > I think this is missing the ultra-important "extra_headers" stuff, which 
> > is what makes my messages actually reach the right people. That's why 
> > I'd like the code shared for generating headers (except for Subject) for 
> > a rev_info between the code that does it for patch messages and the code 
> > for the cover letter. I think it's also missing making [PATCH 1/N] a 
> > reply to it if the series is set up as replies.
> 
> Ah yes. Both issues should be relatively easy to integrate into my patch.
> 
> > I like the design, in any case. I want the blurb actually stored in the 
> > objects directory somehow, so that I don't have to trawl through my sent 
> > email for it when I send the series again for some reason, but that's a 
> > relatively straightforward extension to your code. (Read an object with 
> > a hash and stick the text in instead of the ***...*** parts.)
> 
> I do not understand. But then, my patch should be a good starting point. 
> Go wild.
> 
> Ciao,
> Dscho
Anybody working on this?
Somebody, *cough* convinced me into working on this.
I already split up the existing patch into logic parts, and will now try to read message / header
from file or a commit.

Greetings Peter

Re: [PATCH] Teach the --cover-letter option to format-patch

[Daniel Barkalow]

On Tue, 24 Jul 2007, Peter Oberndorfer wrote:

> On Tuesday 10 July 2007 19:20, Johannes Schindelin wrote:
> > Hi,
> > 
> > On Tue, 10 Jul 2007, Daniel Barkalow wrote:
> > 
> > > I think this is missing the ultra-important "extra_headers" stuff, which 
> > > is what makes my messages actually reach the right people. That's why 
> > > I'd like the code shared for generating headers (except for Subject) for 
> > > a rev_info between the code that does it for patch messages and the code 
> > > for the cover letter. I think it's also missing making [PATCH 1/N] a 
> > > reply to it if the series is set up as replies.
> > 
> > Ah yes. Both issues should be relatively easy to integrate into my patch.
> > 
> > > I like the design, in any case. I want the blurb actually stored in the 
> > > objects directory somehow, so that I don't have to trawl through my sent 
> > > email for it when I send the series again for some reason, but that's a 
> > > relatively straightforward extension to your code. (Read an object with 
> > > a hash and stick the text in instead of the ***...*** parts.)
> > 
> > I do not understand. But then, my patch should be a good starting point. 
> > Go wild.
> > 
> > Ciao,
> > Dscho
> Anybody working on this?
> Somebody, *cough* convinced me into working on this.
> I already split up the existing patch into logic parts, and will now try to read message / header
> from file or a commit.

Last night, I was thinking about it, but I hadn't actually gotten started. 
If you do it, I'd suggest using an unsigned tag for the message rather 
than a commit, since it fits the design for a message in an unsigned tag 
(extra information about the tagged version).

	-Daniel
*This .sig left intentionally blank*