{"thread":{"id":"37632","subject":"[PATCH RFC] log-tree: let format-patch not indent notes","startedAt":"2014-09-25T16:10:09Z","lastAt":"2014-09-25T18:08:32Z","messageCount":4,"participants":["Uwe Kleine-König","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"249848","messageId":"1411661409-24562-1-git-send-email-u.kleine-koenig@pengutronix.de","threadId":"37632","inReplyTo":null,"subject":"[PATCH RFC] log-tree: let format-patch not indent notes","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2014-09-25T16:10:09Z","receivedAt":"2014-09-25T16:10:09Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Commit logs as shown by git-log are usually indented by four spaces so\nhere it makes sense to do the same for commit notes.\n\nHowever when using format-patch to create a patch for submission via\ne-mail the commit log isn't indented and also the \"Notes:\" header isn't\nreally useful. So consequently don't indent and skip the header in this\ncase. This also removes the empty line between the end-of-commit marker\nand the start of the notes.\n\nSigned-off-by: Uwe Kleine-König \n---\nThis commit changes the output of format-patch (applied on this commit) from:\n\n\t...\n\tcase.\n\n\tSigned-off-by: Uwe Kleine-König \n\t---\n\n\tNotes:\n\t This commit changes the output of format-patch (applied on this commit) from:\n\nto\n\n\t...\n\tcase.\n\n\tSigned-off-by: Uwe Kleine-König \n\t---\n\tThis commit changes the output of format-patch (applied on this commit) from:\n\nwhich I consider to be more useful.\n\n log-tree.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex bcee7c596696..c1d73d8fecdf 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -585,7 +585,8 @@ void show_log(struct rev_info *opt)\n \t\tint raw;\n \t\tstruct strbuf notebuf = STRBUF_INIT;\n \n-\t\traw = (opt->commit_format == CMIT_FMT_USERFORMAT);\n+\t\traw = (opt->commit_format == CMIT_FMT_USERFORMAT) ||\n+\t\t\t(opt->commit_format == CMIT_FMT_EMAIL);\n \t\tformat_display_notes(commit->object.sha1, ¬ebuf,\n \t\t\t\t get_log_output_encoding(), raw);\n \t\tctx.notes_message = notebuf.len\n-- \n2.1.1.274.gb3e1830.dirty\n"},{"id":"249852","messageId":"xmqqeguzboka.fsf@gitster.dls.corp.google.com","threadId":"37632","inReplyTo":"1411661409-24562-1-git-send-email-u.kleine-koenig@pengutronix.de","subject":"Re: [PATCH RFC] log-tree: let format-patch not indent notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-25T17:24:53Z","receivedAt":"2014-09-25T17:24:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Uwe Kleine-König writes:\n\n> Commit logs as shown by git-log are usually indented by four spaces so\n> here it makes sense to do the same for commit notes.\n>\n> However when using format-patch to create a patch for submission via\n> e-mail the commit log isn't indented and also the \"Notes:\" header isn't\n> really useful. So consequently don't indent and skip the header in this\n> case. This also removes the empty line between the end-of-commit marker\n> and the start of the notes.\n>\n> Signed-off-by: Uwe Kleine-König \n> ---\n> This commit changes the output of format-patch (applied on this commit) from:\n>\n> \t...\n> \tcase.\n>\n> \tSigned-off-by: Uwe Kleine-König \n> \t---\n>\n> \tNotes:\n> \t This commit changes the output of format-patch (applied on this commit) from:\n>\n> to\n>\n> \t...\n> \tcase.\n>\n> \tSigned-off-by: Uwe Kleine-König \n> \t---\n> \tThis commit changes the output of format-patch (applied on this commit) from:\n>\n> which I consider to be more useful.\n\nI suspect that is fairly subjective, as the current one is in that\nform because those who wrote this feature first, reviewed, applied\nwould have considered it more useful, isn't it?\n\nBecause I never send out a format-patch output without looking it\nover in an editor, I know I can easily remove it if I find the\n\"Notes:\" out of place in the output, but if the \"Notes:\" thing\nweren't there in the first place I may scratch my head trying to\nfigure out where to update it if the information there were stale,\nso for that reason I'd find it more useful to have Notes: to remind\nme where that information comes from.\n\nBut that is just my personal preference and I am willing to be\npersuaded either way with a better argument than \"to me it looks\nnicer\".\n\nAs to indenting, because the material after three-dashes is meant to\nbe fed to \"git apply\" or \"patch\", I'd prefer to keep it to avoid\nhaving to worry about a payload that may look like part of a patch.\nThis preference is a bit stronger than the presence/absence of\n\"Notes:\".\n\nThanks.\n\n> log-tree.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/log-tree.c b/log-tree.c\n> index bcee7c596696..c1d73d8fecdf 100644\n> --- a/log-tree.c\n> +++ b/log-tree.c\n> @@ -585,7 +585,8 @@ void show_log(struct rev_info *opt)\n> \t\tint raw;\n> \t\tstruct strbuf notebuf = STRBUF_INIT;\n> \n> -\t\traw = (opt->commit_format == CMIT_FMT_USERFORMAT);\n> +\t\traw = (opt->commit_format == CMIT_FMT_USERFORMAT) ||\n> +\t\t\t(opt->commit_format == CMIT_FMT_EMAIL);\n> \t\tformat_display_notes(commit->object.sha1, ¬ebuf,\n> \t\t\t\t get_log_output_encoding(), raw);\n> \t\tctx.notes_message = notebuf.len\n"},{"id":"249854","messageId":"20140925175651.GA11673@peff.net","threadId":"37632","inReplyTo":"1411661409-24562-1-git-send-email-u.kleine-koenig@pengutronix.de","subject":"Re: [PATCH RFC] log-tree: let format-patch not indent notes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-09-25T17:56:52Z","receivedAt":"2014-09-25T17:56:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 25, 2014 at 06:10:09PM +0200, Uwe Kleine-König wrote:\n\n> Commit logs as shown by git-log are usually indented by four spaces so\n> here it makes sense to do the same for commit notes.\n> \n> However when using format-patch to create a patch for submission via\n> e-mail the commit log isn't indented and also the \"Notes:\" header isn't\n> really useful. So consequently don't indent and skip the header in this\n> case. This also removes the empty line between the end-of-commit marker\n> and the start of the notes.\n> \n> Signed-off-by: Uwe Kleine-König \n> ---\n\nI like this, though I think it is somewhat subjective, and there may be\nsome corner cases. This topic has come up before (this is the tip of\nwhat I dug up, but I did not bother reading back further myself):\n\n http://article.gmane.org/gmane.comp.version-control.git/163144\n\nYou'd also need to consider what happens with non-default notes. If you\ndo \"--show-notes=foo\" then your header is more like:\n\n Notes (foo):\n blah blah blah\n\nand your patch loses the information on the source. You may even be\npulling in from multiple sets of notes, in which case there are multiple\nheaders with multiple sources.\n\nI wonder if we would need an option to say \"I am showing notes, but from\njust one ref and I prefer the simple three-dash format\". Like\n\"--cover-notes[=]\" or something. I dunno.\n\n-Peff\n"},{"id":"249859","messageId":"20140925180832.GA31554@pengutronix.de","threadId":"37632","inReplyTo":"xmqqeguzboka.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH RFC] log-tree: let format-patch not indent notes","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2014-09-25T18:08:32Z","receivedAt":"2014-09-25T18:08:32Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello Junio,\n\nOn Thu, Sep 25, 2014 at 10:24:53AM -0700, Junio C Hamano wrote:\n> Uwe Kleine-König writes:\n> > Commit logs as shown by git-log are usually indented by four spaces so\n> > here it makes sense to do the same for commit notes.\n> >\n> > However when using format-patch to create a patch for submission via\n> > e-mail the commit log isn't indented and also the \"Notes:\" header isn't\n> > really useful. So consequently don't indent and skip the header in this\n> > case. This also removes the empty line between the end-of-commit marker\n> > and the start of the notes.\n> >\n> > Signed-off-by: Uwe Kleine-König \n> > ---\n> > This commit changes the output of format-patch (applied on this commit) from:\n> >\n> > \t...\n> > \tcase.\n> >\n> > \tSigned-off-by: Uwe Kleine-König \n> > \t---\n> >\n> > \tNotes:\n> > \t This commit changes the output of format-patch (applied on this commit) from:\n> >\n> > to\n> >\n> > \t...\n> > \tcase.\n> >\n> > \tSigned-off-by: Uwe Kleine-König \n> > \t---\n> > \tThis commit changes the output of format-patch (applied on this commit) from:\n> >\n> > which I consider to be more useful.\n> \n> I suspect that is fairly subjective, as the current one is in that\n> form because those who wrote this feature first, reviewed, applied\n> would have considered it more useful, isn't it?\nWell, I thought when the feature to dump the notes into a patch was\ncreated there was exactly one way these notes were written. This was was\ndesigned for git-log and so intended and with \"Notes:\". For\ngit-format-patch it was good enough.\n\n> Because I never send out a format-patch output without looking it\n> over in an editor, I know I can easily remove it if I find the\n> \"Notes:\" out of place in the output, but if the \"Notes:\" thing\n> weren't there in the first place I may scratch my head trying to\n> figure out where to update it if the information there were stale,\n> so for that reason I'd find it more useful to have Notes: to remind\n> me where that information comes from.\nAs you must explicitly request notes to be included in patches (--notes)\nI think it's unusual to not know where the info comes from, doesn't it?\n\nI don't know how many people use git-notes to track their comments, but\nthe first thing I do when editing patches is to remove the Notes: header\nand s/^ // on the remaining lines. And most of the time this is the\nonly thing I do and I need to touch every patch only because of\nthat.\n\n> But that is just my personal preference and I am willing to be\n> persuaded either way with a better argument than \"to me it looks\n> nicer\".\n> \n> As to indenting, because the material after three-dashes is meant to\n> be fed to \"git apply\" or \"patch\", I'd prefer to keep it to avoid\n> having to worry about a payload that may look like part of a patch.\n> This preference is a bit stronger than the presence/absence of\n> \"Notes:\".\nOk, that's a valid concern. If we want to assert that this doesn't look\nlike a patch we need to at least parse the notes and quote it somehow.\nHmm.\n\nBest regards\nUwe\n\n-- \nPengutronix e.K. | Uwe Kleine-König |\nIndustrial Linux Solutions | http://www.pengutronix.de/ |\n"}]}