{"thread":{"id":"22298","subject":"git notes: notes","startedAt":"2010-01-20T05:03:43Z","lastAt":"2010-01-27T20:01:22Z","messageCount":36,"participants":["Joey Hess","Thomas Rast","Johan Herland","Junio C Hamano","Jeff King","Michael J Gruber","Johannes Schindelin","Matthieu Moy","Sverre Rabbelier","John Koleszar","Christian Couder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"132164","messageId":"20100120050343.GA12860@gnu.kitenet.net","threadId":"22298","inReplyTo":null,"subject":"git notes: notes","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2010-01-20T05:03:43Z","receivedAt":"2010-01-20T05:03:43Z","isPatch":false,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Just a quick note that the new notes feature can break things that parse\ngit log. For example a parser that assumes it can split the log on blank\nlines to separate the header and commit message, can easily become\nconfused by the new blank line before \"Notes:\".\n\nMight be worth documenting in release notes, maybe too late now though.\nBut really, it's all good, notes are a great feature.\n\nPS, Has anyone thought about using notes to warn bisect away from\ncommits that are known to be unbuildable or otherwise cause bisection\ntrouble?\n\n-- \nsee shy jo\n"},{"id":"132177","messageId":"201001201049.01108.trast@student.ethz.ch","threadId":"22298","inReplyTo":"20100120050343.GA12860@gnu.kitenet.net","subject":"Re: git notes: notes","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2010-01-20T09:48:58Z","receivedAt":"2010-01-20T09:48:58Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Joey Hess wrote:\n> Just a quick note that the new notes feature can break things that parse\n> git log.\n\nUmm. git-log is porcelain and we're allowed to change it. Worse,\neven the user can change it in very significant ways, just try:\n\n git config format.pretty email\n git log\n\nFor a better alternative, I'm afraid you'll either have to look to\ngit-rev-list (which also takes --pretty) or 'git cat-file --batch'.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"132185","messageId":"201001201148.11701.johan@herland.net","threadId":"22298","inReplyTo":"20100120050343.GA12860@gnu.kitenet.net","subject":"Re: git notes: notes","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2010-01-20T10:48:11Z","receivedAt":"2010-01-20T10:48:11Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wednesday 20 January 2010, Joey Hess wrote:\n> Just a quick note that the new notes feature can break things that parse\n> git log. For example a parser that assumes it can split the log on blank\n> lines to separate the header and commit message, can easily become\n> confused by the new blank line before \"Notes:\".\n\nAs Thomas already stated, git log is porcelain, and its output format is not \nset in stone. If you need a stable, script-friendly format, you should \nprobably use the --format option, or use plumbing instead (such as e.g. git \nrev-list, which also has a --format option).\n\n> Might be worth documenting in release notes, maybe too late now though.\n> But really, it's all good, notes are a great feature.\n> \n> PS, Has anyone thought about using notes to warn bisect away from\n> commits that are known to be unbuildable or otherwise cause bisection\n> trouble?\n\nNo, I haven't thought of that specific use case. Great idea! :)\n\nBTW, since I started talking about git notes, people on this list have found \nmore and more interesting use cases for them:\n\n- Free-form text extension to the commit message\n\n- Help in bug tracking with header-like lines such as:\n - Causes-Bug: #12345\n - Fixes-Bug: #54321\n\n- Store after-the-fact \"Acked-By\", \"Reviewed-By\", etc. annotations\n\n- In a repo converted from a merge-unfriendly VCS (such as CVS), use notes\n to identify merges without having to rewrite Git history (note that you\n can also use grafts, or \"git replace\" to accomplish this).\n\n- Refer to related commits elsewhere in the repo (i.e. relationships that\n are not already apparent from the commit graph)\n\n- When cherry-picking, add a reverse link from the source commit to the\n cherry-picked commit (since it may be of interest to people reviewing the\n source commit\n\n- Rebasing public branches is forbidden, but if you wanted to change that,\n you could potentially help solve it by using notes to add reverse links\n from source commits to rebased commits, so that downstream people could\n more easily traverse your history when rebasing/merging their own\n branches.\n\n- Initially, there were some discussion whether it could also be used to\n guide git blame to make better decisions, although I don't currently see\n how that would be done in practice.\n\nIn any case, it seems the notes idea may have the potential to become one of \nthe more useful features in Git.\n\n\nHave fun! :)\n\n...Johan\n\n\n[1]: ...almost 3 years ago (wow, time flies...):\n http://article.gmane.org/gmane.comp.version-control.git/46883\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"132203","messageId":"20100120181456.GA31507@gnu.kitenet.net","threadId":"22298","inReplyTo":"201001201049.01108.trast@student.ethz.ch","subject":"Re: git notes: notes","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2010-01-20T18:14:57Z","receivedAt":"2010-01-20T18:14:57Z","isPatch":false,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Thomas Rast wrote:\n> Umm. git-log is porcelain and we're allowed to change it. Worse,\n> even the user can change it in very significant ways, just try:\n> \n> git config format.pretty email\n> git log\n\nIs git log --pretty=raw --raw really intended to be porcelain?\nAbove does not affect it.\n\n> For a better alternative, I'm afraid you'll either have to look to\n> git-rev-list (which also takes --pretty) or 'git cat-file --batch'.\n\nI don't see a way to get the per-commit diff-tree info using rev-list.\n\n-- \nsee shy jo\n"},{"id":"132205","messageId":"20100120182438.GB31507@gnu.kitenet.net","threadId":"22298","inReplyTo":"201001201148.11701.johan@herland.net","subject":"Re: git notes: notes","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2010-01-20T18:24:38Z","receivedAt":"2010-01-20T18:24:38Z","isPatch":false,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Johan Herland wrote:\n> As Thomas already stated, git log is porcelain, and its output format is not \n> set in stone. If you need a stable, script-friendly format, you should \n> probably use the --format option, or use plumbing instead (such as e.g. git \n> rev-list, which also has a --format option).\n\nBut git log --format=raw --raw output was changed by notes.\n\n> > Might be worth documenting in release notes, maybe too late now though.\n> > But really, it's all good, notes are a great feature.\n> > \n> > PS, Has anyone thought about using notes to warn bisect away from\n> > commits that are known to be unbuildable or otherwise cause bisection\n> > trouble?\n> \n> No, I haven't thought of that specific use case. Great idea! :)\n\nOnly problem I see with doing it is it might be too easy to overwrite\nsuch a note with git notes edit -m\n\nDid you consider having -m append a line to an existing note?\n\n-- \nsee shy jo\n"},{"id":"132207","messageId":"7vhbqg376b.fsf@alter.siamese.dyndns.org","threadId":"22298","inReplyTo":"20100120182438.GB31507@gnu.kitenet.net","subject":"Re: git notes: notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-20T19:30:52Z","receivedAt":"2010-01-20T19:30:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joey Hess writes:\n\n> Johan Herland wrote:\n>> As Thomas already stated, git log is porcelain, and its output format is not \n>> set in stone. If you need a stable, script-friendly format, you should \n>> probably use the --format option, or use plumbing instead (such as e.g. git \n>> rev-list, which also has a --format option).\n>\n> But git log --format=raw --raw output was changed by notes.\n\nLet me ask a stupid question. Did the output change before and after the\nnotes code even when your history does not have notes?\n\n>> > Might be worth documenting in release notes, maybe too late now though.\n\nThis depends on the answer to the above question. If the answers is \"No\",\nthen I don't see the need to say much more than \"New 'git notes' feature\nallows comments applied to existing commits after the fact to be shown by\nlog and friends\". If it is \"Yes\", we should fix the code not to change\nthe output.\n\nIn any case, \"log\" is still a Porcelain, so it is understandable that by\ntriggering a new feature you would get output from the new feature. It is\ncalled progress ;-)\n"},{"id":"132210","messageId":"20100120195626.GA6641@gnu.kitenet.net","threadId":"22298","inReplyTo":"7vhbqg376b.fsf@alter.siamese.dyndns.org","subject":"Re: git notes: notes","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2010-01-20T19:56:26Z","receivedAt":"2010-01-20T19:56:26Z","isPatch":false,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Junio C Hamano wrote:\n> Let me ask a stupid question. Did the output change before and after the\n> notes code even when your history does not have notes?\n\nNo. \n\n> >> > Might be worth documenting in release notes, maybe too late now though.\n> \n> This depends on the answer to the above question. If the answers is \"No\",\n> then I don't see the need to say much more than \"New 'git notes' feature\n> allows comments applied to existing commits after the fact to be shown by\n> log and friends\". If it is \"Yes\", we should fix the code not to change\n> the output.\n> \n> In any case, \"log\" is still a Porcelain, so it is understandable that by\n> triggering a new feature you would get output from the new feature. It is\n> called progress ;-)\n\nDo you think it makes sense for even git log --format=format:%s to be\nporcelain and potentially change when new features are used? Seems to\nme that parts of git log walk the line between porcelain and plumbing.\nSo it's not clear which parts are safe to use.\n\n-- \nsee shy jo\n"},{"id":"132211","messageId":"7vska01qrt.fsf@alter.siamese.dyndns.org","threadId":"22298","inReplyTo":"20100120195626.GA6641@gnu.kitenet.net","subject":"Re: git notes: notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-20T20:10:30Z","receivedAt":"2010-01-20T20:10:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joey Hess writes:\n\n> Do you think it makes sense for even git log --format=format:%s to be\n> porcelain and potentially change when new features are used?\n\nIf the series changed the meaning of \"%s\" format to mean \"the subject of\nthe commit and notes information\", with or without documenting it, then it\nis just a bug we would like to fix.\n\nBut I cannot reproduce such a bug. In my tree locally:\n\n $ git notes show a97a74\n Origin of commit notes feature was at\n a97a74).\n $ git show -s --pretty=short a97a74\n commit a97a74686d70a318cd802003498054cc1e8b0ae2\n Author: Johannes Schindelin \n\n Introduce commit notes\n\n Notes:\n Origin of commit notes feature was at\n a97a74).\n $ git show -s --pretty=format:%s a97a74\n Introduce commit notes\n\nPuzzled...\n"},{"id":"132219","messageId":"20100120203636.GA9221@gnu.kitenet.net","threadId":"22298","inReplyTo":"7vska01qrt.fsf@alter.siamese.dyndns.org","subject":"Re: git notes: notes","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2010-01-20T20:36:36Z","receivedAt":"2010-01-20T20:36:36Z","isPatch":false,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Junio C Hamano wrote:\n> Joey Hess writes:\n> \n> > Do you think it makes sense for even git log --format=format:%s to be\n> > porcelain and potentially change when new features are used?\n> \n> If the series changed the meaning of \"%s\" format to mean \"the subject of\n> the commit and notes information\", with or without documenting it, then it\n> is just a bug we would like to fix.\n> \n> But I cannot reproduce such a bug. In my tree locally:\n\nI was asking hypothetically, trying to point out that parts of git log\nseem to make sense to be used as plumbing, with the hope I can continue\nto use it that way.\n\n(Note that git instaweb parses output of git log --pretty=format:%H --raw\nlike it's plumbing.)\n\n-- \nsee shy jo\n"},{"id":"132220","messageId":"20100120205452.GA8843@coredump.intra.peff.net","threadId":"22298","inReplyTo":"20100120203636.GA9221@gnu.kitenet.net","subject":"Re: git notes: notes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-20T20:54:52Z","receivedAt":"2010-01-20T20:54:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 20, 2010 at 03:36:36PM -0500, Joey Hess wrote:\n\n> I was asking hypothetically, trying to point out that parts of git log\n> seem to make sense to be used as plumbing, with the hope I can continue\n> to use it that way.\n> \n> (Note that git instaweb parses output of git log --pretty=format:%H --raw\n> like it's plumbing.)\n\nI think this is a valid point. Note that \"gitk\" uses \"git log\n--pretty=raw\". However, I believe it splits the entries on \"^commit\". So\nI think there is some precedent for scripting \"git log\"; it has features\nthat are simply not available through other interfaces. And scripting\naround \"--pretty=raw\" seems pretty reasonable to me, too. Why else would\nyou want the raw format?\n\nIs splitting on blank lines an error? I don't think so. The original\nformat was never strictly defined, but given the --pretty=raw format, it\nseems like a fairly obvious thing to do.\n\nI am inclined to cut the notes output from --pretty=raw, and let callers\nask for them explicitly with --show-notes or something similar. We can\nleave them on by default in the \"normal\" output. This will still break\nscripts doing \"git log | ./script\", but I don't think we have ever\ncondoned that practice.\n\n-Peff\n"},{"id":"132223","messageId":"7viqaw1ohx.fsf@alter.siamese.dyndns.org","threadId":"22298","inReplyTo":"20100120205452.GA8843@coredump.intra.peff.net","subject":"Re: git notes: notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-20T20:59:38Z","receivedAt":"2010-01-20T20:59:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Is splitting on blank lines an error? I don't think so. The original\n> format was never strictly defined, but given the --pretty=raw format, it\n> seems like a fairly obvious thing to do.\n>\n> I am inclined to cut the notes output from --pretty=raw, and let callers\n> ask for them explicitly with --show-notes or something similar. We can\n> leave them on by default in the \"normal\" output. This will still break\n> scripts doing \"git log | ./script\", but I don't think we have ever\n> condoned that practice.\n\nSounds like a plan.\n"},{"id":"132224","messageId":"4B576F5C.2050102@drmicha.warpmail.net","threadId":"22298","inReplyTo":"7vska01qrt.fsf@alter.siamese.dyndns.org","subject":"Re: git notes: notes","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-01-20T21:02:20Z","receivedAt":"2010-01-20T21:02:20Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 20.01.2010 21:10:\n> Joey Hess writes:\n> \n>> Do you think it makes sense for even git log --format=format:%s to be\n>> porcelain and potentially change when new features are used?\n> \n> If the series changed the meaning of \"%s\" format to mean \"the subject of\n> the commit and notes information\", with or without documenting it, then it\n> is just a bug we would like to fix.\n\nNo, but outputting the note as part of the log is the standard. So for\nexample, when you do a format-patch | apply cycle, format-patch will\ninsert the note as part of the commit message, and apply will *store*\nthe note text (including Note:\\n) as part of the commit message of the\nnew commit.\n\nSo, I would say the notes feature is not that well integrated right now,\nand either log has to learn --no-notes (and format-patch has to use it,\nor rather the corresponding internal flag), or apply has to learn to\nparse \"Note:\" headers. Or, depending on how you use notes, it may be\nbetter if format-patch puts the note after the \"--\"; that way you can\nstore the usual \"after-the-message\" patch comments in a note.\n\nSimilarly, I don't think rebasing and cherry-picking adjust the notes\ntree for commits with notes whose sha1 changes - which may or may not be\nthe appropriate behaviour.\n\nIn both cases, the \"right\" way depends on how you use notes, and there\nshould be an easy way to specify your choice.\n\nI'm not complaining, I actually have this on a maybe-to-do list, but the\nway the series went kept me from investing time.\n\nCheers,\nMichael\n"},{"id":"132226","messageId":"7veilk1o3s.fsf@alter.siamese.dyndns.org","threadId":"22298","inReplyTo":"4B576F5C.2050102@drmicha.warpmail.net","subject":"Re: git notes: notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-20T21:08:07Z","receivedAt":"2010-01-20T21:08:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Junio C Hamano venit, vidit, dixit 20.01.2010 21:10:\n>> Joey Hess writes:\n>> \n>>> Do you think it makes sense for even git log --format=format:%s to be\n>>> porcelain and potentially change when new features are used?\n>> \n>> If the series changed the meaning of \"%s\" format to mean \"the subject of\n>> the commit and notes information\", with or without documenting it, then it\n>> is just a bug we would like to fix.\n>\n> No, but outputting the note as part of the log is the standard. So for\n> example, when you do a format-patch | apply cycle, format-patch will\n> insert the note as part of the commit message, and apply will *store*\n> the note text (including Note:\\n) as part of the commit message of the\n> new commit.\n\nThanks; that was the kind of breakage report I was looking for (and wished\nto have heard a lot earlier). Personally I find it is unexcusable that\nformat-patch defaults to giving notes.\n\n> So, I would say the notes feature is not that well integrated right now,\n\nNo question about it.\n\n> I'm not complaining, I actually have this on a maybe-to-do list, but the\n> way the series went kept me from investing time.\n\nHmm, that hints there is a failure in the review and merge process. Care\nto explain how we could have done better please?\n"},{"id":"132229","messageId":"20100120213137.GA9107@coredump.intra.peff.net","threadId":"22298","inReplyTo":"7viqaw1ohx.fsf@alter.siamese.dyndns.org","subject":"Re: git notes: notes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-20T21:31:38Z","receivedAt":"2010-01-20T21:31:38Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 20, 2010 at 12:59:38PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Is splitting on blank lines an error? I don't think so. The original\n> > format was never strictly defined, but given the --pretty=raw format, it\n> > seems like a fairly obvious thing to do.\n> >\n> > I am inclined to cut the notes output from --pretty=raw, and let callers\n> > ask for them explicitly with --show-notes or something similar. We can\n> > leave them on by default in the \"normal\" output. This will still break\n> > scripts doing \"git log | ./script\", but I don't think we have ever\n> > condoned that practice.\n> \n> Sounds like a plan.\n\nWe can start with this patch, which clears up Joey's problem.\n\n-- >8 --\nSubject: [PATCH] don't show notes for --pretty=raw\n\nThe --pretty=raw format of the log family is likely to be\nused by scripts. Such scripts may parse the output into\nrecords on blank lines, since doing so in the past has\nalways worked. However, with the recently added notes\noutput, such parsers will see an extra stanza for any\ncommits that have notes.\n\nThis patch turns off the notes output for the raw format to\navoid breaking such scripts.\n\nSigned-off-by: Jeff King \n---\n pretty.c | 2 +-\n t/t3301-notes.sh | 14 ++++++++++++++\n 2 files changed, 15 insertions(+), 1 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 9001379..0674027 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1094,7 +1094,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,\n \tif (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)\n \t\tstrbuf_addch(sb, '\\n');\n \n-\tif (fmt != CMIT_FMT_ONELINE)\n+\tif (fmt != CMIT_FMT_ONELINE && fmt != CMIT_FMT_RAW)\n \t\tget_commit_notes(commit, sb, encoding,\n \t\t\t\t NOTES_SHOW_HEADER | NOTES_INDENT);\n \ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 1e34f48..4c3de9d 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -147,4 +147,18 @@ test_expect_success 'show -m and -F notes' '\n \ttest_cmp expect-m-and-F output\n '\n \n+cat >expect << EOF\n+commit 15023535574ded8b1a89052b32673f84cf9582b8\n+tree e070e3af51011e47b183c33adf9736736a525709\n+parent 1584215f1d29c65e99c6c6848626553fdd07fd75\n+author A U Thor 1112912173 -0700\n+committer C O Mitter 1112912173 -0700\n+\n+ 4th\n+EOF\n+test_expect_success 'git log --pretty=raw does not show notes' '\n+\tgit log -1 --pretty=raw >output &&\n+\ttest_cmp expect output\n+'\n+\n test_done\n-- \n1.6.6.510.g159cf\n"},{"id":"132230","messageId":"20100120213631.GB9107@coredump.intra.peff.net","threadId":"22298","inReplyTo":"7veilk1o3s.fsf@alter.siamese.dyndns.org","subject":"Re: git notes: notes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-20T21:36:31Z","receivedAt":"2010-01-20T21:36:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 20, 2010 at 01:08:07PM -0800, Junio C Hamano wrote:\n\n> > No, but outputting the note as part of the log is the standard. So for\n> > example, when you do a format-patch | apply cycle, format-patch will\n> > insert the note as part of the commit message, and apply will *store*\n> > the note text (including Note:\\n) as part of the commit message of the\n> > new commit.\n> \n> Thanks; that was the kind of breakage report I was looking for (and wished\n> to have heard a lot earlier). Personally I find it is unexcusable that\n> format-patch defaults to giving notes.\n\nI agree. I noticed this while doing the \"don't show in raw\" feature\nelsewhere in the thread and wanted to ask: which formats _should_ have\nnotes by default?\n\nTo be honest, I am not sure _any_ format should have it by default. If I\nam running \"git log\" and my notes are filled with random automatically\ngenerated bisection cruft, I don't want to see that cluttering my\noutput. Yes, all of our test notes are human-written annotations, but I\nthink we really don't know yet what sorts of things people will be\nputting in them.\n\nLong ago I proposed a set of notes namespaces to deal with this (so\nautomatic bisection cruft would go into its own notes namespace, and\nhuman-readable ones would be in some default namespace), but I don't\nknow how much of that idea (if any) survived into the current\nimplementation.\n\n> > I'm not complaining, I actually have this on a maybe-to-do list, but the\n> > way the series went kept me from investing time.\n> \n> Hmm, that hints there is a failure in the review and merge process. Care\n> to explain how we could have done better please?\n\nPersonally, I stopped paying attention simply because it was gigantic\nand I am not all that interested in using the feature personally.\n\n-Peff\n"},{"id":"132231","messageId":"20100120214152.GC9107@coredump.intra.peff.net","threadId":"22298","inReplyTo":"20100120213137.GA9107@coredump.intra.peff.net","subject":"Re: git notes: notes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-20T21:41:52Z","receivedAt":"2010-01-20T21:41:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 20, 2010 at 04:31:38PM -0500, Jeff King wrote:\n\n> We can start with this patch, which clears up Joey's problem.\n> \n> -- >8 --\n> Subject: [PATCH] don't show notes for --pretty=raw\n> \n> The --pretty=raw format of the log family is likely to be\n> used by scripts. Such scripts may parse the output into\n> records on blank lines, since doing so in the past has\n> always worked. However, with the recently added notes\n> output, such parsers will see an extra stanza for any\n> commits that have notes.\n> \n> This patch turns off the notes output for the raw format to\n> avoid breaking such scripts.\n\nAnd the second half would be something like the patch below implementing\n--show-notes, so that things like \"gitk\" which can handle the new\nfeature can turn it on.\n\nI'm not that happy with this patch, though. If we have --show-notes, we\nshould probably have --no-show-notes, which this doesn't do, since a\nlack of --show-notes simply means \"do whatever the format dictates\".\n\nAlso, passing everything through a pretty_print_context seems kind of\nhack-ish, as we just end up copying items from the rev-list context into\nthe pp_context. I did so here only in the log_tree_commit case. I don't\nthink there are other places which would want to propagate it, but I\nmight have missed one. I wonder if we would do better to simply past the\nrev-list options into pretty_print_commit and let it pick what it wants\nout of the struct.\n\nAnyway, I am out of time to work on git for now, so maybe somebody who\ncares more about notes can pick this up. I think the first patch (the\none I am replying to) should definitely go in to un-break Joey's case\n(or an alternative patch that turns it off in even more cases), and\npeople who care about it can fight about whether and by what mechanism\nthings like gitk should see the notes.\n\n-- >8 --\nSubject: [PATCH] teach log family to --show-notes\n\nMost log formats will implicitly show commit notes, but the\nraw format will not. Callers can now explicitly call\n--show-notes to enable them.\n\nSigned-off-by: Jeff King \n---\n commit.h | 1 +\n log-tree.c | 1 +\n pretty.c | 3 ++-\n revision.c | 2 ++\n revision.h | 1 +\n t/t3301-notes.sh | 16 ++++++++++++++++\n 6 files changed, 23 insertions(+), 1 deletions(-)\n\ndiff --git a/commit.h b/commit.h\nindex 24128d7..4646751 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -71,6 +71,7 @@ struct pretty_print_context\n \tenum date_mode date_mode;\n \tint need_8bit_cte;\n \tstruct reflog_walk_info *reflog_info;\n+\tint show_notes;\n };\n \n extern int has_non_ascii(const char *text);\ndiff --git a/log-tree.c b/log-tree.c\nindex 0fdf159..9155a31 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -412,6 +412,7 @@ void show_log(struct rev_info *opt)\n \tctx.abbrev = opt->diffopt.abbrev;\n \tctx.after_subject = extra_headers;\n \tctx.reflog_info = opt->reflog_info;\n+\tctx.show_notes = opt->show_notes;\n \tpretty_print_commit(opt->commit_format, commit, &msgbuf, &ctx);\n \n \tif (opt->add_signoff)\ndiff --git a/pretty.c b/pretty.c\nindex 0674027..95fe39a 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1094,7 +1094,8 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,\n \tif (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)\n \t\tstrbuf_addch(sb, '\\n');\n \n-\tif (fmt != CMIT_FMT_ONELINE && fmt != CMIT_FMT_RAW)\n+\tif (context->show_notes ||\n+\t (fmt != CMIT_FMT_ONELINE && fmt != CMIT_FMT_RAW))\n \t\tget_commit_notes(commit, sb, encoding,\n \t\t\t\t NOTES_SHOW_HEADER | NOTES_INDENT);\n \ndiff --git a/revision.c b/revision.c\nindex 7328201..3815dd3 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1175,6 +1175,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->verbose_header = 1;\n \t\tget_commit_format(\"oneline\", revs);\n \t\trevs->abbrev_commit = 1;\n+\t} else if (!strcmp(arg, \"--show-notes\")) {\n+\t\trevs->show_notes = 1;\n \t} else if (!strcmp(arg, \"--graph\")) {\n \t\trevs->topo_order = 1;\n \t\trevs->rewrite_parents = 1;\ndiff --git a/revision.h b/revision.h\nindex d368003..e51842f 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -83,6 +83,7 @@ struct rev_info {\n \t\t\tabbrev_commit:1,\n \t\t\tuse_terminator:1,\n \t\t\tmissing_newline:1,\n+\t\t\tshow_notes:1,\n \t\t\tdate_mode_explicit:1;\n \tunsigned int\tdisable_stdin:1;\n \ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 4c3de9d..d51e8bd 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -161,4 +161,20 @@ test_expect_success 'git log --pretty=raw does not show notes' '\n \ttest_cmp expect output\n '\n \n+cat >>expect <output &&\n+\ttest_cmp expect output\n+'\n+\n test_done\n-- \n1.6.6.510.g159cf\n"},{"id":"132234","messageId":"7v3a201lpz.fsf@alter.siamese.dyndns.org","threadId":"22298","inReplyTo":"7veilk1o3s.fsf@alter.siamese.dyndns.org","subject":"Re: git notes: notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-20T21:59:36Z","receivedAt":"2010-01-20T21:59:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Michael J Gruber writes:\n>\n>> No, but outputting the note as part of the log is the standard. So for\n>> example, when you do a format-patch | apply cycle, format-patch will\n>> insert the note as part of the commit message, and apply will *store*\n>> the note text (including Note:\\n) as part of the commit message of the\n>> new commit.\n>\n> Thanks; that was the kind of breakage report I was looking for (and wished\n> to have heard a lot earlier). Personally I find it is unexcusable that\n> format-patch defaults to giving notes.\n>\n>> So, I would say the notes feature is not that well integrated right now,\n>\n> No question about it.\n\nHow about solving it this way?\n\nIt _could_ break some tests, if the set of tests were carefully written to\ncover not only the positive (\"I am showing off my shiny new toy\") cases\nbut also the negative (\"These commands share the same codepath touched by\nthe series, but I don't intend to change their behaviour, and here is to\nmake sure the new toy does not affect them\") cases and the latter set\nassumed it is ok to sprinkle notes in commit log messages without being\nasked, but I haven't tried running the test suite yet.\n\n---\nSubject: Fix \"log\" family not to be too agressive about showing notes\n\nGiving \"Notes\" information in the default output format of \"log\" and\n\"show\" is a sensible progress (the user has asked for it by having the\nnotes), but for some commands (e.g. \"format-patch\") spewing notes into the\nformatted commit log message without being asked is too aggressive.\n\nEnable notes output only for \"log\", \"show\", \"whatchanged\" by default;\nother users can ask for it by setting show_notes field to true.\n\nSigned-off-by: Junio C Hamano \n---\n builtin-log.c | 2 ++\n commit.h | 1 +\n log-tree.c | 1 +\n pretty.c | 2 +-\n revision.c | 4 ++++\n revision.h | 1 +\n 6 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 41b6df4..da0ba1d 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -41,6 +41,8 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \trev->commit_format = CMIT_FMT_DEFAULT;\n \tif (fmt_pretty)\n \t\tget_commit_format(fmt_pretty, rev);\n+\telse\n+\t\trev->show_notes = 1;\n \trev->verbose_header = 1;\n \tDIFF_OPT_SET(&rev->diffopt, RECURSIVE);\n \trev->show_root_diff = default_show_root;\ndiff --git a/commit.h b/commit.h\nindex e5332ef..2c0742b 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -70,6 +70,7 @@ struct pretty_print_context\n \tconst char *after_subject;\n \tenum date_mode date_mode;\n \tint need_8bit_cte;\n+\tint show_notes;\n \tstruct reflog_walk_info *reflog_info;\n };\n \ndiff --git a/log-tree.c b/log-tree.c\nindex 0fdf159..27afcf6 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -284,6 +284,7 @@ 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 \ndiff --git a/pretty.c b/pretty.c\nindex 8f5bd1a..b2ee7fe 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1094,7 +1094,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,\n \tif (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)\n \t\tstrbuf_addch(sb, '\\n');\n \n-\tif (fmt != CMIT_FMT_ONELINE)\n+\tif (context->show_notes)\n \t\tget_commit_notes(commit, sb, encoding,\n \t\t\t\t NOTES_SHOW_HEADER | NOTES_INDENT);\n \ndiff --git a/revision.c b/revision.c\nindex 25fa14d..03c280f 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1165,6 +1165,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!prefixcmp(arg, \"--pretty=\") || !prefixcmp(arg, \"--format=\")) {\n \t\trevs->verbose_header = 1;\n \t\tget_commit_format(arg+9, revs);\n+\t} else if (!strcmp(arg, \"--show-notes\")) {\n+\t\trevs->show_notes = 1;\n+\t} else if (!strcmp(arg, \"--no-notes\")) {\n+\t\trevs->show_notes = 0;\n \t} else if (!strcmp(arg, \"--oneline\")) {\n \t\trevs->verbose_header = 1;\n \t\tget_commit_format(\"oneline\", revs);\ndiff --git a/revision.h b/revision.h\nindex d368003..4167c1e 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -80,6 +80,7 @@ struct rev_info {\n \t/* Format info */\n \tunsigned int\tshown_one:1,\n \t\t\tshow_merge:1,\n+\t\t\tshow_notes:1,\n \t\t\tabbrev_commit:1,\n \t\t\tuse_terminator:1,\n \t\t\tmissing_newline:1,\n"},{"id":"132235","messageId":"7vy6jszaz0.fsf@alter.siamese.dyndns.org","threadId":"22298","inReplyTo":"20100120214152.GC9107@coredump.intra.peff.net","subject":"Re: git notes: notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-20T22:07:47Z","receivedAt":"2010-01-20T22:07:47Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> diff --git a/pretty.c b/pretty.c\n> index 0674027..95fe39a 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -1094,7 +1094,8 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,\n> \tif (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)\n> \t\tstrbuf_addch(sb, '\\n');\n> \n> -\tif (fmt != CMIT_FMT_ONELINE && fmt != CMIT_FMT_RAW)\n> +\tif (context->show_notes ||\n> +\t (fmt != CMIT_FMT_ONELINE && fmt != CMIT_FMT_RAW))\n> \t\tget_commit_notes(commit, sb, encoding,\n> \t\t\t\t NOTES_SHOW_HEADER | NOTES_INDENT);\n\nHeh, without this hunk I would have thought Peff and Gitster were the same\nperson ;-).\n\nOnce you introduce --no-notes, the above condition would not work well.\n"},{"id":"132236","messageId":"20100120222137.GC15936@coredump.intra.peff.net","threadId":"22298","inReplyTo":"7vy6jszaz0.fsf@alter.siamese.dyndns.org","subject":"Re: git notes: notes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-20T22:21:37Z","receivedAt":"2010-01-20T22:21:37Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 20, 2010 at 02:07:47PM -0800, Junio C Hamano wrote:\n\n> > -\tif (fmt != CMIT_FMT_ONELINE && fmt != CMIT_FMT_RAW)\n> > +\tif (context->show_notes ||\n> > +\t (fmt != CMIT_FMT_ONELINE && fmt != CMIT_FMT_RAW))\n> > \t\tget_commit_notes(commit, sb, encoding,\n> > \t\t\t\t NOTES_SHOW_HEADER | NOTES_INDENT);\n> \n> Heh, without this hunk I would have thought Peff and Gitster were the same\n> person ;-).\n> \n> Once you introduce --no-notes, the above condition would not work well.\n\nYeah, I know, or I would have just added the 2 lines for\n--no-show-notes. :) I think your patch is better; I'll comment on it\nseparately.\n\n-Peff\n"},{"id":"132238","messageId":"20100120222548.GD15936@coredump.intra.peff.net","threadId":"22298","inReplyTo":"7v3a201lpz.fsf@alter.siamese.dyndns.org","subject":"Re: git notes: notes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-20T22:25:48Z","receivedAt":"2010-01-20T22:25:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 20, 2010 at 01:59:36PM -0800, Junio C Hamano wrote:\n\n> Subject: Fix \"log\" family not to be too agressive about showing notes\n> \n> Giving \"Notes\" information in the default output format of \"log\" and\n> \"show\" is a sensible progress (the user has asked for it by having the\n> notes), but for some commands (e.g. \"format-patch\") spewing notes into the\n> formatted commit log message without being asked is too aggressive.\n> \n> Enable notes output only for \"log\", \"show\", \"whatchanged\" by default;\n> other users can ask for it by setting show_notes field to true.\n\nWhat I didn't get out of reading this but did from reading the code (I\nthink) is what you meant by \"by default\" here. That is, doing:\n\n git log\n\nwill show notes, but neither\n\n git log --pretty=raw\n\nnor even\n\n git log --pretty=medium\n\nwill do so, even though the latter otherwise produces identical output\nto the default.\n\nThat seems like a reasonable rule to me, but I just wanted to make sure\nthat was both what was happening and what was intended.\n\n> Signed-off-by: Junio C Hamano \n> ---\n> builtin-log.c | 2 ++\n> commit.h | 1 +\n> log-tree.c | 1 +\n> pretty.c | 2 +-\n> revision.c | 4 ++++\n> revision.h | 1 +\n> 6 files changed, 10 insertions(+), 1 deletions(-)\n\nNo tests or docs, of course. :) You can squash the --pretty=raw test\nfrom my patch, but you will need to exercise --show-notes and\n--no-show-notes, too, as well as checking other formats and things like\nformat-patch. So probably writing your own tests will make it easier to\nmore thoroughly check each case.\n\n-Peff\n"},{"id":"132239","messageId":"7vpr54z9rn.fsf@alter.siamese.dyndns.org","threadId":"22298","inReplyTo":"20100120222548.GD15936@coredump.intra.peff.net","subject":"Re: git notes: notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-20T22:33:48Z","receivedAt":"2010-01-20T22:33:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> No tests or docs, of course. :) You can squash the --pretty=raw test\n> from my patch, but you will need to exercise --show-notes and\n> --no-show-notes, too, as well as checking other formats and things like\n> format-patch. So probably writing your own tests will make it easier to\n> more thoroughly check each case.\n\nThanks, but Ugh.\n\nDidn't I say elsewhere that I am too busy to become a janitor for\neverybody's itch, especially for topics that are merely \"Meh\" to me?\n\nIf there is no volunteer, I might be forced to do something about it, but\nno promises, and I am reasonably certain that not much will happen at my\nend for coming 48 hours, as I am cutting 1.6.6.1 (and perhaps 1.6.5.8) and\nlooking at other topics in 'next' that deserve to go to 1.7.0-rc0.\n"},{"id":"132241","messageId":"alpine.DEB.1.00.1001202354070.4985@pacific.mpi-cbg.de","threadId":"22298","inReplyTo":"7v3a201lpz.fsf@alter.siamese.dyndns.org","subject":"Re: git notes: notes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-01-20T22:58:07Z","receivedAt":"2010-01-20T22:58:07Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 20 Jan 2010, Junio C Hamano wrote:\n\n> Subject: Fix \"log\" family not to be too agressive about showing notes\n> \n> Giving \"Notes\" information in the default output format of \"log\" and\n> \"show\" is a sensible progress (the user has asked for it by having the\n> notes), but for some commands (e.g. \"format-patch\") spewing notes into the\n> formatted commit log message without being asked is too aggressive.\n> \n> Enable notes output only for \"log\", \"show\", \"whatchanged\" by default;\n> other users can ask for it by setting show_notes field to true.\n> \n> Signed-off-by: Junio C Hamano \n> ---\n\nMakes sense, and the patch actually removes what could be seen as an ugly \nside effect (why it only ONELINE not getting notes?).\n\nI would agree with Peff about the mention of --pretty disabling notes \n(unless asked for by a user format) in the commit notes as well as in the \npretty options, but I fully disagree on the need for tests. We should not \nhave a thorough test suite that runs for days on end, but we should \nconcentrate on things that are more likely to get broken. And the added \ncode is just too obvious for that.\n\n(Anybody remember the initial suggestion for testing git-commit before \nmaking it builtin? It had something like 70 tests.)\n\nCiao,\nDscho\n"},{"id":"132243","messageId":"20100120230618.GC25051@coredump.intra.peff.net","threadId":"22298","inReplyTo":"alpine.DEB.1.00.1001202354070.4985@pacific.mpi-cbg.de","subject":"Re: git notes: notes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-20T23:06:18Z","receivedAt":"2010-01-20T23:06:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 20, 2010 at 11:58:07PM +0100, Johannes Schindelin wrote:\n\n> I would agree with Peff about the mention of --pretty disabling notes \n> (unless asked for by a user format) in the commit notes as well as in the \n> pretty options, but I fully disagree on the need for tests. We should not \n> have a thorough test suite that runs for days on end, but we should \n> concentrate on things that are more likely to get broken. And the added \n> code is just too obvious for that.\n\nSure, we don't have to go all out. But I think there is some confusion\nright now about just what behavior we _should_ have, so I think\ndocumenting it in the form of tests is reasonable.\n\n-Peff\n"},{"id":"132244","messageId":"7vljfsz7vx.fsf@alter.siamese.dyndns.org","threadId":"22298","inReplyTo":"alpine.DEB.1.00.1001202354070.4985@pacific.mpi-cbg.de","subject":"Re: git notes: notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-20T23:14:26Z","receivedAt":"2010-01-20T23:14:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Makes sense, and the patch actually removes what could be seen as an ugly \n> side effect (why it only ONELINE not getting notes?).\n\nThanks.\n\nThe motivation was that the user should be able to get notes even under\nONELINE mode if desired. But then the call to get_commit_notes() may want\nto inspect the commit format being used and tweak the flag parameter; right\nnow it always sends NOTES_SHOW_HEADER and NOTES_INDENT.\n\n> I would agree with Peff about the mention of --pretty disabling notes \n> (unless asked for by a user format) in the commit notes as well as in the \n> pretty options,...\n\nActually I am of two minds regarding --pretty={short,medium} and the\nlike. The \"how about this\" patch may be the safest for people who are\nused to read \"log --pretty=xxx\" output with scripts, but it does look\ninconsistent and hard to explain to new people who do not even know that\nthere were versions of git that does not know about notes.\n\n> but I fully disagree on the need for tests. We should not \n> have a thorough test suite that runs for days on end, but we should \n> concentrate on things that are more likely to get broken. And the added \n> code is just too obvious for that.\n\nI agree with that principle, but it doesn't explain nor justify the lack\nof tests for format-patch, which would have caught the breakage a lot\nearlier.\n\nOr perhaps we all (not just you but I am just as guilty) misjudged \"things\nthat are more likely to get broken\", even though we are very well aware\nthat touching log-tree infrastructure will have fallout all over the \"log\"\nfamily.\n"},{"id":"132260","messageId":"201001210305.05309.johan@herland.net","threadId":"22298","inReplyTo":"20100120182438.GB31507@gnu.kitenet.net","subject":"Re: git notes: notes","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2010-01-21T02:05:05Z","receivedAt":"2010-01-21T02:05:05Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wednesday 20 January 2010, Joey Hess wrote:\n> Johan Herland wrote:\n> > > PS, Has anyone thought about using notes to warn bisect away from\n> > > commits that are known to be unbuildable or otherwise cause bisection\n> > > trouble?\n> >\n> > No, I haven't thought of that specific use case. Great idea! :)\n> \n> Only problem I see with doing it is it might be too easy to overwrite\n> such a note with git notes edit -m\n\nWell, you would have to run \"git notes edit -m\" with core.notesRef or \n$GIT_NOTES_REF set to the notes ref where bisect information is stored (e.g. \n\"refs/notes/bisect\").\n\nIn any case, I would not use \"git notes\" to maintain the bisect hints. \nRather, I'd add subcommands to \"git bisect\" that would take care of \nmaintaining the notes tree @ \"refs/notes/bisect\". Much more user-friendly \nthan telling the user to write their own bisect-notes by hand.\n\n> Did you consider having -m append a line to an existing note?\n\nHmm. Not really. The \"git notes\" porcelain was originally written by Dscho, \nand my builtin-ification of it (currently in 'pu') preserves the original \nsemantics of \"git notes edit -m\". It might make sense to change the \ndefaults; what do you think, Dscho?\n\n\nHave fun! :)\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"132264","messageId":"201001210354.22756.johan@herland.net","threadId":"22298","inReplyTo":"7vljfsz7vx.fsf@alter.siamese.dyndns.org","subject":"Re: git notes: notes","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2010-01-21T02:54:22Z","receivedAt":"2010-01-21T02:54:22Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Thursday 21 January 2010, Junio C Hamano wrote:\n> [...]\n\nI just want to note that I've read the whole thread (up to here), and I \nagree with pretty much everything that's been said so far:\n\n- We should be more conservative about showing notes, especially in contexts \nthat may be used by scripts. Disabling notes by default when --pretty/--\nformat is in use, sounds like a good idea. So does adding a --show-notes \noption for overriding the default.\n\n- The format-patch bug is grave and unexcusable and must be fixed. Michael: \nThanks for discovering.\n\n- I'd still like to keep notes as part of the default output from git log \nand friends (when NOT using --pretty/--format). Only notes from a single \nnotes ref (typically the default \"refs/notes/commits\") should be shown.\n\n- Re. Peff's worry that \"git log\" will fill up with random bisection cruft: \nAny notes that are related to bisection (or any other special use case for \nnotes) should live on its own notes ref (typically \"refs/notes/bisect\" for \nbisection cruft) that is not used by \"git log\" (unless you explicitly say so \nthrough $GIT_NOTES_REF or core.notesRef).\n\n- Re. Junio's worry that he will become the janitor for these patches. \nPlease don't. As long as the patch series is in 'pu', it is MY \nresponsibility to address issues and organize any additional patches on top \nof the series. Feel free to ignore all additional patches, and wait for an \nupdated series from my end.\n\n- Yes, there should be more tests verifying that there is no negative impact \non git log and friends. Docs must be updated as well, where needed.\n\n\nUnfortunately I don't have the time to work on this right now, but I'll do \nmy best to get around to it as soon as possible (at least by the end of the \ncoming weekend).\n\n\nAgain, thanks for your involvement. It is really appreciated.\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"132265","messageId":"7vaaw8xip0.fsf@alter.siamese.dyndns.org","threadId":"22298","inReplyTo":"201001210354.22756.johan@herland.net","subject":"Re: git notes: notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-21T03:03:55Z","receivedAt":"2010-01-21T03:03:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland writes:\n\n> - Re. Junio's worry that he will become the janitor for these patches. \n>\n> Please don't. As long as the patch series is in 'pu', it is MY \n> responsibility ...\n\nI am not worried so much about what is not in 'next' yet. My worry right\nnow is primarily about what to do with upcoming 1.7.0 and also the 1.6.6.X\nseries, which already shipped with \"format-patch\" that injects notes in\nits output.\n"},{"id":"132266","messageId":"7vzl48w2jw.fsf@alter.siamese.dyndns.org","threadId":"22298","inReplyTo":"7vljfsz7vx.fsf@alter.siamese.dyndns.org","subject":"Re: git notes: notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-21T03:37:55Z","receivedAt":"2010-01-21T03:37:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Actually I am of two minds regarding --pretty={short,medium} and the\n> like. The \"how about this\" patch may be the safest for people who are\n> used to read \"log --pretty=xxx\" output with scripts, but it does look\n> inconsistent and hard to explain to new people who do not even know that\n> there were versions of git that does not know about notes.\n\nHeh, it is not surprising that we have this bug, given that all of us just\nmissed how bogus my \"how about this\" patch was ;-)\n\nThe check for \"if (fmt_pretty)\" only kicks in when that thing was read\nfrom the configuration; handling of command line --pretty and --pretty=\noptions happen long after that, when we call setup_revisions().\n\nSo if we really wanted to say \"If the user explicitly tells us to run\nunder a particular --pretty mode, we don't show notes by default.\", we\nwould need a patch like this on top of it.\n\nAnother thing to note is that \"work differently between no --pretty on the\ncommand line and an explicit --pretty=medium\" is more work than \"we always\ndefault to show notes if showing in the default verbosity\".\n\nThis version considers a user supplied \"format.pretty\" configuration as\n\"the user told us to use this specific format, and we won't show notes\nunless explicitly told\". I personally don't care either way exactly\nbecause I don't use that configuration (nor teach others to use it), but\nin a sense the configuration is setting a personal \"default\", so I think\nit could be argued that we should instead show the notes by default in\nthat case (i.e. remove \"rev->pretty_given = 1\" in the first hunk).\n\n builtin-log.c | 9 ++++++---\n revision.c | 4 ++++\n revision.h | 2 ++\n 3 files changed, 12 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 3bc3919..1e05b0f 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -39,10 +39,10 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \n \trev->abbrev = DEFAULT_ABBREV;\n \trev->commit_format = CMIT_FMT_DEFAULT;\n-\tif (fmt_pretty)\n+\tif (fmt_pretty) {\n \t\tget_commit_format(fmt_pretty, rev);\n-\telse\n-\t\trev->show_notes = 1;\n+\t\trev->pretty_given = 1;\n+\t}\n \trev->verbose_header = 1;\n \tDIFF_OPT_SET(&rev->diffopt, RECURSIVE);\n \trev->show_root_diff = default_show_root;\n@@ -60,6 +60,9 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \t\tusage(builtin_log_usage);\n \targc = setup_revisions(argc, argv, rev, \"HEAD\");\n \n+\tif (!rev->show_notes_given && !rev->pretty_given)\n+\t\trev->show_notes = 1;\n+\n \tif (rev->diffopt.pickaxe || rev->diffopt.filter)\n \t\trev->always_show_header = 0;\n \tif (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {\ndiff --git a/revision.c b/revision.c\nindex 7e00a6c..0de78fb 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1161,14 +1161,18 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->verbose_header = 1;\n \t} else if (!strcmp(arg, \"--pretty\")) {\n \t\trevs->verbose_header = 1;\n+\t\trevs->pretty_given = 1;\n \t\tget_commit_format(arg+8, revs);\n \t} else if (!prefixcmp(arg, \"--pretty=\") || !prefixcmp(arg, \"--format=\")) {\n \t\trevs->verbose_header = 1;\n+\t\trevs->pretty_given = 1;\n \t\tget_commit_format(arg+9, revs);\n \t} else if (!strcmp(arg, \"--show-notes\")) {\n \t\trevs->show_notes = 1;\n+\t\trevs->show_notes_given = 1;\n \t} else if (!strcmp(arg, \"--no-notes\")) {\n \t\trevs->show_notes = 0;\n+\t\trevs->show_notes_given = 1;\n \t} else if (!strcmp(arg, \"--oneline\")) {\n \t\trevs->verbose_header = 1;\n \t\tget_commit_format(\"oneline\", revs);\ndiff --git a/revision.h b/revision.h\nindex 4167c1e..a14deef 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -81,6 +81,8 @@ struct rev_info {\n \tunsigned int\tshown_one:1,\n \t\t\tshow_merge:1,\n \t\t\tshow_notes:1,\n+\t\t\tshow_notes_given:1,\n+\t\t\tpretty_given:1,\n \t\t\tabbrev_commit:1,\n \t\t\tuse_terminator:1,\n \t\t\tmissing_newline:1,\n"},{"id":"132267","messageId":"alpine.DEB.1.00.1001210457380.4985@pacific.mpi-cbg.de","threadId":"22298","inReplyTo":"201001210305.05309.johan@herland.net","subject":"Re: git notes: notes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-01-21T03:59:40Z","receivedAt":"2010-01-21T03:59:40Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 21 Jan 2010, Johan Herland wrote:\n\n> On Wednesday 20 January 2010, Joey Hess wrote:\n>\n> > Did you consider having -m append a line to an existing note?\n> \n> Hmm. Not really. The \"git notes\" porcelain was originally written by \n> Dscho, and my builtin-ification of it (currently in 'pu') preserves the \n> original semantics of \"git notes edit -m\". It might make sense to change \n> the defaults; what do you think, Dscho?\n\nI do not really care as long as there is a nice way to edit the complete \nnote interactively.\n\nOf course, I _do_ expect people to get confused just like they do with the \ncurrent inconsistencies: \"git commit -m\" does not really append, but set \nthe commit message, even if you amend a commit.\n\nSo maybe you want to use a different command line option for that.\n\nCiao,\nDscho\n"},{"id":"132269","messageId":"20100121040533.GA13597@gnu.kitenet.net","threadId":"22298","inReplyTo":"alpine.DEB.1.00.1001210457380.4985@pacific.mpi-cbg.de","subject":"Re: git notes: notes","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2010-01-21T04:05:34Z","receivedAt":"2010-01-21T04:05:34Z","isPatch":false,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Johannes Schindelin wrote:\n> I do not really care as long as there is a nice way to edit the complete \n> note interactively.\n> \n> Of course, I _do_ expect people to get confused just like they do with the \n> current inconsistencies: \"git commit -m\" does not really append, but set \n> the commit message, even if you amend a commit.\n> \n> So maybe you want to use a different command line option for that.\n\nMaybe: git notes add [-m|-F]\n\n-- \nsee shy jo\n"},{"id":"132284","messageId":"4B581410.9080506@drmicha.warpmail.net","threadId":"22298","inReplyTo":"7veilk1o3s.fsf@alter.siamese.dyndns.org","subject":"Re: git notes: notes","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-01-21T08:45:04Z","receivedAt":"2010-01-21T08:45:04Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"[Adding cc from elsewhere in this thread, for fairness.]\nJunio C Hamano venit, vidit, dixit 20.01.2010 22:08:\n> Michael J Gruber writes:\n> \n>> Junio C Hamano venit, vidit, dixit 20.01.2010 21:10:\n>>> Joey Hess writes:\n>>>\n>>>> Do you think it makes sense for even git log --format=format:%s to be\n>>>> porcelain and potentially change when new features are used?\n>>>\n>>> If the series changed the meaning of \"%s\" format to mean \"the subject of\n>>> the commit and notes information\", with or without documenting it, then it\n>>> is just a bug we would like to fix.\n>>\n>> No, but outputting the note as part of the log is the standard. So for\n>> example, when you do a format-patch | apply cycle, format-patch will\n>> insert the note as part of the commit message, and apply will *store*\n>> the note text (including Note:\\n) as part of the commit message of the\n>> new commit.\n> \n> Thanks; that was the kind of breakage report I was looking for (and wished\n> to have heard a lot earlier). Personally I find it is unexcusable that\n> format-patch defaults to giving notes.\n> \n>> So, I would say the notes feature is not that well integrated right now,\n> \n> No question about it.\n> \n>> I'm not complaining, I actually have this on a maybe-to-do list, but the\n>> way the series went kept me from investing time.\n> \n> Hmm, that hints there is a failure in the review and merge process. Care\n> to explain how we could have done better please?\n\nWell, I can only recall why it kept *me* from investing more time. I\nactually have very little free time available, I contribute to Git\nbecause it's fun, or I want a certain feature, or, admittedly, having a\ncommit in a project like Git gives me a certain satisfaction.\n\nThe notes feature looked very promising to me, and when Dscho came up\nwith it (or followed up on someone else's proposal, I don't remember) I\ninvested time in testing and contributing (minor) fixes. Then the series\ntook on a life on it's own, first \"disappearing\" (when I was wondering\nif anything is going forward), then reappearing with (not only my)\ncommits squashed in (which took away the satisfaction part), then going\nthrough a lengthy technical discussion on fan-out schemes (which was\nnecessary, but took away the fun part). [The last two parts may have\nbeen the other way round, which doesn't matter.]\n\nWhen the first part of the series landed I began to look at it again and\nuse it for the comments which go after the \"--\" part of a patch, only to\nfind out that format-patch issue. I noticed the bad consequence on\nformat-patch|apply only later. So I looked at the code, the log flags,\nand was about to code when the notes API changed again (on pu). So I\ndecided to wait until the API is baked (that decision may have been\ninfluenced by my expectation that my patches would get squashed in\nagain) and to fix that issue before 1.7.0.\n\nNote that I have to match the project's necessities with time\navailability on my side - otherwise I would have written that patch when\nmore of that series had landed. Now I reported it because it came up in\nsome disguise (and didn't want anyone spend time needlessly fixing a\nside issue), and I'm not the one fixing it, but that's fine.\n\nBesides the sociological aspect, I think you mentioned the main\ntechnical aspects:\n\n* If you introduce a new features, write extensive tests covering\nnon-uses and mixed uses of the feature.\n\n* Write redundancy tests, such as checking that format-patch|apply and\napply|format-patch both amount to the \"identity\" in the appropriate sense.\n\nRight now we do very atomic testing, which does have its merits (for\ndetermining the cause of a breakage). But since many features and\ncommands are not orthogonal, atomic testing does not test for side\neffects, and test repos are minimal. Trying to test for specific\ncombinations makes you miss some combinations, especially combinations\nwith future features. But testing for those identity operations\n(quasi-noops, or cycles) should ensure some consistency.\n\nMaybe we should have a test repo which has all kinds of features turned\non and used, and on which a set of those identities are tested. With\nevery new feature, the repo as well as the set of supposed identities\nwould need to be amended (maybe by cloning the repo, adding commits, and\ntesting on an increasing set of repos). That would have caught at least\nthe current issue immediately.\n\nCheers,\nMichael\n"},{"id":"132540","messageId":"vpq7hr7zism.fsf@bauges.imag.fr","threadId":"22298","inReplyTo":"7veilk1o3s.fsf@alter.siamese.dyndns.org","subject":"Re: git notes: notes","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-01-24T14:20:09Z","receivedAt":"2010-01-24T14:20:09Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Michael J Gruber writes:\n>\n>> No, but outputting the note as part of the log is the standard. So for\n>> example, when you do a format-patch | apply cycle, format-patch will\n>> insert the note as part of the commit message, and apply will *store*\n>> the note text (including Note:\\n) as part of the commit message of the\n>> new commit.\n>\n> Thanks; that was the kind of breakage report I was looking for (and wished\n> to have heard a lot earlier). Personally I find it is unexcusable that\n> format-patch defaults to giving notes.\n\nOTOH, format-patch could give the notes, below the ---, where it will\nbe ignored by apply. That would make notes handy to prepare a patch\nserie with additional messages: prepare everything within Git, and use\ngit send-email to send it.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"132541","messageId":"fabb9a1e1001240627t60f0ab0fmfe910b80439f94c@mail.gmail.com","threadId":"22298","inReplyTo":"vpq7hr7zism.fsf@bauges.imag.fr","subject":"Re: git notes: notes","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-01-24T14:27:35Z","receivedAt":"2010-01-24T14:27:35Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Sun, Jan 24, 2010 at 15:20, Matthieu Moy\n wrote:\n\n> OTOH, format-patch could give the notes, below the ---, where it will\n> be ignored by apply. That would make notes handy to prepare a patch\n> serie with additional messages: prepare everything within Git, and use\n> git send-email to send it.\n\nI like that idea, but as an orthogonal feature, prepare notes in a\nspecial namespace, say refs/notes/format-patch/, and then teach\nformat-patch a flag to honor that namespace.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"132615","messageId":"1264442884.14641.33.camel@cp-jk-linux.corp.on2.com","threadId":"22298","inReplyTo":"201001210305.05309.johan@herland.net","subject":"Re: git notes: notes","fromName":"John Koleszar","fromEmail":"john.koleszar@on2.com","sentAt":"2010-01-25T18:08:04Z","receivedAt":"2010-01-25T18:08:04Z","isPatch":false,"sender":{"key":"john.koleszar@on2.com","avatar":null},"body":"On Wed, 2010-01-20 at 21:05 -0500, Johan Herland wrote:\n> On Wednesday 20 January 2010, Joey Hess wrote:\n> > Johan Herland wrote:\n> > > > PS, Has anyone thought about using notes to warn bisect away from\n> > > > commits that are known to be unbuildable or otherwise cause bisection\n> > > > trouble?\n> > >\n> > > No, I haven't thought of that specific use case. Great idea! :)\n> > \n[...]\n> \n> In any case, I would not use \"git notes\" to maintain the bisect hints. \n> Rather, I'd add subcommands to \"git bisect\" that would take care of \n> maintaining the notes tree @ \"refs/notes/bisect\". Much more user-friendly \n> than telling the user to write their own bisect-notes by hand.\n> \n\nI haven't read up on notes more than enough to know its in the pipe, but\nI had a similar idea for using them to store bisect hints. I've been\ndoing a lot of bisecting lately into a range that had a couple dormant\nbugs where I'm trying to bisect bug B but bug A prevents me from making\na determination. Rather than skip what I know is an interesting commit,\nI cherry-pick the bugfix commit(s) A' and test that, then reset and\ncontinue bisecting.\n\nTeaching bisect to consistently skip a commit, or to automatically\nsquash in A' if we have A and not A', would be a desirable feature. I\nwill have to read up some more on notes.\n"},{"id":"132794","messageId":"201001271255.20848.johan@herland.net","threadId":"22298","inReplyTo":"20100121040533.GA13597@gnu.kitenet.net","subject":"Re: git notes: notes","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2010-01-27T11:55:20Z","receivedAt":"2010-01-27T11:55:20Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Thursday 21 January 2010, Joey Hess wrote:\n> Johannes Schindelin wrote:\n> > I do not really care as long as there is a nice way to edit the\n> > complete note interactively.\n> >\n> > Of course, I _do_ expect people to get confused just like they do with\n> > the current inconsistencies: \"git commit -m\" does not really append,\n> > but set the commit message, even if you amend a commit.\n> >\n> > So maybe you want to use a different command line option for that.\n> \n> Maybe: git notes add [-m|-F]\n\nThanks for the suggestion. I've added this to the new iteration of the \njh/notes series.\n\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"132829","messageId":"201001272101.22870.chriscool@tuxfamily.org","threadId":"22298","inReplyTo":"1264442884.14641.33.camel@cp-jk-linux.corp.on2.com","subject":"Re: git notes: notes","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-01-27T20:01:22Z","receivedAt":"2010-01-27T20:01:22Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On lundi 25 janvier 2010, John Koleszar wrote:\n> On Wed, 2010-01-20 at 21:05 -0500, Johan Herland wrote:\n> > On Wednesday 20 January 2010, Joey Hess wrote:\n>\n> > In any case, I would not use \"git notes\" to maintain the bisect hints.\n> > Rather, I'd add subcommands to \"git bisect\" that would take care of\n> > maintaining the notes tree @ \"refs/notes/bisect\". Much more\n> > user-friendly than telling the user to write their own bisect-notes by\n> > hand.\n>\n> I haven't read up on notes more than enough to know its in the pipe, but\n> I had a similar idea for using them to store bisect hints. I've been\n> doing a lot of bisecting lately into a range that had a couple dormant\n> bugs where I'm trying to bisect bug B but bug A prevents me from making\n> a determination. Rather than skip what I know is an interesting commit,\n> I cherry-pick the bugfix commit(s) A' and test that, then reset and\n> continue bisecting.\n>\n> Teaching bisect to consistently skip a commit, or to automatically\n> squash in A' if we have A and not A', would be a desirable feature. I\n> will have to read up some more on notes.\n\nPerhaps you can read about \"git replace\" in my article:\n\nhttp://www.kernel.org/pub/software/scm/git/docs/git-bisect-lk2009.html\n\nand/or my related presentation:\n\nhttp://www.linux-kongress.org/2009/slides/fighting_regressions_with_git_bisect_christian_couder.pdf\n\nI think in the long run it's much better to use git replace rather than \nnotes, especially as replace refs for bisecting could be in their own \nrefs/replace/bisect namespace. I may take the time to implement that soon \nif you or other people are interested.\n\nRegards,\nChristian.\n"}]}