{"thread":{"id":"31857","subject":"[PATCH 0/6] Bring \"format-patch --notes\" closer to a real feature","startedAt":"2012-10-18T05:45:22Z","lastAt":"2012-10-21T21:33:54Z","messageCount":19,"participants":["Junio C Hamano","Jeff King","Nguyen Thai Ngoc Duy","Michael J Gruber","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"201468","messageId":"1350539128-21577-1-git-send-email-gitster@pobox.com","threadId":"31857","inReplyTo":null,"subject":"[PATCH 0/6] Bring \"format-patch --notes\" closer to a real feature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-18T05:45:22Z","receivedAt":"2012-10-18T05:45:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This replaces the earlier \"wip\" with a real thing.\n\nWe never advertised the \"--notes\" option to format-patch (or\nanything related to the pretty format options for that matter)\nbecause the behaviour of these options was whatever they happened to\ndo, not what they were designed to do.\n\nIt had a few obvious glitches:\n\n * The notes section was appended immediately after the log message,\n   and then the three-dash line was added.  Such a supplimental\n   material should come after the three-dash line.\n\n * The logic to append a new sign-off with \"format-patch --signoff\"\n   worked on the message after the note was added, which made the\n   detection of existing sign-off lines incorrect.\n\nThis updates the handling of \"--notes\" option to correct these, in\nan attempt to bring it closer to a real feature.\n\nJunio C Hamano (6):\n  pretty: remove reencode_commit_message()\n  format_note(): simplify API\n  pretty: prepare notes message at a centralized place\n  pretty_print_commit(): do not append notes message\n  format-patch: append --signature after notes\n  format-patch --notes: show notes after three-dashes\n\n builtin/blame.c         |  5 +++--\n commit.h                |  4 +---\n log-tree.c              | 32 ++++++++++++++++++++++++++++----\n notes.c                 | 13 +++++++------\n notes.h                 |  6 +-----\n pretty.c                | 22 ++++------------------\n revision.c              |  2 +-\n revision.h              |  1 +\n t/t4014-format-patch.sh | 15 +++++++++++++--\n 9 files changed, 59 insertions(+), 41 deletions(-)\n\n-- \n1.8.0.rc3.112.gdb88a5e\n"},{"id":"201469","messageId":"1350539128-21577-2-git-send-email-gitster@pobox.com","threadId":"31857","inReplyTo":"1350539128-21577-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/6] pretty: remove reencode_commit_message()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-18T05:45:23Z","receivedAt":"2012-10-18T05:45:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This function has only two callsites, and is a thin wrapper whose\nusefulness is dubious.  When the caller needs to learn the log\noutput encoding, it should be able to do so by directly calling\nget_log_output_encoding() and calling the underlying\nlogmsg_reencode() with it.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/blame.c |  5 +++--\n commit.h        |  2 --\n pretty.c        | 13 ++-----------\n 3 files changed, 5 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex c27ef21..cfae569 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1425,7 +1425,7 @@ static void get_commit_info(struct commit *commit,\n \t\t\t    int detailed)\n {\n \tint len;\n-\tconst char *subject;\n+\tconst char *subject, *encoding;\n \tchar *reencoded, *message;\n \tstatic char author_name[1024];\n \tstatic char author_mail[1024];\n@@ -1446,7 +1446,8 @@ static void get_commit_info(struct commit *commit,\n \t\t\tdie(\"Cannot read commit %s\",\n \t\t\t    sha1_to_hex(commit->object.sha1));\n \t}\n-\treencoded = reencode_commit_message(commit, NULL);\n+\tencoding = get_log_output_encoding();\n+\treencoded = logmsg_reencode(commit, encoding);\n \tmessage   = reencoded ? reencoded : commit->buffer;\n \tret->author = author_name;\n \tret->author_mail = author_mail;\ndiff --git a/commit.h b/commit.h\nindex 9f21313..a822af8 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -99,8 +99,6 @@ extern int has_non_ascii(const char *text);\n struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */\n extern char *logmsg_reencode(const struct commit *commit,\n \t\t\t     const char *output_encoding);\n-extern char *reencode_commit_message(const struct commit *commit,\n-\t\t\t\t     const char **encoding_p);\n extern void get_commit_format(const char *arg, struct rev_info *);\n extern const char *format_subject(struct strbuf *sb, const char *msg,\n \t\t\t\t  const char *line_separator);\ndiff --git a/pretty.c b/pretty.c\nindex 8b1ea9f..c311a68 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1341,16 +1341,6 @@ void pp_remainder(const struct pretty_print_context *pp,\n \t}\n }\n \n-char *reencode_commit_message(const struct commit *commit, const char **encoding_p)\n-{\n-\tconst char *encoding;\n-\n-\tencoding = get_log_output_encoding();\n-\tif (encoding_p)\n-\t\t*encoding_p = encoding;\n-\treturn logmsg_reencode(commit, encoding);\n-}\n-\n void pretty_print_commit(const struct pretty_print_context *pp,\n \t\t\t const struct commit *commit,\n \t\t\t struct strbuf *sb)\n@@ -1367,7 +1357,8 @@ void pretty_print_commit(const struct pretty_print_context *pp,\n \t\treturn;\n \t}\n \n-\treencoded = reencode_commit_message(commit, &encoding);\n+\tencoding = get_log_output_encoding();\n+\treencoded = logmsg_reencode(commit, encoding);\n \tif (reencoded) {\n \t\tmsg = reencoded;\n \t}\n-- \n1.8.0.rc3.112.gdb88a5e\n"},{"id":"201470","messageId":"1350539128-21577-3-git-send-email-gitster@pobox.com","threadId":"31857","inReplyTo":"1350539128-21577-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/6] format_note(): simplify API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-18T05:45:24Z","receivedAt":"2012-10-18T05:45:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We either stuff the notes message without modification for %N\nuserformat, or format it for human consumption.  Using two bits\nis an overkill that does not benefit anybody.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n notes.c    | 13 +++++++------\n notes.h    |  6 +-----\n pretty.c   |  5 ++---\n revision.c |  2 +-\n 4 files changed, 11 insertions(+), 15 deletions(-)\n\ndiff --git a/notes.c b/notes.c\nindex bc454e1..97097db 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -1204,10 +1204,11 @@ void free_notes(struct notes_tree *t)\n  * If the given notes_tree is NULL, the internal/default notes_tree will be\n  * used instead.\n  *\n- * 'flags' is a bitwise combination of the flags for format_display_notes.\n+ * (raw != 0) gives the %N userformat; otherwise, the note message is given\n+ * for human consumption.\n  */\n static void format_note(struct notes_tree *t, const unsigned char *object_sha1,\n-\t\t\tstruct strbuf *sb, const char *output_encoding, int flags)\n+\t\t\tstruct strbuf *sb, const char *output_encoding, int raw)\n {\n \tstatic const char utf8[] = \"utf-8\";\n \tconst unsigned char *sha1;\n@@ -1244,7 +1245,7 @@ static void format_note(struct notes_tree *t, const unsigned char *object_sha1,\n \tif (msglen && msg[msglen - 1] == '\\n')\n \t\tmsglen--;\n \n-\tif (flags & NOTES_SHOW_HEADER) {\n+\tif (!raw) {\n \t\tconst char *ref = t->ref;\n \t\tif (!ref || !strcmp(ref, GIT_NOTES_DEFAULT_REF)) {\n \t\t\tstrbuf_addstr(sb, \"\\nNotes:\\n\");\n@@ -1260,7 +1261,7 @@ static void format_note(struct notes_tree *t, const unsigned char *object_sha1,\n \tfor (msg_p = msg; msg_p < msg + msglen; msg_p += linelen + 1) {\n \t\tlinelen = strchrnul(msg_p, '\\n') - msg_p;\n \n-\t\tif (flags & NOTES_INDENT)\n+\t\tif (!raw)\n \t\t\tstrbuf_addstr(sb, \"    \");\n \t\tstrbuf_add(sb, msg_p, linelen);\n \t\tstrbuf_addch(sb, '\\n');\n@@ -1270,13 +1271,13 @@ static void format_note(struct notes_tree *t, const unsigned char *object_sha1,\n }\n \n void format_display_notes(const unsigned char *object_sha1,\n-\t\t\t  struct strbuf *sb, const char *output_encoding, int flags)\n+\t\t\t  struct strbuf *sb, const char *output_encoding, int raw)\n {\n \tint i;\n \tassert(display_notes_trees);\n \tfor (i = 0; display_notes_trees[i]; i++)\n \t\tformat_note(display_notes_trees[i], object_sha1, sb,\n-\t\t\t    output_encoding, flags);\n+\t\t\t    output_encoding, raw);\n }\n \n int copy_note(struct notes_tree *t,\ndiff --git a/notes.h b/notes.h\nindex 3592b19..3324c48 100644\n--- a/notes.h\n+++ b/notes.h\n@@ -237,10 +237,6 @@ void prune_notes(struct notes_tree *t, int flags);\n  */\n void free_notes(struct notes_tree *t);\n \n-/* Flags controlling how notes are formatted */\n-#define NOTES_SHOW_HEADER 1\n-#define NOTES_INDENT 2\n-\n struct string_list;\n \n struct display_notes_opt {\n@@ -274,7 +270,7 @@ void init_display_notes(struct display_notes_opt *opt);\n  * You *must* call init_display_notes() before using this function.\n  */\n void format_display_notes(const unsigned char *object_sha1,\n-\t\t\t  struct strbuf *sb, const char *output_encoding, int flags);\n+\t\t\t  struct strbuf *sb, const char *output_encoding, int raw);\n \n /*\n  * Load the notes tree from each ref listed in 'refs'.  The output is\ndiff --git a/pretty.c b/pretty.c\nindex c311a68..735cf0f 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1035,7 +1035,7 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \tcase 'N':\n \t\tif (c->pretty_ctx->show_notes) {\n \t\t\tformat_display_notes(commit->object.sha1, sb,\n-\t\t\t\t    get_log_output_encoding(), 0);\n+\t\t\t\t\t     get_log_output_encoding(), 1);\n \t\t\treturn 1;\n \t\t}\n \t\treturn 0;\n@@ -1419,8 +1419,7 @@ void pretty_print_commit(const struct pretty_print_context *pp,\n \t\tstrbuf_addch(sb, '\\n');\n \n \tif (pp->show_notes)\n-\t\tformat_display_notes(commit->object.sha1, sb, encoding,\n-\t\t\t\t     NOTES_SHOW_HEADER | NOTES_INDENT);\n+\t\tformat_display_notes(commit->object.sha1, sb, encoding, 0);\n \n \tfree(reencoded);\n }\ndiff --git a/revision.c b/revision.c\nindex a09e60b..ddfba11 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2236,7 +2236,7 @@ static int commit_match(struct commit *commit, struct rev_info *opt)\n \t\tif (!buf.len)\n \t\t\tstrbuf_addstr(&buf, commit->buffer);\n \t\tformat_display_notes(commit->object.sha1, &buf,\n-\t\t\t\t     get_log_output_encoding(), 0);\n+\t\t\t\t     get_log_output_encoding(), 1);\n \t}\n \n \t/* Find either in the commit object, or in the temporary */\n-- \n1.8.0.rc3.112.gdb88a5e\n"},{"id":"201471","messageId":"1350539128-21577-4-git-send-email-gitster@pobox.com","threadId":"31857","inReplyTo":"1350539128-21577-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/6] pretty: prepare notes message at a centralized place","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-18T05:45:25Z","receivedAt":"2012-10-18T05:45:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Instead of passing a boolean show_notes around, pass an optional\nstring that is to be inserted after the log message proper is shown.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n commit.h   |  2 +-\n log-tree.c | 14 +++++++++++++-\n pretty.c   |  9 ++++-----\n 3 files changed, 18 insertions(+), 7 deletions(-)\n\ndiff --git a/commit.h b/commit.h\nindex a822af8..7b43e45 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -86,7 +86,7 @@ struct pretty_print_context {\n \tenum date_mode date_mode;\n \tunsigned date_mode_explicit:1;\n \tint need_8bit_cte;\n-\tint show_notes;\n+\tchar *notes_message;\n \tstruct reflog_walk_info *reflog_info;\n \tconst char *output_encoding;\n };\ndiff --git a/log-tree.c b/log-tree.c\nindex c894930..84e9f5b 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -540,7 +540,6 @@ void show_log(struct rev_info *opt)\n \tstruct pretty_print_context ctx = {0};\n \n \topt->loginfo = NULL;\n-\tctx.show_notes = opt->show_notes;\n \tif (!opt->verbose_header) {\n \t\tgraph_show_commit(opt->graph);\n \n@@ -648,6 +647,18 @@ void show_log(struct rev_info *opt)\n \tif (!commit->buffer)\n \t\treturn;\n \n+\tif (opt->show_notes) {\n+\t\tint raw;\n+\t\tstruct strbuf notebuf = STRBUF_INIT;\n+\n+\t\traw = (opt->commit_format == CMIT_FMT_USERFORMAT);\n+\t\tformat_display_notes(commit->object.sha1, &notebuf,\n+\t\t\t\t     get_log_output_encoding(), raw);\n+\t\tctx.notes_message = notebuf.len\n+\t\t\t? strbuf_detach(&notebuf, NULL)\n+\t\t\t: xcalloc(1, 1);\n+\t}\n+\n \t/*\n \t * And then the pretty-printed message itself\n \t */\n@@ -689,6 +700,7 @@ void show_log(struct rev_info *opt)\n \t}\n \n \tstrbuf_release(&msgbuf);\n+\tfree(ctx.notes_message);\n }\n \n int log_tree_diff_flush(struct rev_info *opt)\ndiff --git a/pretty.c b/pretty.c\nindex 735cf0f..a53eb53 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1033,9 +1033,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \t\t}\n \t\treturn 0;\t/* unknown %g placeholder */\n \tcase 'N':\n-\t\tif (c->pretty_ctx->show_notes) {\n-\t\t\tformat_display_notes(commit->object.sha1, sb,\n-\t\t\t\t\t     get_log_output_encoding(), 1);\n+\t\tif (c->pretty_ctx->notes_message) {\n+\t\t\tstrbuf_addstr(sb, c->pretty_ctx->notes_message);\n \t\t\treturn 1;\n \t\t}\n \t\treturn 0;\n@@ -1418,8 +1417,8 @@ void pretty_print_commit(const struct pretty_print_context *pp,\n \tif (pp->fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)\n \t\tstrbuf_addch(sb, '\\n');\n \n-\tif (pp->show_notes)\n-\t\tformat_display_notes(commit->object.sha1, sb, encoding, 0);\n+\tif (pp->notes_message && *pp->notes_message)\n+\t\tstrbuf_addstr(sb, pp->notes_message);\n \n \tfree(reencoded);\n }\n-- \n1.8.0.rc3.112.gdb88a5e\n"},{"id":"201472","messageId":"1350539128-21577-5-git-send-email-gitster@pobox.com","threadId":"31857","inReplyTo":"1350539128-21577-1-git-send-email-gitster@pobox.com","subject":"[PATCH 4/6] pretty_print_commit(): do not append notes message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-18T05:45:26Z","receivedAt":"2012-10-18T05:45:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The only case pretty_print_commit() appends notes message to the log\nmessage taken from the commit is when show_log() calls it with the\nnotes_message field set, and the output format is not the userformat\n(i.e. when substituting \"%N\").  No other users of this function sets\nthis field in the pretty_print_context, as can be easily verified in\nthe previous step.\n\nHoist the code to append the notes message to the caller.\n\nUp to this point, no functionality change is intended.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n log-tree.c | 3 +++\n pretty.c   | 3 ---\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 84e9f5b..e7e08f4 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -672,6 +672,9 @@ void show_log(struct rev_info *opt)\n \tctx.reflog_info = opt->reflog_info;\n \tctx.fmt = opt->commit_format;\n \tpretty_print_commit(&ctx, commit, &msgbuf);\n+\tif ((ctx.fmt != CMIT_FMT_USERFORMAT) &&\n+\t    ctx.notes_message && *ctx.notes_message)\n+\t\tstrbuf_addstr(&msgbuf, ctx.notes_message);\n \n \tif (opt->add_signoff)\n \t\tappend_signoff(&msgbuf, opt->add_signoff);\ndiff --git a/pretty.c b/pretty.c\nindex a53eb53..1925e9c 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1417,9 +1417,6 @@ void pretty_print_commit(const struct pretty_print_context *pp,\n \tif (pp->fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)\n \t\tstrbuf_addch(sb, '\\n');\n \n-\tif (pp->notes_message && *pp->notes_message)\n-\t\tstrbuf_addstr(sb, pp->notes_message);\n-\n \tfree(reencoded);\n }\n \n-- \n1.8.0.rc3.112.gdb88a5e\n"},{"id":"201474","messageId":"1350539128-21577-6-git-send-email-gitster@pobox.com","threadId":"31857","inReplyTo":"1350539128-21577-1-git-send-email-gitster@pobox.com","subject":"[PATCH 5/6] format-patch: append --signature after notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-18T05:45:27Z","receivedAt":"2012-10-18T05:45:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When appending a new signature with \"format-patch --signature\", if\nthe \"--notes\" option is also in effect, the location of the new\nsignature (and if the signature should be added in the first place)\nshould be decided using the contents of the original commit log\nmessage, before the message from the notes is added.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n log-tree.c              |  6 ++++--\n t/t4014-format-patch.sh | 12 ++++++++++--\n 2 files changed, 14 insertions(+), 4 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex e7e08f4..4390b11 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -672,12 +672,14 @@ void show_log(struct rev_info *opt)\n \tctx.reflog_info = opt->reflog_info;\n \tctx.fmt = opt->commit_format;\n \tpretty_print_commit(&ctx, commit, &msgbuf);\n+\n+\tif (opt->add_signoff)\n+\t\tappend_signoff(&msgbuf, opt->add_signoff);\n+\n \tif ((ctx.fmt != CMIT_FMT_USERFORMAT) &&\n \t    ctx.notes_message && *ctx.notes_message)\n \t\tstrbuf_addstr(&msgbuf, ctx.notes_message);\n \n-\tif (opt->add_signoff)\n-\t\tappend_signoff(&msgbuf, opt->add_signoff);\n \tif (opt->show_log_size) {\n \t\tprintf(\"log size %i\\n\", (int)msgbuf.len);\n \t\tgraph_show_oneline(opt->graph);\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 959aa26..bea6381 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -616,8 +616,16 @@ test_expect_success 'format-patch --in-reply-to' '\n '\n \n test_expect_success 'format-patch --signoff' '\n-\tgit format-patch -1 --signoff --stdout |\n-\tgrep \"^Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\"\n+\tgit format-patch -1 --signoff --stdout >out &&\n+\tgrep \"^Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" out\n+'\n+\n+test_expect_success 'format-patch --notes --signoff' '\n+\tgit notes --ref test add -m \"test message\" HEAD &&\n+\tgit format-patch -1 --signoff --stdout --notes=test >out &&\n+\t# Notes message must come after S-o-b\n+\t! sed \"/^Signed-off-by: /q\" out | grep \"test message\" &&\n+\tsed \"1,/^Signed-off-by: /d\" out | grep \"test message\"\n '\n \n echo \"fatal: --name-only does not make sense\" > expect.name-only\n-- \n1.8.0.rc3.112.gdb88a5e\n"},{"id":"201473","messageId":"1350539128-21577-7-git-send-email-gitster@pobox.com","threadId":"31857","inReplyTo":"1350539128-21577-1-git-send-email-gitster@pobox.com","subject":"[PATCH 6/6] format-patch --notes: show notes after three-dashes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-18T05:45:28Z","receivedAt":"2012-10-18T05:45:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When inserting the note after the commit log message to format-patch\noutput, add three dashes before the note.  Record the fact that we\ndid so in the rev_info and omit showing duplicated three dashes in\nthe usual codepath that is used when notes are not being shown.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n log-tree.c              | 15 +++++++++++----\n revision.h              |  1 +\n t/t4014-format-patch.sh |  7 +++++--\n 3 files changed, 17 insertions(+), 6 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 4390b11..712a22b 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -677,8 +677,13 @@ void show_log(struct rev_info *opt)\n \t\tappend_signoff(&msgbuf, opt->add_signoff);\n \n \tif ((ctx.fmt != CMIT_FMT_USERFORMAT) &&\n-\t    ctx.notes_message && *ctx.notes_message)\n+\t    ctx.notes_message && *ctx.notes_message) {\n+\t\tif (ctx.fmt == CMIT_FMT_EMAIL) {\n+\t\t\tstrbuf_addstr(&msgbuf, \"---\\n\");\n+\t\t\topt->shown_dashes = 1;\n+\t\t}\n \t\tstrbuf_addstr(&msgbuf, ctx.notes_message);\n+\t}\n \n \tif (opt->show_log_size) {\n \t\tprintf(\"log size %i\\n\", (int)msgbuf.len);\n@@ -710,6 +715,7 @@ void show_log(struct rev_info *opt)\n \n int log_tree_diff_flush(struct rev_info *opt)\n {\n+\topt->shown_dashes = 0;\n \tdiffcore_std(&opt->diffopt);\n \n \tif (diff_queue_is_empty()) {\n@@ -737,10 +743,11 @@ int log_tree_diff_flush(struct rev_info *opt)\n \t\t\t\t\topt->diffopt.output_prefix_data);\n \t\t\t\tfwrite(msg->buf, msg->len, 1, stdout);\n \t\t\t}\n-\t\t\tif ((pch & opt->diffopt.output_format) == pch) {\n-\t\t\t\tprintf(\"---\");\n+\t\t\tif (!opt->shown_dashes) {\n+\t\t\t\tif ((pch & opt->diffopt.output_format) == pch)\n+\t\t\t\t\tprintf(\"---\");\n+\t\t\t\tputchar('\\n');\n \t\t\t}\n-\t\t\tputchar('\\n');\n \t\t}\n \t}\n \tdiff_flush(&opt->diffopt);\ndiff --git a/revision.h b/revision.h\nindex a95bd0b..059bfff 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -111,6 +111,7 @@ struct rev_info {\n \n \t/* Format info */\n \tunsigned int\tshown_one:1,\n+\t\t\tshown_dashes:1,\n \t\t\tshow_merge:1,\n \t\t\tshow_notes:1,\n \t\t\tshow_notes_given:1,\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex bea6381..9750ba6 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -623,9 +623,12 @@ test_expect_success 'format-patch --signoff' '\n test_expect_success 'format-patch --notes --signoff' '\n \tgit notes --ref test add -m \"test message\" HEAD &&\n \tgit format-patch -1 --signoff --stdout --notes=test >out &&\n-\t# Notes message must come after S-o-b\n+\t# Three dashes must come after S-o-b\n \t! sed \"/^Signed-off-by: /q\" out | grep \"test message\" &&\n-\tsed \"1,/^Signed-off-by: /d\" out | grep \"test message\"\n+\tsed \"1,/^Signed-off-by: /d\" out | grep \"test message\" &&\n+\t# Notes message must come after three dashes\n+\t! sed \"/^---$/q\" out | grep \"test message\" &&\n+\tsed \"1,/^---$/d\" out | grep \"test message\"\n '\n \n echo \"fatal: --name-only does not make sense\" > expect.name-only\n-- \n1.8.0.rc3.112.gdb88a5e\n"},{"id":"201483","messageId":"20121018074921.GC9999@sigill.intra.peff.net","threadId":"31857","inReplyTo":"1350539128-21577-4-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 3/6] pretty: prepare notes message at a centralized place","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-18T07:49:21Z","receivedAt":"2012-10-18T07:49:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 17, 2012 at 10:45:25PM -0700, Junio C Hamano wrote:\n\n> +\tif (opt->show_notes) {\n> +\t\tint raw;\n> +\t\tstruct strbuf notebuf = STRBUF_INIT;\n> +\n> +\t\traw = (opt->commit_format == CMIT_FMT_USERFORMAT);\n> +\t\tformat_display_notes(commit->object.sha1, &notebuf,\n> +\t\t\t\t     get_log_output_encoding(), raw);\n> +\t\tctx.notes_message = notebuf.len\n> +\t\t\t? strbuf_detach(&notebuf, NULL)\n> +\t\t\t: xcalloc(1, 1);\n> +\t}\n\nThis last line seems like it is caused by a bug in the strbuf API.\nDetaching an empty string will sometimes get you NULL and sometimes not.\nFor example, this:\n\n  struct strbuf foo = STRBUF_INIT;\n  strbuf_detach(&foo, NULL);\n\nwill return NULL. But this:\n\n  struct strbuf foo = STRBUF_INIT;\n  strbuf_addstr(&foo, \"bar\");\n  strbuf_reset(&foo);\n  strbuf_detach(&foo, NULL);\n\nwill get you a zero-length string. Which just seems insane to me. The\nwhole point of strbuf_detach is that you do not have to care about the\ninternal representation. It should probably always return a newly\nallocated zero-length string.\n\nLooking through a few uses of strbuf_detach, it looks like callers\nassume they will always get a pointer from strbuf_detach, and we are\nsaved by implementation details. For example, sha1_file_to_archive might\nhave an empty file, but the fact that strbuf_attach always allocates a\nbyte means that the detach will never return NULL. Similarly,\nargv_array_pushf would never want to turn an empty string into an\naccidental NULL; it is saved by the fact that strbuf_vaddf will always\npreemptively allocate 64 bytes.\n\nIt's possible that switching it would create bugs elsewhere (there are\nover 100 uses of strbuf_detach, so maybe somebody really does want this\nNULL behavior), but I tend to think it is just as likely to be fixing\nundiscovered bugs.\n\n-Peff\n"},{"id":"201489","messageId":"7vfw5cm96a.fsf@alter.siamese.dyndns.org","threadId":"31857","inReplyTo":"20121018074921.GC9999@sigill.intra.peff.net","subject":"Re: [PATCH 3/6] pretty: prepare notes message at a centralized place","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-18T09:17:01Z","receivedAt":"2012-10-18T09:17:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> It's possible that switching it would create bugs elsewhere (there are\n> over 100 uses of strbuf_detach, so maybe somebody really does want this\n> NULL behavior), but I tend to think it is just as likely to be fixing\n> undiscovered bugs.\n\nYeah, I tend to agree.\n\nThis \"format-patch --notes\" is obviously a post 1.8.0 topic, and so\nis the strbuf_detach() clean-up.  Let me bookmark this thread in\ncase it hasn't been resolved when I came back from my vacation, so\nthat I won't forget ;-).\n"},{"id":"201491","messageId":"20121018091828.GA14838@sigill.intra.peff.net","threadId":"31857","inReplyTo":"7vfw5cm96a.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/6] pretty: prepare notes message at a centralized place","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-18T09:18:28Z","receivedAt":"2012-10-18T09:18:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 18, 2012 at 02:17:01AM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > It's possible that switching it would create bugs elsewhere (there are\n> > over 100 uses of strbuf_detach, so maybe somebody really does want this\n> > NULL behavior), but I tend to think it is just as likely to be fixing\n> > undiscovered bugs.\n> \n> Yeah, I tend to agree.\n> \n> This \"format-patch --notes\" is obviously a post 1.8.0 topic, and so\n> is the strbuf_detach() clean-up.  Let me bookmark this thread in\n> case it hasn't been resolved when I came back from my vacation, so\n> that I won't forget ;-).\n\nActually, I have found a few segfaults, one of them remotely triggerable\nin http-backend. I think it can probably wait until post-1.8.0 as it\ndoes not have any security implications, though.\n\nDetails in a moment.\n\n-Peff\n"},{"id":"201497","messageId":"20121018100012.GE9999@sigill.intra.peff.net","threadId":"31857","inReplyTo":"20121018074921.GC9999@sigill.intra.peff.net","subject":"[PATCH] strbuf: always return a non-NULL value from strbuf_detach","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-18T10:00:12Z","receivedAt":"2012-10-18T10:00:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 18, 2012 at 03:49:21AM -0400, Jeff King wrote:\n\n> This last line seems like it is caused by a bug in the strbuf API.\n> Detaching an empty string will sometimes get you NULL and sometimes not.\n> For example, this:\n> \n>   struct strbuf foo = STRBUF_INIT;\n>   strbuf_detach(&foo, NULL);\n> \n> will return NULL. But this:\n> \n>   struct strbuf foo = STRBUF_INIT;\n>   strbuf_addstr(&foo, \"bar\");\n>   strbuf_reset(&foo);\n>   strbuf_detach(&foo, NULL);\n> \n> will get you a zero-length string. Which just seems insane to me. The\n> whole point of strbuf_detach is that you do not have to care about the\n> internal representation. It should probably always return a newly\n> allocated zero-length string.\n> [...]\n> It's possible that switching it would create bugs elsewhere (there are\n> over 100 uses of strbuf_detach, so maybe somebody really does want this\n> NULL behavior), but I tend to think it is just as likely to be fixing\n> undiscovered bugs.\n\nSo I just read through all 108 grep hits for strbuf_detach. In almost\nevery case, the NULL return is not triggerable, because we do _some_\nstrbuf operation. And even if it is empty, like strbuf_read from an\nempty source, or strbuf_addstr on an empty string, we still end up\ncalling `strbuf_grow(sb, 0)`, which will allocate.\n\nThere are a few cases where there are code paths where we really might\nnot add anything to the strbuf, and changing strbuf_detach would have an\nimpact:\n\n  In log.c:cmd_format_patch, creating the rev.extra_headers string from\n  the individual headers currently ends up NULL when you have no such\n  headers. But it ends up not mattering if we have NULL or an empty\n  string, since all code paths just end up appending it to our headers\n  anyway, and the empty string is a noop.\n\n  In commit.c:read_commit_extra_header_lines, a commit without a value\n  will get a NULL value instead of the empty string. But we end up not\n  dereferencing the NULL, because it just gets fed to add_extra_header\n  later, which will only look at the value if its length parameter is\n  non-zero. So it is built to expect the current behavior, but would be\n  fine with a switch.\n\n  In http-push.c:xml_entities, we will return NULL if fed an empty\n  string. You can dereference NULL by doing \"git http-push ''\", although\n  on glibc systems it will not segfault, because we feed the NULL to\n  printf, which converts it to \"(null)\".\n\n  In http-backend.c:get_parameters, we call url_decode_parameter_* to\n  look at the contents of $QUERY_STRING.  These functions can return\n  NULL via strbuf_detach if fed an empty string. We are ready to handle\n  a NULL value like \"empty=&other=value\". But not an empty name, like\n  \"one=1&&two=2\" (note the bogus double-& which yields an empty\n  parameter). You can get a segfault by sending that to a smart-http\n  server.\n\nProbably more detail than you wanted, but I'm pretty confident now that\nwe should switch it, and that it won't cause any regressions. Patch is\nbelow.\n\n-- >8 --\nSubject: strbuf: always return a non-NULL value from strbuf_detach\n\nThe current behavior is to return NULL when strbuf did not\nactually allocate a string. This can be quite surprising to\ncallers, though, who may feed the strbuf from arbitrary data\nand expect to always get a valid value.\n\nIn most cases, it does not make a difference because calling\nany strbuf function will cause an allocation (even if the\nfunction ends up not inserting any data). But if the code is\nstructured like:\n\n  struct strbuf buf = STRBUF_INIT;\n  if (some_condition)\n\t  strbuf_addstr(&buf, some_string);\n  return strbuf_detach(&buf, NULL);\n\nthen you may or may not return NULL, depending on the\ncondition. This can cause us to segfault in http-push\n(when fed an empty URL) and in http-backend (when an empty\nparameter like \"foo=bar&&\" is in the $QUERY_STRING).\n\nThis patch forces strbuf_detach to allocate an empty\nNUL-terminated string when it is called on a strbuf that has\nnot been allocated.\n\nI investigated all call-sites of strbuf_detach. The majority\nare either not affected by the change (because they call a\nstrbuf_* function unconditionally), or can handle the empty\nstring just as easily as NULL.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n strbuf.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex 0510f76..4b9e30c 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -44,7 +44,9 @@ char *strbuf_detach(struct strbuf *sb, size_t *sz)\n \n char *strbuf_detach(struct strbuf *sb, size_t *sz)\n {\n-\tchar *res = sb->alloc ? sb->buf : NULL;\n+\tchar *res;\n+\tstrbuf_grow(sb, 0);\n+\tres = sb->buf;\n \tif (sz)\n \t\t*sz = sb->len;\n \tstrbuf_init(sb, 0);\n-- \n1.8.0.rc3.3.gba630e1\n"},{"id":"201498","messageId":"20121018100234.GF9999@sigill.intra.peff.net","threadId":"31857","inReplyTo":"1350539128-21577-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/6] Bring \"format-patch --notes\" closer to a real feature","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-18T10:02:34Z","receivedAt":"2012-10-18T10:02:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 17, 2012 at 10:45:22PM -0700, Junio C Hamano wrote:\n\n> We never advertised the \"--notes\" option to format-patch (or\n> anything related to the pretty format options for that matter)\n> because the behaviour of these options was whatever they happened to\n> do, not what they were designed to do.\n> \n> It had a few obvious glitches:\n> \n>  * The notes section was appended immediately after the log message,\n>    and then the three-dash line was added.  Such a supplimental\n>    material should come after the three-dash line.\n> \n>  * The logic to append a new sign-off with \"format-patch --signoff\"\n>    worked on the message after the note was added, which made the\n>    detection of existing sign-off lines incorrect.\n> \n> This updates the handling of \"--notes\" option to correct these, in\n> an attempt to bring it closer to a real feature.\n\nI just read through the whole series. Aside from the tangent about\nstrbuf_detach, I didn't see anything wrong. Thanks for moving this\nforward.\n\n-Peff\n"},{"id":"201502","messageId":"CACsJy8Dxx6JtRXP_HWNBrobNems+4UsaOGSRef08cf8--b5dOw@mail.gmail.com","threadId":"31857","inReplyTo":"1350539128-21577-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/6] Bring \"format-patch --notes\" closer to a real feature","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-10-18T11:06:10Z","receivedAt":"2012-10-18T11:06:10Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Oct 18, 2012 at 12:45 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> This replaces the earlier \"wip\" with a real thing.\n>\n> We never advertised the \"--notes\" option to format-patch (or\n> anything related to the pretty format options for that matter)\n> because the behaviour of these options was whatever they happened to\n> do, not what they were designed to do.\n\nStupid question: does \"git am\" recreate notes from \"format-patch\n--notes\" output? If it does not, should it? I think it could be a nice\nway of copying notes from one machine to another, but not enabled by\ndefault (iow \"am --notes\").\n-- \nDuy\n"},{"id":"201510","messageId":"507FF500.2090702@drmicha.warpmail.net","threadId":"31857","inReplyTo":"CACsJy8Dxx6JtRXP_HWNBrobNems+4UsaOGSRef08cf8--b5dOw@mail.gmail.com","subject":"Re: [PATCH 0/6] Bring \"format-patch --notes\" closer to a real feature","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2012-10-18T12:24:32Z","receivedAt":"2012-10-18T12:24:32Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Nguyen Thai Ngoc Duy venit, vidit, dixit 18.10.2012 13:06:\n> On Thu, Oct 18, 2012 at 12:45 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> This replaces the earlier \"wip\" with a real thing.\n>>\n>> We never advertised the \"--notes\" option to format-patch (or\n>> anything related to the pretty format options for that matter)\n>> because the behaviour of these options was whatever they happened to\n>> do, not what they were designed to do.\n> \n> Stupid question: does \"git am\" recreate notes from \"format-patch\n> --notes\" output? If it does not, should it? I think it could be a nice\n> way of copying notes from one machine to another, but not enabled by\n> default (iow \"am --notes\").\n> \n\nNot yet, but you can already start basing your am patches on top of\nJunio's series ;)\n\nam should allow a notes ref (--notes=hereyougo) like other similar\ninstances.\n\nMichael\n"},{"id":"201522","messageId":"7v391bn244.fsf@alter.siamese.dyndns.org","threadId":"31857","inReplyTo":"CACsJy8Dxx6JtRXP_HWNBrobNems+4UsaOGSRef08cf8--b5dOw@mail.gmail.com","subject":"Re: [PATCH 0/6] Bring \"format-patch --notes\" closer to a real feature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-18T17:04:11Z","receivedAt":"2012-10-18T17:04:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n\n> On Thu, Oct 18, 2012 at 12:45 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> This replaces the earlier \"wip\" with a real thing.\n>>\n>> We never advertised the \"--notes\" option to format-patch (or\n>> anything related to the pretty format options for that matter)\n>> because the behaviour of these options was whatever they happened to\n>> do, not what they were designed to do.\n>\n> Stupid question: does \"git am\" recreate notes from \"format-patch\n> --notes\" output? If it does not, should it? I think it could be a nice\n> way of copying notes from one machine to another, but not enabled by\n> default (iow \"am --notes\").\n\nThinking about what the \"notes\" are, I do not think it should, at\nleast by default; the model is broken at the conceptual level.\n\nThe notes are comments on a commit you make after the fact, because\nyou do not want to (or cannot afford to) amend the commit to include\nthe comment.  When you are sending it out over e-mailto be applied\nwith \"am\", as opposed to asking the other to pull, you are by\ndefinition willing to see the commit replayed with modification.\n\nI think it is sensible for format-patch/am pipeline when asked to\nuse --notes to add the notes section after \"---\" as additional\nmaterial to help the recipient understand the context of the patch\nbetter, which is done with this series.  If the submitter (or the\nrecipient) wants to incorporate the description from the notes to\nupdate the proposed log message, it can easily be done by editing\nthe output of \"format-patch --notes\" before sending it out (or\nbefore applying it with \"am\").\n\nThat does not mean that the recipient should not use notes for his\nown purpose on the resulting commit, by the way.  It would be a\nconvenient feature to prime the contents of such a new note the\nrecipient creates on the resulting commit from the comment after\n\"---\" before the diffstat or \"diff --git\" line.  Note that (no pun\nintended) that additional comment does not have to originate from\nany notes in the submitter's repository.  If saving the additional\ncomments the submitter attached from the notes to the patch is\nuseful, it would equally be useful to save typed-in comments on the\npatch that came from the submitter's fingers, not from the notes.\n\nIt is something you can do by inspecting $dotest/patch file in your\npost-applypatch hook with today's git.  If many people use and find\nsuch a feature desirable, we could add inbuilt support for it, but I\ndo not think we are there yet.\n"},{"id":"201529","messageId":"C2D3CE0879B345FFAC2A6487760B5378@PhilipOakley","threadId":"31857","inReplyTo":"1350539128-21577-7-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 6/6] format-patch --notes: show notes after three-dashes","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2012-10-18T21:35:25Z","receivedAt":"2012-10-18T21:35:25Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" <gitster@pobox.com>\n> When inserting the note after the commit log message to format-patch\n> output, add three dashes before the note.  Record the fact that we\n> did so in the rev_info and omit showing duplicated three dashes in\n> the usual codepath that is used when notes are not being shown.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nShould this also include a documentation update to make this substantive \nbenefit visible, whether that be in the format-patch man pages, the \nSubmittingPatches guide, in the git-notes description of 'A typical \nuse...', or even in the user-manual?\n\nDo you have a preference?\n\n> ---\n> log-tree.c              | 15 +++++++++++----\n> revision.h              |  1 +\n> t/t4014-format-patch.sh |  7 +++++--\n> 3 files changed, 17 insertions(+), 6 deletions(-)\n>\n> diff --git a/log-tree.c b/log-tree.c\n> index 4390b11..712a22b 100644\n> --- a/log-tree.c\n> +++ b/log-tree.c\n> @@ -677,8 +677,13 @@ void show_log(struct rev_info *opt)\n>  append_signoff(&msgbuf, opt->add_signoff);\n>\n>  if ((ctx.fmt != CMIT_FMT_USERFORMAT) &&\n> -     ctx.notes_message && *ctx.notes_message)\n> +     ctx.notes_message && *ctx.notes_message) {\n> + if (ctx.fmt == CMIT_FMT_EMAIL) {\n> + strbuf_addstr(&msgbuf, \"---\\n\");\n> + opt->shown_dashes = 1;\n> + }\n>  strbuf_addstr(&msgbuf, ctx.notes_message);\n> + }\n>\n>  if (opt->show_log_size) {\n>  printf(\"log size %i\\n\", (int)msgbuf.len);\n> @@ -710,6 +715,7 @@ void show_log(struct rev_info *opt)\n>\n> int log_tree_diff_flush(struct rev_info *opt)\n> {\n> + opt->shown_dashes = 0;\n>  diffcore_std(&opt->diffopt);\n>\n>  if (diff_queue_is_empty()) {\n> @@ -737,10 +743,11 @@ int log_tree_diff_flush(struct rev_info *opt)\n>  opt->diffopt.output_prefix_data);\n>  fwrite(msg->buf, msg->len, 1, stdout);\n>  }\n> - if ((pch & opt->diffopt.output_format) == pch) {\n> - printf(\"---\");\n> + if (!opt->shown_dashes) {\n> + if ((pch & opt->diffopt.output_format) == pch)\n> + printf(\"---\");\n> + putchar('\\n');\n>  }\n> - putchar('\\n');\n>  }\n>  }\n>  diff_flush(&opt->diffopt);\n> diff --git a/revision.h b/revision.h\n> index a95bd0b..059bfff 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -111,6 +111,7 @@ struct rev_info {\n>\n>  /* Format info */\n>  unsigned int shown_one:1,\n> + shown_dashes:1,\n>  show_merge:1,\n>  show_notes:1,\n>  show_notes_given:1,\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index bea6381..9750ba6 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -623,9 +623,12 @@ test_expect_success 'format-patch --signoff' '\n> test_expect_success 'format-patch --notes --signoff' '\n>  git notes --ref test add -m \"test message\" HEAD &&\n>  git format-patch -1 --signoff --stdout --notes=test >out &&\n> - # Notes message must come after S-o-b\n> + # Three dashes must come after S-o-b\n>  ! sed \"/^Signed-off-by: /q\" out | grep \"test message\" &&\n> - sed \"1,/^Signed-off-by: /d\" out | grep \"test message\"\n> + sed \"1,/^Signed-off-by: /d\" out | grep \"test message\" &&\n> + # Notes message must come after three dashes\n> + ! sed \"/^---$/q\" out | grep \"test message\" &&\n> + sed \"1,/^---$/d\" out | grep \"test message\"\n> '\n>\n> echo \"fatal: --name-only does not make sense\" > expect.name-only\n> -- \n> 1.8.0.rc3.112.gdb88a5e\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n>\n> -----\n> No virus found in this message.\n> Checked by AVG - www.avg.com\n> Version: 2013.0.2741 / Virus Database: 2614/5837 - Release Date: \n> 10/17/12\n> \n"},{"id":"201531","messageId":"7vlif3l9fw.fsf@alter.siamese.dyndns.org","threadId":"31857","inReplyTo":"C2D3CE0879B345FFAC2A6487760B5378@PhilipOakley","subject":"Re: [PATCH 6/6] format-patch --notes: show notes after three-dashes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-18T22:08:51Z","receivedAt":"2012-10-18T22:08:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" <philipoakley@iee.org> writes:\n\n> From: \"Junio C Hamano\" <gitster@pobox.com>\n>> When inserting the note after the commit log message to format-patch\n>> output, add three dashes before the note.  Record the fact that we\n>> did so in the rev_info and omit showing duplicated three dashes in\n>> the usual codepath that is used when notes are not being shown.\n>>\n>> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n>\n> Should this also include a documentation update to make this\n> substantive benefit visible, whether that be in the format-patch man\n> pages, the SubmittingPatches guide, in the git-notes description of 'A\n> typical use...', or even in the user-manual?\n\nEric Blake (http://mid.gmane.org/507EB310.8020904@redhat.com) was\nalready working on a documentation updates already, I thought.\n\nAs long as what it does is explained in format-patch, that is fine.\n\nI do not think this deserves to be in the SubmittingPatches.  We do\ntell people to hide \"here is the context of the change\" additional\nexplanation after three dashes, but how the submitters prepare that\ntext is entirely up to them (and I personally do not think notes is\nnot necessarily the right tool to do so).\n"},{"id":"201563","messageId":"7vzk3ii5vd.fsf@alter.siamese.dyndns.org","threadId":"31857","inReplyTo":"7vlif3l9fw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 6/6] format-patch --notes: show notes after three-dashes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-19T20:06:30Z","receivedAt":"2012-10-19T20:06:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> As long as what it does is explained in format-patch, that is fine.\n>\n> I do not think this deserves to be in the SubmittingPatches.  We do\n> tell people to hide \"here is the context of the change\" additional\n> explanation after three dashes, but how the submitters prepare that\n> text is entirely up to them (and I personally do not think notes is\n> not necessarily the right tool to do so).\n\nJust in case, here is what I'll queue as a placeholder on 'pu'.\n\n-- >8 --\nSubject: [PATCH] Documentation: decribe format-patch --notes\n\nEven though I coded this, I am not sure what use scenarios would benefit\nfrom this option, so the description is unnecessarily negative at this\nmoment. People who do want to use this feature need to come up with a\nmore plausible use case and replace it.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/git-format-patch.txt | 15 ++++++++++++++-\n 1 file changed, 14 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 6d43f56..066dc8b 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -20,7 +20,7 @@ SYNOPSIS\n \t\t   [--ignore-if-in-upstream]\n \t\t   [--subject-prefix=Subject-Prefix]\n \t\t   [--to=<email>] [--cc=<email>]\n-\t\t   [--cover-letter] [--quiet]\n+\t\t   [--cover-letter] [--quiet] [--notes[=<ref>]]\n \t\t   [<common diff options>]\n \t\t   [ <since> | <revision range> ]\n \n@@ -191,6 +191,19 @@ will want to ensure that threading is disabled for `git send-email`.\n \tcontaining the shortlog and the overall diffstat.  You can\n \tfill in a description in the file before sending it out.\n \n+--notes[=<ref>]::\n+\tAppend the notes (see linkgit:git-notes[1]) for the commit\n+\tafter the three-dash line.\n++\n+The expected use case of this is to write supporting explanation for\n+the commit that does not belong to the commit log message proper\n+when (or after) you create the commit, and include it in your patch\n+submission.  But if you can plan ahead and write it down, there may\n+not be a good reason not to write it in your commit message, and if\n+you can't, you can always edit the output of format-patch before\n+sending it out, so the practical value of this option is somewhat\n+dubious, unless your workflow is broken.\n+\n --[no]-signature=<signature>::\n \tAdd a signature to each message produced. Per RFC 3676 the signature\n \tis separated from the body by a line with '-- ' on it. If the\n-- \n1.8.0.rc3.162.g1f53438\n"},{"id":"201658","messageId":"B15B2D6C9AAB43AF9D480D9DEB3D2F47@PhilipOakley","threadId":"31857","inReplyTo":"7vlif3l9fw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 6/6] format-patch --notes: show notes after three-dashes","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2012-10-21T21:33:54Z","receivedAt":"2012-10-21T21:33:54Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" <gitster@pobox.com>\n> \"Philip Oakley\" <philipoakley@iee.org> writes:\n>\n>> From: \"Junio C Hamano\" <gitster@pobox.com>\n>>> When inserting the note after the commit log message to format-patch\n>>> output, add three dashes before the note.  Record the fact that we\n>>> did so in the rev_info and omit showing duplicated three dashes in\n>>> the usual codepath that is used when notes are not being shown.\n>>>\n>>> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n>>\n>> Should this also include a documentation update to make this\n>> substantive benefit visible, whether that be in the format-patch man\n>> pages, the SubmittingPatches guide, in the git-notes description of \n>> 'A\n>> typical use...', or even in the user-manual?\n>\n> Eric Blake (http://mid.gmane.org/507EB310.8020904@redhat.com) was\nAdded to distribution.\n\n> already working on a documentation updates already, I thought.\n>\n> As long as what it does is explained in format-patch, that is fine.\n>\n> I do not think this deserves to be in the SubmittingPatches.  We do\n> tell people to hide \"here is the context of the change\" additional\n> explanation after three dashes, but how the submitters prepare that\n> text is entirely up to them (and I personally do not think notes is\n> not necessarily the right tool to do so).\n>\nI've prepared a short set of patches for the documenation I mentioned.\n\nI, like Eric, feel some of the methods available are a bit of a Catch \n22.\nWe do need to at least hint about capabilities as a way of helping new\nusers who aren't already experts (the inverse Kruger Dunning problem).\n\nI have tried hard to make the patches concise so hopefully they will be\nacceptable.\n\nPhilip \n"}]}