{"thread":{"id":"40406","subject":"[PATCH] notes: document behavior of --ref and --notes DWIMery","startedAt":"2015-09-22T20:24:36Z","lastAt":"2015-09-22T21:23:09Z","messageCount":5,"participants":["Jacob Keller","Junio C Hamano","Keller, Jacob E"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"270523","messageId":"1442953476-24537-1-git-send-email-jacob.e.keller@intel.com","threadId":"40406","inReplyTo":null,"subject":"[PATCH] notes: document behavior of --ref and --notes DWIMery","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-09-22T20:24:36Z","receivedAt":"2015-09-22T20:24:36Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nThe --notes and --ref parameter for selecting which notes ref to operate\non are based off of expand_notes_ref functionality. The documentation\nmentioned that an unqualified ref argument would be taken as under\n`refs/notes/`. However, this does not clearly indicate that\n`refs/heads/master` will expand to `refs/notes/refs/heads/master`, so\ndocument this behavior.\n\nAdd a further test for the expected behavior of git notes --ref\nrefs/heads/master get-ref as well, to ensure future patches do not break\nthis assumption.\n\nSigned-off-by: Jacob Keller \n---\n Documentation/git-notes.txt | 4 +++-\n Documentation/pretty-options.txt | 5 +++--\n t/t3301-notes.sh | 6 ++++++\n 3 files changed, 12 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt\nindex a9a916f360ec..2ed9c16a1aac 100644\n--- a/Documentation/git-notes.txt\n+++ b/Documentation/git-notes.txt\n@@ -162,7 +162,9 @@ OPTIONS\n --ref ::\n \tManipulate the notes tree in . This overrides\n \t'GIT_NOTES_REF' and the \"core.notesRef\" configuration. The ref\n-\tis taken to be in `refs/notes/` if it is not qualified.\n+\tis taken to be in `refs/notes/` even if it is qualified under some\n+\tother location; in other words, `refs/heads/master` will be expanded\n+\tto `refs/notes/refs/heads/master`.\n \n --ignore-missing::\n \tDo not consider it an error to request removing notes from an\ndiff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\nindex 8d6c5cec4c5e..abcb787e7149 100644\n--- a/Documentation/pretty-options.txt\n+++ b/Documentation/pretty-options.txt\n@@ -55,8 +55,9 @@ By default, the notes shown are from the notes refs listed in the\n environment overrides). See linkgit:git-config[1] for more details.\n +\n With an optional '' 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+default notes ref(s). The ref is taken to be in `refs/notes/` even if it is\n+qualified under some other location; in other words, `refs/heads/master`\n+will be expanded to `refs/notes/refs/heads/master`.\n +\n Multiple --notes options can be combined to control which notes are\n being displayed. Examples: \"--notes=foo\" will show only notes from\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 06b45847c147..2d200fdf36c6 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -1132,6 +1132,12 @@ test_expect_success 'git notes copy diagnoses too many or too few parameters' '\n \ttest_must_fail git notes copy one two three\n '\n \n+test_expect_success 'git notes get-ref expands refs/heads/master to refs/notes/refs/heads/master' '\n+\ttest_unconfig core.notesRef &&\n+\tsane_unset GIT_NOTES_REF &&\n+\ttest \"$(git notes --ref=refs/heads/master get-ref)\" = \"refs/notes/refs/heads/master\"\n+'\n+\n test_expect_success 'git notes get-ref (no overrides)' '\n \ttest_unconfig core.notesRef &&\n \tsane_unset GIT_NOTES_REF &&\n-- \n2.6.0.rc3.238.gc07a1e8\n"},{"id":"270525","messageId":"xmqq8u7y2nnq.fsf@gitster.mtv.corp.google.com","threadId":"40406","inReplyTo":"1442953476-24537-1-git-send-email-jacob.e.keller@intel.com","subject":"Re: [PATCH] notes: document behavior of --ref and --notes DWIMery","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-22T20:40:25Z","receivedAt":"2015-09-22T20:40:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> From: Jacob Keller \n>\n> The --notes and --ref parameter for selecting which notes ref to operate\n> on are based off of expand_notes_ref functionality. The documentation\n> mentioned that an unqualified ref argument would be taken as under\n> `refs/notes/`. However, this does not clearly indicate that\n> `refs/heads/master` will expand to `refs/notes/refs/heads/master`, so\n> document this behavior.\n>\n> Add a further test for the expected behavior of git notes --ref\n> refs/heads/master get-ref as well, to ensure future patches do not break\n> this assumption.\n>\n> Signed-off-by: Jacob Keller \n> ---\n\nLooks OK to a cursory read, but I find \"even if it is qualified\nunder some other location\" a bit tiring to read without adding much\nvalue. To readers who consider \"other\" in that phrase to be clear\nenough (i.e. \"location other than refs/notes\"), it is totally\nredundant. To other readers who feel \"other\" in that phrase to be\nunder qualified (i.e. \"location other than what???\"), it is not\ninformative enough. Middle-ground readers who would not know if\n\"refs/a\" is inside or outside some \"other\" location, it is confusing.\n\nAfter all, \"a/b\" is qualified under some location (i.e. a/) other\nthan \"refs/notes/\", and it does mean \"refs/notes/a/b\".\n\nHow about phrasing it totally differently?\n\n\tThe ref specifies the full refname when it begins with\n\t`refs/notes/`; otherwise `ref/notes/` is prefixed to form a\n\tfull name of the ref.\n\nI think that would remove the need to illustrate with concrete\nexamples like refs/heads/blah.\n"},{"id":"270529","messageId":"1442955762.26280.49.camel@intel.com","threadId":"40406","inReplyTo":"xmqq8u7y2nnq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] notes: document behavior of --ref and --notes DWIMery","fromName":"Keller, Jacob E","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-09-22T21:02:42Z","receivedAt":"2015-09-22T21:02:42Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Tue, 2015-09-22 at 13:40 -0700, Junio C Hamano wrote:\n> Jacob Keller writes:\n> \n> > From: Jacob Keller \n> > \n> > The --notes and --ref parameter for selecting which notes ref to\n> > operate\n> > on are based off of expand_notes_ref functionality. The\n> > documentation\n> > mentioned that an unqualified ref argument would be taken as under\n> > `refs/notes/`. However, this does not clearly indicate that\n> > `refs/heads/master` will expand to `refs/notes/refs/heads/master`,\n> > so\n> > document this behavior.\n> > \n> > Add a further test for the expected behavior of git notes --ref\n> > refs/heads/master get-ref as well, to ensure future patches do not\n> > break\n> > this assumption.\n> > \n> > Signed-off-by: Jacob Keller \n> > ---\n> \n> Looks OK to a cursory read, but I find \"even if it is qualified\n> under some other location\" a bit tiring to read without adding much\n> value. To readers who consider \"other\" in that phrase to be clear\n> enough (i.e. \"location other than refs/notes\"), it is totally\n> redundant. To other readers who feel \"other\" in that phrase to be\n> under qualified (i.e. \"location other than what???\"), it is not\n> informative enough. Middle-ground readers who would not know if\n> \"refs/a\" is inside or outside some \"other\" location, it is confusing.\n> \n> After all, \"a/b\" is qualified under some location (i.e. a/) other\n> than \"refs/notes/\", and it does mean \"refs/notes/a/b\".\n> \n> How about phrasing it totally differently?\n> \n> \tThe ref specifies the full refname when it begins with\n> \t`refs/notes/`; otherwise `ref/notes/` is prefixed to form a\n> \tfull name of the ref.\n> \n> I think that would remove the need to illustrate with concrete\n> examples like refs/heads/blah.\n> \n\nYes, let's go with that.\n\nRegards,\nJake"},{"id":"270530","messageId":"1442955811.26280.50.camel@intel.com","threadId":"40406","inReplyTo":"xmqq8u7y2nnq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] notes: document behavior of --ref and --notes DWIMery","fromName":"Keller, Jacob E","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-09-22T21:03:31Z","receivedAt":"2015-09-22T21:03:31Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Tue, 2015-09-22 at 13:40 -0700, Junio C Hamano wrote:\n> How about phrasing it totally differently?\n> \n> \tThe ref specifies the full refname when it begins with\n> \t`refs/notes/`; otherwise `ref/notes/` is prefixed to form a\n> \tfull name of the ref.\n> \n> I think that would remove the need to illustrate with concrete\n> examples like refs/heads/blah.\n> \n\nWait, what about the DWIM of notes/ goes to refs/notes/..\ndo we need to explain that here? it might seem that \"notes/foo\" ends up\nas \"refs/notes/notes/foo\" which is not really what we mean.\n\nRegards,\nJake"},{"id":"270531","messageId":"xmqqpp1a1742.fsf@gitster.mtv.corp.google.com","threadId":"40406","inReplyTo":"1442955811.26280.50.camel@intel.com","subject":"Re: [PATCH] notes: document behavior of --ref and --notes DWIMery","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-22T21:23:09Z","receivedAt":"2015-09-22T21:23:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Keller, Jacob E\" writes:\n\n> On Tue, 2015-09-22 at 13:40 -0700, Junio C Hamano wrote:\n>> How about phrasing it totally differently?\n>> \n>> \tThe ref specifies the full refname when it begins with\n>> \t`refs/notes/`; otherwise `ref/notes/` is prefixed to form a\n>> \tfull name of the ref.\n>> \n>> I think that would remove the need to illustrate with concrete\n>> examples like refs/heads/blah.\n>> \n>\n> Wait, what about the DWIM of notes/ goes to refs/notes/..\n> do we need to explain that here?\n\nYeah, throw that in, too.\n\n\t... when it begins with `refs/notes/`; when it begins with\n\t`notes/`, `refs/` and otherwise `refs/notes/` is prefixed to\n\tform a full name of the ref.\n\nWe could emphasize the end result by ending the sentence like so:\n\n\t... form a full name of the ref that is under `refs/notes/`.\n\nif you wanted to. I am neutral (I do not think it would hurt, but I\ndo not think it adds much clarity).\n\nThanks.\n"}]}