{"thread":{"id":"26906","subject":"[PATCH] revision.c: introduce --notes-ref= to use one notes ref only","startedAt":"2011-03-29T10:05:09Z","lastAt":"2011-03-30T03:32:05Z","messageCount":23,"participants":["Michael J Gruber","Johan Herland","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"164555","messageId":"e83f8b622fba5add563fc331ae3922b79a0af008.1301392999.git.git@drmicha.warpmail.net","threadId":"26906","inReplyTo":null,"subject":"[PATCH] revision.c: introduce --notes-ref= to use one notes ref only","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-03-29T10:05:09Z","receivedAt":"2011-03-29T10:05:09Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"As notes become increasingly popular, it's often interesting to show\nnotes from a particular notes ref only. Introduce '--notes-ref=<ref>'\nas a convenience shortcut for '--no-standard-notes --show-notes=<ref>'.\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\nThe idea is to use the same name as in \"git notes --ref=<ref>\" but make\nit clear for the rev-list option to be about notes, thus \"--notes-ref=<ref>\".\n\n Documentation/git-log.txt        |    3 ++-\n Documentation/pretty-options.txt |    4 ++++\n revision.c                       |   15 +++++++++++----\n t/t3301-notes.sh                 |    5 +++++\n 4 files changed, 22 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex 2c84028..56ffccd 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -179,7 +179,8 @@ multiple times.  A warning will be issued for refs that do not exist,\n but a glob that does not match any refs is silently ignored.\n +\n This setting can be disabled by the `--no-standard-notes` option,\n-overridden by the 'GIT_NOTES_DISPLAY_REF' environment variable,\n+overridden by the 'GIT_NOTES_DISPLAY_REF' environment variable\n+or the `--notes-ref` option,\n and supplemented by the `--show-notes` option.\n \n GIT\ndiff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\nindex 50923e2..ef5eed4 100644\n--- a/Documentation/pretty-options.txt\n+++ b/Documentation/pretty-options.txt\n@@ -46,3 +46,7 @@ is taken to be in `refs/notes/` if it is not qualified.\n \t'core.notesRef' and 'notes.displayRef' variables (or\n \tcorresponding environment overrides).  Enabled by default.\n \tSee linkgit:git-config[1].\n+\n+--notes-ref[=<ref>]::\n+\tThis is the same as `--no-standard-notes --show-notes=<ref>`,\n+\ti.e. it shows only the notes from the notes tree at `<ref>`.\ndiff --git a/revision.c b/revision.c\nindex 0f38364..d620926 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1368,19 +1368,26 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\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 (!prefixcmp(arg, \"--show-notes=\")) {\n+\t} else if (!prefixcmp(arg, \"--show-notes=\") || !prefixcmp(arg, \"--notes-ref=\")) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tint offset = strlen(\"--show-notes=\");\n \t\trevs->show_notes = 1;\n \t\trevs->show_notes_given = 1;\n+\t\tif (!prefixcmp(arg, \"--notes-ref=\")) {\n+\t\t\toffset = strlen(\"--notes-ref=\");\n+\t\t\tif (revs->notes_opt.extra_notes_refs)\n+\t\t\t\tstring_list_clear(revs->notes_opt.extra_notes_refs, 0);\n+\t\t\trevs->notes_opt.suppress_default_notes = 1;\n+\t\t}\n \t\tif (!revs->notes_opt.extra_notes_refs)\n \t\t\trevs->notes_opt.extra_notes_refs = xcalloc(1, sizeof(struct string_list));\n-\t\tif (!prefixcmp(arg+13, \"refs/\"))\n+\t\tif (!prefixcmp(arg+offset, \"refs/\"))\n \t\t\t/* happy */;\n-\t\telse if (!prefixcmp(arg+13, \"notes/\"))\n+\t\telse if (!prefixcmp(arg+offset, \"notes/\"))\n \t\t\tstrbuf_addstr(&buf, \"refs/\");\n \t\telse\n \t\t\tstrbuf_addstr(&buf, \"refs/notes/\");\n-\t\tstrbuf_addstr(&buf, arg+13);\n+\t\tstrbuf_addstr(&buf, arg+offset);\n \t\tstring_list_append(revs->notes_opt.extra_notes_refs,\n \t\t\t\t   strbuf_detach(&buf, NULL));\n \t} else if (!strcmp(arg, \"--no-notes\")) {\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 1921ca3..3fcfdc7 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -625,6 +625,11 @@ test_expect_success '--show-notes=ref accumulates' '\n \ttest_cmp expect-both-reversed output\n '\n \n+test_expect_success '--notes-ref=' '\n+\tgit log --notes-ref=other -1 > output &&\n+\ttest_cmp expect-other output\n+'\n+\n test_expect_success 'Allow notes on non-commits (trees, blobs, tags)' '\n \tgit config core.notesRef refs/notes/other &&\n \techo \"Note on a tree\" > expect &&\n-- \n1.7.4.1.607.g888da\n"},{"id":"164566","messageId":"201103291439.17197.johan@herland.net","threadId":"26906","inReplyTo":"e83f8b622fba5add563fc331ae3922b79a0af008.1301392999.git.git@drmicha.warpmail.net","subject":"Re: [PATCH] revision.c: introduce --notes-ref= to use one notes ref only","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2011-03-29T12:39:17Z","receivedAt":"2011-03-29T12:39:17Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Tuesday 29 March 2011, Michael J Gruber wrote:\n> As notes become increasingly popular, it's often interesting to show\n> notes from a particular notes ref only. Introduce '--notes-ref=<ref>'\n> as a convenience shortcut for '--no-standard-notes\n> --show-notes=<ref>'.\n>\n> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n> ---\n> The idea is to use the same name as in \"git notes --ref=<ref>\" but\n> make it clear for the rev-list option to be about notes, thus\n> \"--notes-ref=<ref>\".\n\nThe idea and implementation look good to me. Not sure I like the \noption \"bloat\" (somehow feels it should be possible to express the same \nbehavior using fewer options), but if there's not a better way to \nreorganize the options, then you can consider it Acked-by me.\n\n\nThanks! :)\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"164575","messageId":"20110329143357.GA10771@sigill.intra.peff.net","threadId":"26906","inReplyTo":"201103291439.17197.johan@herland.net","subject":"Re: [PATCH] revision.c: introduce --notes-ref= to use one notes ref only","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-29T14:33:57Z","receivedAt":"2011-03-29T14:33:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 29, 2011 at 02:39:17PM +0200, Johan Herland wrote:\n\n> On Tuesday 29 March 2011, Michael J Gruber wrote:\n> > As notes become increasingly popular, it's often interesting to show\n> > notes from a particular notes ref only. Introduce '--notes-ref=<ref>'\n> > as a convenience shortcut for '--no-standard-notes\n> > --show-notes=<ref>'.\n> >\n> > Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n> > ---\n> > The idea is to use the same name as in \"git notes --ref=<ref>\" but\n> > make it clear for the rev-list option to be about notes, thus\n> > \"--notes-ref=<ref>\".\n> \n> The idea and implementation look good to me. Not sure I like the \n> option \"bloat\" (somehow feels it should be possible to express the same \n> behavior using fewer options), but if there's not a better way to \n> reorganize the options, then you can consider it Acked-by me.\n\nI feel this would be more consistent with most other options that take\nan optional argument:\n\n  1. \"--show-notes\" uses default refs\n\n  2. \"--show-notes=<ref>\" shows _just_ <ref>, no defaults\n\n  3. \"--show-notes=<ref1> --show-notes=<ref2>\" shows <ref1> and <ref2>\n\n  4. (Probably) \"--show-notes --show-notes=<ref>\" should show default\n     refs and <ref>. This is the one I'm least sure of, as it leaves no\n     way to override what came earlier on the command line (which is\n     useful if, for example, we end up with Michael's proposed ui.log).\n     Perhaps \"--no-notes\" would reset, so:\n\n       --show-notes --no-notes --show-notes=<ref>\n\n     would be equivalent to:\n\n       --show-notes=<ref>\n\nOf course a total behavior change of what --show-notes currently does.\n\nSpeaking of which, it is kind of weird that --show-notes is negated by\n--no-notes. So maybe it makes sense to introduce \"--notes[=<ref>]\" to do\nwhat I wrote above, and deprecate --show-notes.\n\n-Peff\n"},{"id":"164576","messageId":"20110329143547.GB10771@sigill.intra.peff.net","threadId":"26906","inReplyTo":"e83f8b622fba5add563fc331ae3922b79a0af008.1301392999.git.git@drmicha.warpmail.net","subject":"Re: [PATCH] revision.c: introduce --notes-ref= to use one notes ref only","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-29T14:35:47Z","receivedAt":"2011-03-29T14:35:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 29, 2011 at 12:05:09PM +0200, Michael J Gruber wrote:\n\n> -\t\tif (!prefixcmp(arg+13, \"refs/\"))\n> +\t\tif (!prefixcmp(arg+offset, \"refs/\"))\n>  \t\t\t/* happy */;\n> -\t\telse if (!prefixcmp(arg+13, \"notes/\"))\n> +\t\telse if (!prefixcmp(arg+offset, \"notes/\"))\n>  \t\t\tstrbuf_addstr(&buf, \"refs/\");\n>  \t\telse\n>  \t\t\tstrbuf_addstr(&buf, \"refs/notes/\");\n> -\t\tstrbuf_addstr(&buf, arg+13);\n> +\t\tstrbuf_addstr(&buf, arg+offset);\n>  \t\tstring_list_append(revs->notes_opt.extra_notes_refs,\n>  \t\t\t\t   strbuf_detach(&buf, NULL));\n\nThis issue is not introduced by your patch, but maybe it is a good\nopportunity to refactor this to use expand_notes_ref from notes.c?\n\n-Peff\n"},{"id":"164580","messageId":"4D91F7E4.1040707@drmicha.warpmail.net","threadId":"26906","inReplyTo":"20110329143357.GA10771@sigill.intra.peff.net","subject":"Re: [PATCH] revision.c: introduce --notes-ref= to use one notes ref only","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-03-29T15:16:52Z","receivedAt":"2011-03-29T15:16:52Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 29.03.2011 16:33:\n> On Tue, Mar 29, 2011 at 02:39:17PM +0200, Johan Herland wrote:\n> \n>> On Tuesday 29 March 2011, Michael J Gruber wrote:\n>>> As notes become increasingly popular, it's often interesting to show\n>>> notes from a particular notes ref only. Introduce '--notes-ref=<ref>'\n>>> as a convenience shortcut for '--no-standard-notes\n>>> --show-notes=<ref>'.\n>>>\n>>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n>>> ---\n>>> The idea is to use the same name as in \"git notes --ref=<ref>\" but\n>>> make it clear for the rev-list option to be about notes, thus\n>>> \"--notes-ref=<ref>\".\n>>\n>> The idea and implementation look good to me. Not sure I like the \n>> option \"bloat\" (somehow feels it should be possible to express the same \n>> behavior using fewer options), but if there's not a better way to \n>> reorganize the options, then you can consider it Acked-by me.\n> \n> I feel this would be more consistent with most other options that take\n> an optional argument:\n> \n>   1. \"--show-notes\" uses default refs\n> \n>   2. \"--show-notes=<ref>\" shows _just_ <ref>, no defaults\n> \n>   3. \"--show-notes=<ref1> --show-notes=<ref2>\" shows <ref1> and <ref2>\n> \n>   4. (Probably) \"--show-notes --show-notes=<ref>\" should show default\n>      refs and <ref>. This is the one I'm least sure of, as it leaves no\n>      way to override what came earlier on the command line (which is\n>      useful if, for example, we end up with Michael's proposed ui.log).\n\nMy \"git log\" shows notes from ref/notes/commits by default without alias\nor config, and that is what I want to override per command (to show\nThomas' notes, e.g.).\n\n>      Perhaps \"--no-notes\" would reset, so:\n> \n>        --show-notes --no-notes --show-notes=<ref>\n> \n>      would be equivalent to:\n> \n>        --show-notes=<ref>\n> \n> Of course a total behavior change of what --show-notes currently does.\n\nI somehow stopped proposing behavior changes. Guess why? (I know I have\nmy occasional relapse, but still...)\n\n> \n> Speaking of which, it is kind of weird that --show-notes is negated by\n> --no-notes. So maybe it makes sense to introduce \"--notes[=<ref>]\" to do\n> what I wrote above, and deprecate --show-notes.\n\nAlso, \"git notes\" has \"--ref\". Maybe this (which may be what you\nproposed above):\n\n--notes: show standard notes\n--notes=<ref>: show notes from <ref> only\n--notes --notes=<ref>: show standard notes + those from <ref>\n(i.e., if any notes argument was given they accumulate; a single\nargument does not add to, but replaces the default)\n--no-notes: you guess it\n\nOne could deprecate --[no-]stand-notes as well, then.\n\nChanging status \"PATCH\" back to \"PATCH/RFC\"...\n\nMichael\n"},{"id":"164605","messageId":"7vwrjhrd53.fsf@alter.siamese.dyndns.org","threadId":"26906","inReplyTo":"20110329143357.GA10771@sigill.intra.peff.net","subject":"Re: [PATCH] revision.c: introduce --notes-ref= to use one notes ref only","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-29T18:32:24Z","receivedAt":"2011-03-29T18:32:24Z","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> Speaking of which, it is kind of weird that --show-notes is negated by\n> --no-notes. So maybe it makes sense to introduce \"--notes[=<ref>]\" to do\n> what I wrote above, and deprecate --show-notes.\n\nI think that is sensible.\n\nI personally think that \"notes\" are way too premature to be used seriously\nby normal people yet, and if we want to fix UI and semantics warts (it is\nunderstandable if we had plenty of them, simply because we didn't know\nenough about possible use cases during the period we prototyped the notes\nfeature), the time to do so is now.\n"},{"id":"164612","messageId":"20110329190138.GA23599@sigill.intra.peff.net","threadId":"26906","inReplyTo":"20110329143547.GB10771@sigill.intra.peff.net","subject":"Re: [PATCH] revision.c: introduce --notes-ref= to use one notes ref only","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-29T19:01:38Z","receivedAt":"2011-03-29T19:01:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 29, 2011 at 10:35:47AM -0400, Jeff King wrote:\n\n> On Tue, Mar 29, 2011 at 12:05:09PM +0200, Michael J Gruber wrote:\n> \n> > -\t\tif (!prefixcmp(arg+13, \"refs/\"))\n> > +\t\tif (!prefixcmp(arg+offset, \"refs/\"))\n> >  \t\t\t/* happy */;\n> > -\t\telse if (!prefixcmp(arg+13, \"notes/\"))\n> > +\t\telse if (!prefixcmp(arg+offset, \"notes/\"))\n> >  \t\t\tstrbuf_addstr(&buf, \"refs/\");\n> >  \t\telse\n> >  \t\t\tstrbuf_addstr(&buf, \"refs/notes/\");\n> > -\t\tstrbuf_addstr(&buf, arg+13);\n> > +\t\tstrbuf_addstr(&buf, arg+offset);\n> >  \t\tstring_list_append(revs->notes_opt.extra_notes_refs,\n> >  \t\t\t\t   strbuf_detach(&buf, NULL));\n> \n> This issue is not introduced by your patch, but maybe it is a good\n> opportunity to refactor this to use expand_notes_ref from notes.c?\n\nOops, I just realized this is in builtin/notes.c in master. I had\nalready written a patch for another topic that made it globally\naccessible. :)\n\n-Peff\n"},{"id":"164617","messageId":"4D923792.9010101@drmicha.warpmail.net","threadId":"26906","inReplyTo":"20110329190138.GA23599@sigill.intra.peff.net","subject":"Re: [PATCH] revision.c: introduce --notes-ref= to use one notes ref only","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-03-29T19:48:34Z","receivedAt":"2011-03-29T19:48:34Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 29.03.2011 21:01:\n> On Tue, Mar 29, 2011 at 10:35:47AM -0400, Jeff King wrote:\n> \n>> On Tue, Mar 29, 2011 at 12:05:09PM +0200, Michael J Gruber wrote:\n>>\n>>> -\t\tif (!prefixcmp(arg+13, \"refs/\"))\n>>> +\t\tif (!prefixcmp(arg+offset, \"refs/\"))\n>>>  \t\t\t/* happy */;\n>>> -\t\telse if (!prefixcmp(arg+13, \"notes/\"))\n>>> +\t\telse if (!prefixcmp(arg+offset, \"notes/\"))\n>>>  \t\t\tstrbuf_addstr(&buf, \"refs/\");\n>>>  \t\telse\n>>>  \t\t\tstrbuf_addstr(&buf, \"refs/notes/\");\n>>> -\t\tstrbuf_addstr(&buf, arg+13);\n>>> +\t\tstrbuf_addstr(&buf, arg+offset);\n>>>  \t\tstring_list_append(revs->notes_opt.extra_notes_refs,\n>>>  \t\t\t\t   strbuf_detach(&buf, NULL));\n>>\n>> This issue is not introduced by your patch, but maybe it is a good\n>> opportunity to refactor this to use expand_notes_ref from notes.c?\n> \n> Oops, I just realized this is in builtin/notes.c in master. I had\n> already written a patch for another topic that made it globally\n> accessible. :)\n\nYeah, I (figured and) factored it out myself meanwhile, and rebased. I'm\nwondering though where we are going. Junio seems to be in a mood for\nmajor changes to the notes ui, so maybe I should hold on until we\ndecided about a ui restructuring.\n\nI think, though, that any notes ui revamp is correlated with our\n(stalled?) discussions about the layout of refs/. It affects not only\nthe default notes ref (\"commits\" for all notes?) but also the question\nwhat a standard notes ref is, and where to store (and how to specify)\nupstream notes refs.\n\nMichael\n"},{"id":"164621","messageId":"20110329202335.GA5997@sigill.intra.peff.net","threadId":"26906","inReplyTo":"4D923792.9010101@drmicha.warpmail.net","subject":"Re: [PATCH] revision.c: introduce --notes-ref= to use one notes ref only","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-29T20:23:35Z","receivedAt":"2011-03-29T20:23:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 29, 2011 at 09:48:34PM +0200, Michael J Gruber wrote:\n\n> >> This issue is not introduced by your patch, but maybe it is a good\n> >> opportunity to refactor this to use expand_notes_ref from notes.c?\n> > \n> > Oops, I just realized this is in builtin/notes.c in master. I had\n> > already written a patch for another topic that made it globally\n> > accessible. :)\n> \n> Yeah, I (figured and) factored it out myself meanwhile, and rebased. I'm\n> wondering though where we are going. Junio seems to be in a mood for\n> major changes to the notes ui, so maybe I should hold on until we\n> decided about a ui restructuring.\n\nI have a series I'll send in a few minutes. It _would_ be a lot cleaner\nif we just dropped --show-notes and company entirely, but I think that\nis perhaps too aggressive, even for such a young feature.\n\n> I think, though, that any notes ui revamp is correlated with our\n> (stalled?) discussions about the layout of refs/. It affects not only\n> the default notes ref (\"commits\" for all notes?) but also the question\n> what a standard notes ref is, and where to store (and how to specify)\n> upstream notes refs.\n\nYeah, I think those are open questions. But we can probably get away\nwith at least this option refactoring without having to answer them.\n\n-Peff\n"},{"id":"164624","messageId":"20110329205307.GA30959@sigill.intra.peff.net","threadId":"26906","inReplyTo":"7vwrjhrd53.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] revision.c: introduce --notes-ref= to use one notes ref only","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-29T20:53:08Z","receivedAt":"2011-03-29T20:53:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 29, 2011 at 11:32:24AM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Speaking of which, it is kind of weird that --show-notes is negated by\n> > --no-notes. So maybe it makes sense to introduce \"--notes[=<ref>]\" to do\n> > what I wrote above, and deprecate --show-notes.\n> \n> I think that is sensible.\n> \n> I personally think that \"notes\" are way too premature to be used seriously\n> by normal people yet, and if we want to fix UI and semantics warts (it is\n> understandable if we had plenty of them, simply because we didn't know\n> enough about possible use cases during the period we prototyped the notes\n> feature), the time to do so is now.\n\nIt is tempting to kill off --show-notes and --standard-notes, as it\nwould clean up this code a bit. But even though they are probably not\nbeing seriously used by normal people, they have been in several\nreleased versions.\n\nHere's the series I ended up with. Getting the refactoring just right\nturned out to be non-trivial, but between several attempts and some\ntests, I think the end result is correct. Hopefully the breakdown of the\nchanges into small patches helps make it easy to review.\n\n  [1/6]: notes: make expand_notes_ref globally accessible\n  [2/6]: revision.c: refactor notes ref expansion\n  [3/6]: notes: refactor display notes extra refs field\n  [4/6]: notes: refactor display notes default handling\n  [5/6]: revision.c: support --notes command-line option\n  [6/6]: revision.c: make --no-notes reset --notes list\n\n-Peff\n"},{"id":"164625","messageId":"20110329205532.GA23466@sigill.intra.peff.net","threadId":"26906","inReplyTo":"20110329205307.GA30959@sigill.intra.peff.net","subject":"[PATCH 1/6] notes: make expand_notes_ref globally accessible","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-29T20:55:32Z","receivedAt":"2011-03-29T20:55:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This function is useful for other commands besides \"git\nnotes\" which want to let users refer to notes by their\nshorthand name.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/notes.c |   10 ----------\n notes.c         |   10 ++++++++++\n notes.h         |    3 +++\n 3 files changed, 13 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex a0f310b..6227607 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -100,16 +100,6 @@ struct msg_arg {\n \tstruct strbuf buf;\n };\n \n-static void expand_notes_ref(struct strbuf *sb)\n-{\n-\tif (!prefixcmp(sb->buf, \"refs/notes/\"))\n-\t\treturn; /* we're happy */\n-\telse if (!prefixcmp(sb->buf, \"notes/\"))\n-\t\tstrbuf_insert(sb, 0, \"refs/\", 5);\n-\telse\n-\t\tstrbuf_insert(sb, 0, \"refs/notes/\", 11);\n-}\n-\n static int list_each_note(const unsigned char *object_sha1,\n \t\tconst unsigned char *note_sha1, char *note_path,\n \t\tvoid *cb_data)\ndiff --git a/notes.c b/notes.c\nindex a013c1b..f6b9b6a 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -1285,3 +1285,13 @@ int copy_note(struct notes_tree *t,\n \n \treturn 0;\n }\n+\n+void expand_notes_ref(struct strbuf *sb)\n+{\n+\tif (!prefixcmp(sb->buf, \"refs/notes/\"))\n+\t\treturn; /* we're happy */\n+\telse if (!prefixcmp(sb->buf, \"notes/\"))\n+\t\tstrbuf_insert(sb, 0, \"refs/\", 5);\n+\telse\n+\t\tstrbuf_insert(sb, 0, \"refs/notes/\", 11);\n+}\ndiff --git a/notes.h b/notes.h\nindex 83bd6e0..60bdf28 100644\n--- a/notes.h\n+++ b/notes.h\n@@ -307,4 +307,7 @@ void string_list_add_refs_by_glob(struct string_list *list, const char *glob);\n void string_list_add_refs_from_colon_sep(struct string_list *list,\n \t\t\t\t\t const char *globs);\n \n+/* Expand inplace a note ref like \"foo\" or \"notes/foo\" into \"refs/notes/foo\" */\n+void expand_notes_ref(struct strbuf *sb);\n+\n #endif\n-- \n1.7.4.2.7.g9407\n"},{"id":"164626","messageId":"20110329205604.GB23466@sigill.intra.peff.net","threadId":"26906","inReplyTo":"20110329205307.GA30959@sigill.intra.peff.net","subject":"[PATCH 2/6] revision.c: refactor notes ref expansion","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-29T20:56:04Z","receivedAt":"2011-03-29T20:56:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"No need to do it ourselves when there is a library function.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n revision.c |    7 +------\n 1 files changed, 1 insertions(+), 6 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 0f38364..5826e5d 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1374,13 +1374,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->show_notes_given = 1;\n \t\tif (!revs->notes_opt.extra_notes_refs)\n \t\t\trevs->notes_opt.extra_notes_refs = xcalloc(1, sizeof(struct string_list));\n-\t\tif (!prefixcmp(arg+13, \"refs/\"))\n-\t\t\t/* happy */;\n-\t\telse if (!prefixcmp(arg+13, \"notes/\"))\n-\t\t\tstrbuf_addstr(&buf, \"refs/\");\n-\t\telse\n-\t\t\tstrbuf_addstr(&buf, \"refs/notes/\");\n \t\tstrbuf_addstr(&buf, arg+13);\n+\t\texpand_notes_ref(&buf);\n \t\tstring_list_append(revs->notes_opt.extra_notes_refs,\n \t\t\t\t   strbuf_detach(&buf, NULL));\n \t} else if (!strcmp(arg, \"--no-notes\")) {\n-- \n1.7.4.2.7.g9407\n"},{"id":"164627","messageId":"20110329205653.GC23466@sigill.intra.peff.net","threadId":"26906","inReplyTo":"20110329205307.GA30959@sigill.intra.peff.net","subject":"[PATCH 3/6] notes: refactor display notes extra refs field","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-29T20:56:53Z","receivedAt":"2011-03-29T20:56:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There's no need to use an extra pointer, which just ends up\nleaking memory. The fact that the list is empty tells us the\nsame thing.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n notes.c    |    4 ++--\n notes.h    |    4 +++-\n revision.c |    4 +---\n 3 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/notes.c b/notes.c\nindex f6b9b6a..2ec604c 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -1066,9 +1066,9 @@ void init_display_notes(struct display_notes_opt *opt)\n \n \tgit_config(notes_display_config, &load_config_refs);\n \n-\tif (opt && opt->extra_notes_refs) {\n+\tif (opt) {\n \t\tstruct string_list_item *item;\n-\t\tfor_each_string_list_item(item, opt->extra_notes_refs)\n+\t\tfor_each_string_list_item(item, &opt->extra_notes_refs)\n \t\t\tstring_list_add_refs_by_glob(&display_notes_refs,\n \t\t\t\t\t\t     item->string);\n \t}\ndiff --git a/notes.h b/notes.h\nindex 60bdf28..7ae3eef 100644\n--- a/notes.h\n+++ b/notes.h\n@@ -1,6 +1,8 @@\n #ifndef NOTES_H\n #define NOTES_H\n \n+#include \"string-list.h\"\n+\n /*\n  * Function type for combining two notes annotating the same object.\n  *\n@@ -257,7 +259,7 @@ struct string_list;\n \n struct display_notes_opt {\n \tunsigned int suppress_default_notes:1;\n-\tstruct string_list *extra_notes_refs;\n+\tstruct string_list extra_notes_refs;\n };\n \n /*\ndiff --git a/revision.c b/revision.c\nindex 5826e5d..24b89eb 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1372,11 +1372,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\tstruct strbuf buf = STRBUF_INIT;\n \t\trevs->show_notes = 1;\n \t\trevs->show_notes_given = 1;\n-\t\tif (!revs->notes_opt.extra_notes_refs)\n-\t\t\trevs->notes_opt.extra_notes_refs = xcalloc(1, sizeof(struct string_list));\n \t\tstrbuf_addstr(&buf, arg+13);\n \t\texpand_notes_ref(&buf);\n-\t\tstring_list_append(revs->notes_opt.extra_notes_refs,\n+\t\tstring_list_append(&revs->notes_opt.extra_notes_refs,\n \t\t\t\t   strbuf_detach(&buf, NULL));\n \t} else if (!strcmp(arg, \"--no-notes\")) {\n \t\trevs->show_notes = 0;\n-- \n1.7.4.2.7.g9407\n"},{"id":"164628","messageId":"20110329205727.GD23466@sigill.intra.peff.net","threadId":"26906","inReplyTo":"20110329205307.GA30959@sigill.intra.peff.net","subject":"[PATCH 4/6] notes: refactor display notes default handling","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-29T20:57:27Z","receivedAt":"2011-03-29T20:57:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is in preparation for more notes-related revision\ncommand-line options.\n\nThe \"suppress_default_notes\" option is renamed to\n\"use_default_notes\", and is now a tri-state with values less\nthan one indicating \"not set\".  If the value is \"not set\",\nthen we show default refs if and only if no other refs were\ngiven.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n notes.c    |    3 ++-\n notes.h    |    2 +-\n revision.c |    9 +++++++--\n 3 files changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git a/notes.c b/notes.c\nindex 2ec604c..f6ce848 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -1053,7 +1053,8 @@ void init_display_notes(struct display_notes_opt *opt)\n \n \tassert(!display_notes_trees);\n \n-\tif (!opt || !opt->suppress_default_notes) {\n+\tif (!opt || opt->use_default_notes > 0 ||\n+\t    (opt->use_default_notes == -1 && !opt->extra_notes_refs.nr)) {\n \t\tstring_list_append(&display_notes_refs, default_notes_ref());\n \t\tdisplay_ref_env = getenv(GIT_NOTES_DISPLAY_REF_ENVIRONMENT);\n \t\tif (display_ref_env) {\ndiff --git a/notes.h b/notes.h\nindex 7ae3eef..c716694 100644\n--- a/notes.h\n+++ b/notes.h\n@@ -258,7 +258,7 @@ void format_note(struct notes_tree *t, const unsigned char *object_sha1,\n struct string_list;\n \n struct display_notes_opt {\n-\tunsigned int suppress_default_notes:1;\n+\tint use_default_notes;\n \tstruct string_list extra_notes_refs;\n };\n \ndiff --git a/revision.c b/revision.c\nindex 24b89eb..315a7f4 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -955,6 +955,8 @@ void init_revisions(struct rev_info *revs, const char *prefix)\n \t\trevs->diffopt.prefix = prefix;\n \t\trevs->diffopt.prefix_length = strlen(prefix);\n \t}\n+\n+\trevs->notes_opt.use_default_notes = -1;\n }\n \n static void add_pending_commit_list(struct rev_info *revs,\n@@ -1368,10 +1370,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--show-notes\")) {\n \t\trevs->show_notes = 1;\n \t\trevs->show_notes_given = 1;\n+\t\trevs->notes_opt.use_default_notes = 1;\n \t} else if (!prefixcmp(arg, \"--show-notes=\")) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n \t\trevs->show_notes = 1;\n \t\trevs->show_notes_given = 1;\n+\t\tif (revs->notes_opt.use_default_notes < 0)\n+\t\t\trevs->notes_opt.use_default_notes = 1;\n \t\tstrbuf_addstr(&buf, arg+13);\n \t\texpand_notes_ref(&buf);\n \t\tstring_list_append(&revs->notes_opt.extra_notes_refs,\n@@ -1381,9 +1386,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->show_notes_given = 1;\n \t} else if (!strcmp(arg, \"--standard-notes\")) {\n \t\trevs->show_notes_given = 1;\n-\t\trevs->notes_opt.suppress_default_notes = 0;\n+\t\trevs->notes_opt.use_default_notes = 1;\n \t} else if (!strcmp(arg, \"--no-standard-notes\")) {\n-\t\trevs->notes_opt.suppress_default_notes = 1;\n+\t\trevs->notes_opt.use_default_notes = 0;\n \t} else if (!strcmp(arg, \"--oneline\")) {\n \t\trevs->verbose_header = 1;\n \t\tget_commit_format(\"oneline\", revs);\n-- \n1.7.4.2.7.g9407\n"},{"id":"164629","messageId":"20110329205747.GE23466@sigill.intra.peff.net","threadId":"26906","inReplyTo":"20110329205307.GA30959@sigill.intra.peff.net","subject":"[PATCH 5/6] revision.c: support --notes command-line option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-29T20:57:47Z","receivedAt":"2011-03-29T20:57:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We already have --show-notes, but it has a few shortcomings:\n\n  1. Using --show-notes=<ref> implies that we should also\n     show the default notes. Which means you also need to\n     use --no-standard-notes if you want to suppress them.\n\n  2. It is negated by --no-notes, which doesn't match.\n\n  3. It's too long to type. :)\n\nThis patch introduces --notes, which behaves exactly like\n--show-notes, except that using \"--notes=<ref>\" does not\nimply showing the default notes.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n revision.c       |   15 ++++++++++-----\n t/t3301-notes.sh |   22 ++++++++++++++++++++++\n 2 files changed, 32 insertions(+), 5 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 315a7f4..c4ffee4 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1367,17 +1367,22 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\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} else if (!strcmp(arg, \"--show-notes\") || !strcmp(arg, \"--notes\")) {\n \t\trevs->show_notes = 1;\n \t\trevs->show_notes_given = 1;\n \t\trevs->notes_opt.use_default_notes = 1;\n-\t} else if (!prefixcmp(arg, \"--show-notes=\")) {\n+\t} else if (!prefixcmp(arg, \"--show-notes=\") ||\n+\t\t   !prefixcmp(arg, \"--notes=\")) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n \t\trevs->show_notes = 1;\n \t\trevs->show_notes_given = 1;\n-\t\tif (revs->notes_opt.use_default_notes < 0)\n-\t\t\trevs->notes_opt.use_default_notes = 1;\n-\t\tstrbuf_addstr(&buf, arg+13);\n+\t\tif (!prefixcmp(arg, \"--show-notes\")) {\n+\t\t\tif (revs->notes_opt.use_default_notes < 0)\n+\t\t\t\trevs->notes_opt.use_default_notes = 1;\n+\t\t\tstrbuf_addstr(&buf, arg+13);\n+\t\t}\n+\t\telse\n+\t\t\tstrbuf_addstr(&buf, arg+8);\n \t\texpand_notes_ref(&buf);\n \t\tstring_list_append(&revs->notes_opt.extra_notes_refs,\n \t\t\t\t   strbuf_detach(&buf, NULL));\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 1921ca3..f0e7a58 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -247,6 +247,28 @@ do\n \t'\n done\n \n+test_expect_success 'setup alternate notes ref' '\n+\tgit notes --ref=alternate add -m alternate\n+'\n+\n+test_expect_success 'git log --notes shows default notes' '\n+\tgit log -1 --notes >output &&\n+\tgrep xyzzy output &&\n+\t! grep alternate output\n+'\n+\n+test_expect_success 'git log --notes=X shows only X' '\n+\tgit log -1 --notes=alternate >output &&\n+\t! grep xyzzy output &&\n+\tgrep alternate output\n+'\n+\n+test_expect_success 'git log --notes --notes=X shows both' '\n+\tgit log -1 --notes --notes=alternate >output &&\n+\tgrep xyzzy output &&\n+\tgrep alternate output\n+'\n+\n test_expect_success 'create -m notes (setup)' '\n \t: > a5 &&\n \tgit add a5 &&\n-- \n1.7.4.2.7.g9407\n"},{"id":"164630","messageId":"20110329205942.GF23466@sigill.intra.peff.net","threadId":"26906","inReplyTo":"20110329205307.GA30959@sigill.intra.peff.net","subject":"[PATCH 6/6] revision.c: make --no-notes reset --notes list","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-29T20:59:42Z","receivedAt":"2011-03-29T20:59:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"With most command line options, later instances of an option\noverride earlier ones. With cumulative options like\n\"--notes\", however, there is no way to say \"forget the\n--notes I gave you before\".\n\nLet's have --no-notes trigger this forgetting, so that:\n\n  git log --notes=foo --no-notes --notes=bar\n\nwill show only the \"bar\" notes.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nTechnically this is a regression for:\n\n  git log --show-notes=foo --no-notes --show-notes=bar\n\nwhich used to show both. But I consider that behavior useless and crazy,\nand since the --no-notes actually did nothing there, why would anyone\nhave been using it?\n\n revision.c       |    6 ++++++\n t/t3301-notes.sh |   16 ++++++++++++++++\n 2 files changed, 22 insertions(+), 0 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex c4ffee4..541f09e 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1389,6 +1389,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--no-notes\")) {\n \t\trevs->show_notes = 0;\n \t\trevs->show_notes_given = 1;\n+\t\trevs->notes_opt.use_default_notes = -1;\n+\t\t/* we have been strdup'ing ourselves, so trick\n+\t\t * string_list into free()ing strings */\n+\t\trevs->notes_opt.extra_notes_refs.strdup_strings = 1;\n+\t\tstring_list_clear(&revs->notes_opt.extra_notes_refs, 0);\n+\t\trevs->notes_opt.extra_notes_refs.strdup_strings = 0;\n \t} else if (!strcmp(arg, \"--standard-notes\")) {\n \t\trevs->show_notes_given = 1;\n \t\trevs->notes_opt.use_default_notes = 1;\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex f0e7a58..8600db7 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -269,6 +269,22 @@ test_expect_success 'git log --notes --notes=X shows both' '\n \tgrep alternate output\n '\n \n+test_expect_success 'git log --no-notes resets default state' '\n+\tgit log -1 --notes --notes=alternate \\\n+\t\t--no-notes --notes=alternate \\\n+\t\t>output &&\n+\t! grep xyzzy output &&\n+\tgrep alternate output\n+'\n+\n+test_expect_success 'git log --no-notes resets ref list' '\n+\tgit log -1 --notes --notes=alternate \\\n+\t\t--no-notes --notes \\\n+\t\t>output &&\n+\tgrep xyzzy output &&\n+\t! grep alternate output\n+'\n+\n test_expect_success 'create -m notes (setup)' '\n \t: > a5 &&\n \tgit add a5 &&\n-- \n1.7.4.2.7.g9407\n"},{"id":"164633","messageId":"201103292344.26249.johan@herland.net","threadId":"26906","inReplyTo":"20110329205307.GA30959@sigill.intra.peff.net","subject":"Re: [PATCH] revision.c: introduce --notes-ref= to use one notes ref only","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2011-03-29T21:44:26Z","receivedAt":"2011-03-29T21:44:26Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Tuesday 29 March 2011, Jeff King wrote:\n> Here's the series I ended up with. Getting the refactoring just right\n> turned out to be non-trivial, but between several attempts and some\n> tests, I think the end result is correct. Hopefully the breakdown of the\n> changes into small patches helps make it easy to review.\n> \n>   [1/6]: notes: make expand_notes_ref globally accessible\n>   [2/6]: revision.c: refactor notes ref expansion\n>   [3/6]: notes: refactor display notes extra refs field\n>   [4/6]: notes: refactor display notes default handling\n>   [5/6]: revision.c: support --notes command-line option\n>   [6/6]: revision.c: make --no-notes reset --notes list\n\nIndeed, the whole series looks good to me.\n\nAcked-by: Johan Herland <johan@herland.net>\n\n\nThanks! :)\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"164634","messageId":"7vfwq5poua.fsf@alter.siamese.dyndns.org","threadId":"26906","inReplyTo":"20110329205727.GD23466@sigill.intra.peff.net","subject":"Re: [PATCH 4/6] notes: refactor display notes default handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-29T22:02:37Z","receivedAt":"2011-03-29T22:02:37Z","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> This is in preparation for more notes-related revision\n> command-line options.\n>\n> The \"suppress_default_notes\" option is renamed to\n> \"use_default_notes\", and is now a tri-state with values less\n> than one indicating \"not set\".  If the value is \"not set\",\n> then we show default refs if and only if no other refs were\n> given.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ...\n> diff --git a/revision.c b/revision.c\n> index 24b89eb..315a7f4 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -1368,10 +1370,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n>  \t} else if (!strcmp(arg, \"--show-notes\")) {\n>  \t\trevs->show_notes = 1;\n>  \t\trevs->show_notes_given = 1;\n> +\t\trevs->notes_opt.use_default_notes = 1;\n>  \t} else if (!prefixcmp(arg, \"--show-notes=\")) {\n>  \t\tstruct strbuf buf = STRBUF_INIT;\n>  \t\trevs->show_notes = 1;\n>  \t\trevs->show_notes_given = 1;\n> +\t\tif (revs->notes_opt.use_default_notes < 0)\n> +\t\t\trevs->notes_opt.use_default_notes = 1;\n\nI didn't quite get this hunk while reading the series, but it is to keep\nthe current semantics that --show-notes=foo means \"show foo in addition to\nwhatever the default one we are going to show anyway\", and you will be\nfixing that with a saner --notes in a later patch in the series.\n\nI like it; thanks.\n"},{"id":"164635","messageId":"7v8vvxpor9.fsf@alter.siamese.dyndns.org","threadId":"26906","inReplyTo":"20110329205942.GF23466@sigill.intra.peff.net","subject":"Re: [PATCH 6/6] revision.c: make --no-notes reset --notes list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-29T22:04:26Z","receivedAt":"2011-03-29T22:04:26Z","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> Technically this is a regression for:\n>\n>   git log --show-notes=foo --no-notes --show-notes=bar\n>\n> which used to show both. But I consider that behavior useless and crazy,\n> and since the --no-notes actually did nothing there, why would anyone\n> have been using it?\n\nI suspect that nobody would complain; we could even ridicule whoever claim\nto have been depending on the buggy behaviour.\n\n    It was a _bug_ that 'git log' still showed A when it is ran with\n    '--show-notes=A --no-notes --show-notes=B'\n\nmay even be more appropriate description of this change.\n\nThanks for bringing in a bit more sanity to the world.\n"},{"id":"164637","messageId":"201103300018.55371.johan@herland.net","threadId":"26906","inReplyTo":"201103292344.26249.johan@herland.net","subject":"[PATCH 7/6] log/pretty-options: Document --[no-]notes and deprecate old notes options","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2011-03-29T22:18:55Z","receivedAt":"2011-03-29T22:18:55Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"Document the behavior or the new --notes, --notes=<ref> and --no-notes\noptions, and list --show-notes[=<ref>] and --[no-]standard-notes options\nas deprecated.\n\nSigned-off-by: Johan Herland <johan@herland.net>\n---\n\nOn Tuesday 29 March 2011, Johan Herland wrote:\n> On Tuesday 29 March 2011, Jeff King wrote:\n> > Here's the series I ended up with. Getting the refactoring just right\n> > turned out to be non-trivial, but between several attempts and some\n> > tests, I think the end result is correct. Hopefully the breakdown of\n> > the changes into small patches helps make it easy to review.\n> > \n> >   [1/6]: notes: make expand_notes_ref globally accessible\n> >   [2/6]: revision.c: refactor notes ref expansion\n> >   [3/6]: notes: refactor display notes extra refs field\n> >   [4/6]: notes: refactor display notes default handling\n> >   [5/6]: revision.c: support --notes command-line option\n> >   [6/6]: revision.c: make --no-notes reset --notes list\n> \n> Indeed, the whole series looks good to me.\n> \n> Acked-by: Johan Herland <johan@herland.net>\n\nAnd here's some documentation to go on top.\n\n\nHave fun! :)\n\n...Johan\n\n Documentation/git-log.txt        |    4 ++--\n Documentation/pretty-options.txt |   32 ++++++++++++++++++++++----------\n 2 files changed, 24 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex ff41784..0c1c10e 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -177,9 +177,9 @@ May be an unabbreviated ref name or a glob and may be specified\n multiple times.  A warning will be issued for refs that do not exist,\n but a glob that does not match any refs is silently ignored.\n +\n-This setting can be disabled by the `--no-standard-notes` option,\n+This setting can be disabled by the `--no-notes` option,\n overridden by the 'GIT_NOTES_DISPLAY_REF' environment variable,\n-and supplemented by the `--show-notes` option.\n+and supplemented by the `--notes=<ref>` option.\n \n Author\n ------\ndiff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\nindex 50923e2..a9b399b 100644\n--- a/Documentation/pretty-options.txt\n+++ b/Documentation/pretty-options.txt\n@@ -30,19 +30,31 @@ people using 80-column terminals.\n \tpreferred by the user.  For non plumbing commands this\n \tdefaults to UTF-8.\n \n---no-notes::\n---show-notes[=<ref>]::\n+--notes[=<ref>]::\n \tShow the notes (see linkgit:git-notes[1]) that annotate the\n \tcommit, when showing the commit log message.  This is the default\n \tfor `git log`, `git show` and `git whatchanged` commands when\n-\tthere is no `--pretty`, `--format` nor `--oneline` option is\n-\tgiven on the command line.\n+\tthere is no `--pretty`, `--format` nor `--oneline` option given\n+\ton the command line.\n++\n+By default, the notes shown are from the notes refs listed in the\n+'core.notesRef' and 'notes.displayRef' variables (or corresponding\n+environment overrides). See linkgit:git-config[1] for more details.\n +\n-With an optional argument, add this ref to the list of notes.  The ref\n-is taken to be in `refs/notes/` if it is not qualified.\n+With an optional '<ref>' argument, add this notes ref to the list of\n+notes refs to be shown. The ref is taken to be in `refs/notes/` if it\n+is not qualified.\n \n+--no-notes::\n+\tDo not show notes. This negates the above `--notes` option, by\n+\tresetting the list of notes refs from which notes are shown.\n+\tThis can be combined with the above `--notes` option to control\n+\texactly which notes refs are shown. E.g. \"--notes=foo\" will show\n+\tnotes, both from the default notes ref, and from \"refs/notes/foo\",\n+\twhile \"--no-notes --notes=foo\" will only show notes from\n+\t\"refs/notes/foo\".\n+\n+--show-notes[=<ref>]::\n --[no-]standard-notes::\n-\tEnable or disable populating the notes ref list from the\n-\t'core.notesRef' and 'notes.displayRef' variables (or\n-\tcorresponding environment overrides).  Enabled by default.\n-\tSee linkgit:git-config[1].\n+\tThese options are deprecated. Use the above --notes/--no-notes\n+\toptions instead.\n-- \n1.7.4\n"},{"id":"164646","messageId":"20110330002206.GB1161@sigill.intra.peff.net","threadId":"26906","inReplyTo":"201103300018.55371.johan@herland.net","subject":"Re: [PATCH 7/6] log/pretty-options: Document --[no-]notes and deprecate old notes options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-30T00:22:06Z","receivedAt":"2011-03-30T00:22:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 30, 2011 at 12:18:55AM +0200, Johan Herland wrote:\n\n> > On Tuesday 29 March 2011, Jeff King wrote:\n> > > Here's the series I ended up with. Getting the refactoring just right\n> > > turned out to be non-trivial, but between several attempts and some\n> > > tests, I think the end result is correct. Hopefully the breakdown of\n> > > the changes into small patches helps make it easy to review.\n> > > \n> > >   [1/6]: notes: make expand_notes_ref globally accessible\n> > >   [2/6]: revision.c: refactor notes ref expansion\n> > >   [3/6]: notes: refactor display notes extra refs field\n> > >   [4/6]: notes: refactor display notes default handling\n> > >   [5/6]: revision.c: support --notes command-line option\n> > >   [6/6]: revision.c: make --no-notes reset --notes list\n> \n> And here's some documentation to go on top.\n\nThanks. I meant to document at the end, but somehow after the fifteenth\n\"rebase -i\" I forgot. :)\n\n> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\n> index ff41784..0c1c10e 100644\n> --- a/Documentation/git-log.txt\n> +++ b/Documentation/git-log.txt\n> @@ -177,9 +177,9 @@ May be an unabbreviated ref name or a glob and may be specified\n>  multiple times.  A warning will be issued for refs that do not exist,\n>  but a glob that does not match any refs is silently ignored.\n>  +\n> -This setting can be disabled by the `--no-standard-notes` option,\n> +This setting can be disabled by the `--no-notes` option,\n>  overridden by the 'GIT_NOTES_DISPLAY_REF' environment variable,\n> -and supplemented by the `--show-notes` option.\n> +and supplemented by the `--notes=<ref>` option.\n\nThis is not really \"supplemented\" any more, but rather \"overridden\", I\nthink.\n\n> ---no-notes::\n> ---show-notes[=<ref>]::\n> +--notes[=<ref>]::\n>  \tShow the notes (see linkgit:git-notes[1]) that annotate the\n>  \tcommit, when showing the commit log message.  This is the default\n>  \tfor `git log`, `git show` and `git whatchanged` commands when\n> -\tthere is no `--pretty`, `--format` nor `--oneline` option is\n> -\tgiven on the command line.\n> +\tthere is no `--pretty`, `--format` nor `--oneline` option given\n> +\ton the command line.\n> ++\n> +By default, the notes shown are from the notes refs listed in the\n> +'core.notesRef' and 'notes.displayRef' variables (or corresponding\n> +environment overrides). See linkgit:git-config[1] for more details.\n>  +\n> -With an optional argument, add this ref to the list of notes.  The ref\n> -is taken to be in `refs/notes/` if it is not qualified.\n> +With an optional '<ref>' argument, add this notes ref to the list of\n> +notes refs to be shown. The ref is taken to be in `refs/notes/` if it\n> +is not qualified.\n\nThis is somewhat vague about the interaction, in particular that\n\"--notes=<ref>\" will disable default notes from being shown unless\n\"--notes\" is also given.\n\n> +--no-notes::\n> +\tDo not show notes. This negates the above `--notes` option, by\n> +\tresetting the list of notes refs from which notes are shown.\n> +\tThis can be combined with the above `--notes` option to control\n> +\texactly which notes refs are shown. E.g. \"--notes=foo\" will show\n> +\tnotes, both from the default notes ref, and from \"refs/notes/foo\",\n> +\twhile \"--no-notes --notes=foo\" will only show notes from\n> +\t\"refs/notes/foo\".\n\nThis example is wrong. \"--notes=foo\" will show _only_ foo. A few\nexamples:\n\n  --notes=foo ;# only foo\n  --notes --notes=foo ;# foo and default\n  --notes=foo --notes=bar --notes --no-notes --notes=foo ;# only foo\n\nMake sense?\n\n-Peff\n"},{"id":"164649","messageId":"201103300257.19921.johan@herland.net","threadId":"26906","inReplyTo":"20110330002206.GB1161@sigill.intra.peff.net","subject":"[PATCH 7/6] log/pretty-options: Document --[no-]notes and deprecate old notes options","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2011-03-30T00:57:19Z","receivedAt":"2011-03-30T00:57:19Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"Document the behavior or the new --notes, --notes=<ref> and --no-notes\noptions, and list --show-notes[=<ref>] and --[no-]standard-notes options\nas deprecated.\n\nSigned-off-by: Johan Herland <johan@herland.net>\n---\n\nOn Wednesday 30 March 2011, Jeff King wrote:\n> On Wed, Mar 30, 2011 at 12:18:55AM +0200, Johan Herland wrote:\n> > And here's some documentation to go on top.\n> [...]\n> This example is wrong. \"--notes=foo\" will show _only_ foo. A few\n> examples:\n> \n>   --notes=foo ;# only foo\n>   --notes --notes=foo ;# foo and default\n>   --notes=foo --notes=bar --notes --no-notes --notes=foo ;# only foo\n> \n> Make sense?\n\nYeah, I really screwed that up. Hope this version is better. :)\n\n...Johan\n\n\n Documentation/git-log.txt        |    4 ++--\n Documentation/pretty-options.txt |   35 +++++++++++++++++++++++++----------\n 2 files changed, 27 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex ff41784..ab3757b 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -177,9 +177,9 @@ May be an unabbreviated ref name or a glob and may be specified\n multiple times.  A warning will be issued for refs that do not exist,\n but a glob that does not match any refs is silently ignored.\n +\n-This setting can be disabled by the `--no-standard-notes` option,\n+This setting can be disabled by the `--no-notes` option,\n overridden by the 'GIT_NOTES_DISPLAY_REF' environment variable,\n-and supplemented by the `--show-notes` option.\n+and overridden by the `--notes=<ref>` option.\n \n Author\n ------\ndiff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\nindex 50923e2..d5c9772 100644\n--- a/Documentation/pretty-options.txt\n+++ b/Documentation/pretty-options.txt\n@@ -30,19 +30,34 @@ people using 80-column terminals.\n \tpreferred by the user.  For non plumbing commands this\n \tdefaults to UTF-8.\n \n---no-notes::\n---show-notes[=<ref>]::\n+--notes[=<ref>]::\n \tShow the notes (see linkgit:git-notes[1]) that annotate the\n \tcommit, when showing the commit log message.  This is the default\n \tfor `git log`, `git show` and `git whatchanged` commands when\n-\tthere is no `--pretty`, `--format` nor `--oneline` option is\n-\tgiven on the command line.\n+\tthere is no `--pretty`, `--format` nor `--oneline` option given\n+\ton the command line.\n++\n+By default, the notes shown are from the notes refs listed in the\n+'core.notesRef' and 'notes.displayRef' variables (or corresponding\n+environment overrides). See linkgit:git-config[1] for more details.\n++\n+With an optional '<ref>' argument, show this notes ref instead of the\n+default notes ref(s). The ref is taken to be in `refs/notes/` if it\n+is not qualified.\n +\n-With an optional argument, add this ref to the list of notes.  The ref\n-is taken to be in `refs/notes/` if it is not qualified.\n+Multiple --notes options can be combined to control which notes are\n+being displayed. Examples: \"--notes=foo\" will show only notes from\n+\"refs/notes/foo\"; \"--notes=foo --notes\" will show both notes from\n+\"refs/notes/foo\" and from the default notes ref(s).\n \n+--no-notes::\n+\tDo not show notes. This negates the above `--notes` option, by\n+\tresetting the list of notes refs from which notes are shown.\n+\tOptions are parsed in the order given on the command line, so e.g.\n+\t\"--notes --notes=foo --no-notes --notes=bar\" will only show notes\n+\tfrom \"refs/notes/bar\".\n+\n+--show-notes[=<ref>]::\n --[no-]standard-notes::\n-\tEnable or disable populating the notes ref list from the\n-\t'core.notesRef' and 'notes.displayRef' variables (or\n-\tcorresponding environment overrides).  Enabled by default.\n-\tSee linkgit:git-config[1].\n+\tThese options are deprecated. Use the above --notes/--no-notes\n+\toptions instead.\n-- \n1.7.4\n"},{"id":"164655","messageId":"20110330033205.GB18157@sigill.intra.peff.net","threadId":"26906","inReplyTo":"201103300257.19921.johan@herland.net","subject":"Re: [PATCH 7/6] log/pretty-options: Document --[no-]notes and deprecate old notes options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-30T03:32:05Z","receivedAt":"2011-03-30T03:32:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 30, 2011 at 02:57:19AM +0200, Johan Herland wrote:\n\n> On Wednesday 30 March 2011, Jeff King wrote:\n> > On Wed, Mar 30, 2011 at 12:18:55AM +0200, Johan Herland wrote:\n> > > And here's some documentation to go on top.\n> > [...]\n> > This example is wrong. \"--notes=foo\" will show _only_ foo. A few\n> > examples:\n> > \n> >   --notes=foo ;# only foo\n> >   --notes --notes=foo ;# foo and default\n> >   --notes=foo --notes=bar --notes --no-notes --notes=foo ;# only foo\n> > \n> > Make sense?\n> \n> Yeah, I really screwed that up. Hope this version is better. :)\n\nYes, it looks good to me. Thanks.\n\nAcked-by: Jeff King <peff@peff.net>\n\n-Peff\n"}]}