{"thread":{"id":"27403","subject":"[PATCH 0/3] \"git notes remove\" updates","startedAt":"2011-05-19T00:14:18Z","lastAt":"2011-05-19T18:47:03Z","messageCount":17,"participants":["Junio C Hamano","Michael J Gruber","Johan Herland","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"168177","messageId":"1305764061-21303-1-git-send-email-gitster@pobox.com","threadId":"27403","inReplyTo":null,"subject":"[PATCH 0/3] \"git notes remove\" updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-19T00:14:18Z","receivedAt":"2011-05-19T00:14:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I wanted to do a bit better than\n\n\tfor sha1 in ... list of old commit objects\n\tdo\n\t\tgit notes --ref refs/notes/amlog remove $sha1\n\tdone\n\nto remove old entries in my \"where did this commit come from\" database.\n\nJunio C Hamano (3):\n  notes remove: allow removing more than one\n  notes remove: --missing-ok\n  notes remove: --stdin reads from the standard input\n\n Documentation/git-notes.txt |   16 ++++++++--\n builtin/notes.c             |   65 ++++++++++++++++++++++++++-------------\n t/t3301-notes.sh            |   71 +++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 127 insertions(+), 25 deletions(-)\n\n-- \n1.7.5.1.414.gb4910\n"},{"id":"168178","messageId":"1305764061-21303-2-git-send-email-gitster@pobox.com","threadId":"27403","inReplyTo":"1305764061-21303-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/3] notes remove: allow removing more than one","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-19T00:14:19Z","receivedAt":"2011-05-19T00:14:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"While \"xargs -n1 git notes rm\" is certainly a possible way to remove notes\nfrom many objects, this would create one notes \"commit\" per removal, which\nis not quite suitable for seasonal housekeeping.\n\nAllow taking more than one on the command line, and record their removal\nas a single atomic event if everthing goes well.\n\nEven though the old code insisted that \"git notes rm\" must be given only\none object (or zero, in which case it would default to HEAD), this\ncondition was not tested. Add tests to handle the new case where we feed\nmultiple objects, and also make sure if there is a bad input, no change\nis recorded.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/git-notes.txt |    7 +++--\n builtin/notes.c             |   47 ++++++++++++++++++++++--------------------\n t/t3301-notes.sh            |   19 +++++++++++++++++\n 3 files changed, 48 insertions(+), 25 deletions(-)\n\ndiff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt\nindex 913ecd8..e2e1c4c 100644\n--- a/Documentation/git-notes.txt\n+++ b/Documentation/git-notes.txt\n@@ -17,7 +17,7 @@ SYNOPSIS\n 'git notes' merge [-v | -q] [-s <strategy> ] <notes_ref>\n 'git notes' merge --commit [-v | -q]\n 'git notes' merge --abort [-v | -q]\n-'git notes' remove [<object>]\n+'git notes' remove [<object>...]\n 'git notes' prune [-n | -v]\n 'git notes' get-ref\n \n@@ -106,8 +106,9 @@ When done, the user can either finalize the merge with\n 'git notes merge --abort'.\n \n remove::\n-\tRemove the notes for a given object (defaults to HEAD).\n-\tThis is equivalent to specifying an empty note message to\n+\tRemove the notes for given objects (defaults to HEAD). When\n+\tgiving zero or one object from the command line, this is\n+\tequivalent to specifying an empty note message to\n \tthe `edit` subcommand.\n \n prune::\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 1fb1f73..30cee0f 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -29,7 +29,7 @@ static const char * const git_notes_usage[] = {\n \t\"git notes [--ref <notes_ref>] merge [-v | -q] [-s <strategy> ] <notes_ref>\",\n \t\"git notes merge --commit [-v | -q]\",\n \t\"git notes merge --abort [-v | -q]\",\n-\t\"git notes [--ref <notes_ref>] remove [<object>]\",\n+\t\"git notes [--ref <notes_ref>] remove [<object>...]\",\n \t\"git notes [--ref <notes_ref>] prune [-n | -v]\",\n \t\"git notes [--ref <notes_ref>] get-ref\",\n \tNULL\n@@ -953,40 +953,43 @@ static int merge(int argc, const char **argv, const char *prefix)\n \treturn result < 0; /* return non-zero on conflicts */\n }\n \n+static int remove_one_note(struct notes_tree *t, const char *name)\n+{\n+\tint status;\n+\tunsigned char sha1[20];\n+\tif (get_sha1(name, sha1))\n+\t\treturn error(_(\"Failed to resolve '%s' as a valid ref.\"), name);\n+\tstatus = remove_note(t, sha1);\n+\tif (status)\n+\t\tfprintf(stderr, _(\"Object %s has no note\\n\"), name);\n+\telse\n+\t\tfprintf(stderr, _(\"Removing note for object %s\\n\"), name);\n+\treturn status;\n+}\n+\n static int remove_cmd(int argc, const char **argv, const char *prefix)\n {\n \tstruct option options[] = {\n \t\tOPT_END()\n \t};\n-\tconst char *object_ref;\n \tstruct notes_tree *t;\n-\tunsigned char object[20];\n-\tint retval;\n+\tint retval = 0;\n \n \targc = parse_options(argc, argv, prefix, options,\n \t\t\t     git_notes_remove_usage, 0);\n \n-\tif (1 < argc) {\n-\t\terror(_(\"too many parameters\"));\n-\t\tusage_with_options(git_notes_remove_usage, options);\n-\t}\n-\n-\tobject_ref = argc ? argv[0] : \"HEAD\";\n-\n-\tif (get_sha1(object_ref, object))\n-\t\tdie(_(\"Failed to resolve '%s' as a valid ref.\"), object_ref);\n-\n \tt = init_notes_check(\"remove\");\n \n-\tretval = remove_note(t, object);\n-\tif (retval)\n-\t\tfprintf(stderr, _(\"Object %s has no note\\n\"), sha1_to_hex(object));\n-\telse {\n-\t\tfprintf(stderr, _(\"Removing note for object %s\\n\"),\n-\t\t\tsha1_to_hex(object));\n-\n-\t\tcommit_notes(t, \"Notes removed by 'git notes remove'\");\n+\tif (!argc) {\n+\t\tretval = remove_one_note(t, \"HEAD\");\n+\t} else {\n+\t\twhile (*argv) {\n+\t\t\tretval |= remove_one_note(t, *argv);\n+\t\t\targv++;\n+\t\t}\n \t}\n+\tif (!retval)\n+\t\tcommit_notes(t, \"Notes removed by 'git notes remove'\");\n \tfree_notes(t);\n \treturn retval;\n }\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 28e17c8..6a6daa9 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -435,6 +435,25 @@ test_expect_success 'removing non-existing note should not create new commit' '\n \ttest_cmp before_commit after_commit\n '\n \n+test_expect_success 'removing more than one' '\n+\tbefore=$(git rev-parse --verify refs/notes/commits) &&\n+\ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n+\tgit notes remove HEAD^^ HEAD^^^ &&\n+\tgit diff --name-only refs/notes/commits^ refs/notes/commits >actual &&\n+\ttest 2 = $(wc -l <actual) &&\n+\tgit ls-tree -r --name-only refs/notes/commits >actual &&\n+\t>empty &&\n+\ttest_cmp empty actual\n+'\n+\n+test_expect_success 'removing is atomic' '\n+\tbefore=$(git rev-parse --verify refs/notes/commits) &&\n+\ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n+\ttest_must_fail git notes remove HEAD^^ HEAD^^^ HEAD^ &&\n+\tafter=$(git rev-parse --verify refs/notes/commits) &&\n+\ttest \"$before\" = \"$after\"\n+'\n+\n test_expect_success 'list notes with \"git notes list\"' '\n \tgit notes list > output &&\n \ttest_cmp expect output\n-- \n1.7.5.1.414.gb4910\n"},{"id":"168180","messageId":"1305764061-21303-3-git-send-email-gitster@pobox.com","threadId":"27403","inReplyTo":"1305764061-21303-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/3] notes remove: --missing-ok","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-19T00:14:20Z","receivedAt":"2011-05-19T00:14:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Depending on the application, it is not necessarily an error for an object\nto lack a note, especially if the only thing the caller wants to make sure\nis that notes are cleared for an object.  By passing this option from the\ncommand line, the \"git notes remove\" command considers it a success if the\nobject did not have any note to begin with.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * It may be that this should have been the default, I suspect.\n---\n Documentation/git-notes.txt |    6 +++++-\n builtin/notes.c             |   14 ++++++++++----\n t/t3301-notes.sh            |   19 +++++++++++++++++++\n 3 files changed, 34 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt\nindex e2e1c4c..6b92060 100644\n--- a/Documentation/git-notes.txt\n+++ b/Documentation/git-notes.txt\n@@ -17,7 +17,7 @@ SYNOPSIS\n 'git notes' merge [-v | -q] [-s <strategy> ] <notes_ref>\n 'git notes' merge --commit [-v | -q]\n 'git notes' merge --abort [-v | -q]\n-'git notes' remove [<object>...]\n+'git notes' remove [--missing-ok] [<object>...]\n 'git notes' prune [-n | -v]\n 'git notes' get-ref\n \n@@ -155,6 +155,10 @@ OPTIONS\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 \n+--missing-ok::\n+\tDo not consider it an error to request removing notes from an\n+\tobject that does not have notes attached to it.\n+\n -n::\n --dry-run::\n \tDo not remove anything; just report the object names whose notes\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 30cee0f..ca045f8 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -953,7 +953,9 @@ static int merge(int argc, const char **argv, const char *prefix)\n \treturn result < 0; /* return non-zero on conflicts */\n }\n \n-static int remove_one_note(struct notes_tree *t, const char *name)\n+#define MISSING_OK 1\n+\n+static int remove_one_note(struct notes_tree *t, const char *name, unsigned flag)\n {\n \tint status;\n \tunsigned char sha1[20];\n@@ -964,12 +966,16 @@ static int remove_one_note(struct notes_tree *t, const char *name)\n \t\tfprintf(stderr, _(\"Object %s has no note\\n\"), name);\n \telse\n \t\tfprintf(stderr, _(\"Removing note for object %s\\n\"), name);\n-\treturn status;\n+\treturn (flag & MISSING_OK) ? 0 : status;\n }\n \n static int remove_cmd(int argc, const char **argv, const char *prefix)\n {\n+\tunsigned flag = 0;\n \tstruct option options[] = {\n+\t\tOPT_BIT(0, \"missing-ok\", &flag,\n+\t\t\t\"attempt to remove non-existent note is not an error\",\n+\t\t\tMISSING_OK),\n \t\tOPT_END()\n \t};\n \tstruct notes_tree *t;\n@@ -981,10 +987,10 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)\n \tt = init_notes_check(\"remove\");\n \n \tif (!argc) {\n-\t\tretval = remove_one_note(t, \"HEAD\");\n+\t\tretval = remove_one_note(t, \"HEAD\", flag);\n \t} else {\n \t\twhile (*argv) {\n-\t\t\tretval |= remove_one_note(t, *argv);\n+\t\t\tretval |= remove_one_note(t, *argv, flag);\n \t\t\targv++;\n \t\t}\n \t}\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 6a6daa9..bdd4036 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -454,6 +454,25 @@ test_expect_success 'removing is atomic' '\n \ttest \"$before\" = \"$after\"\n '\n \n+test_expect_success 'removing with --missing-ok' '\n+\tbefore=$(git rev-parse --verify refs/notes/commits) &&\n+\ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n+\tgit notes remove --missing-ok HEAD^^ HEAD^^^ HEAD^ &&\n+\tgit diff --name-only refs/notes/commits^ refs/notes/commits >actual &&\n+\ttest 2 = $(wc -l <actual) &&\n+\tgit ls-tree -r --name-only refs/notes/commits >actual &&\n+\t>empty &&\n+\ttest_cmp empty actual\n+'\n+\n+test_expect_success 'removing with --missing-ok but bogus ref' '\n+\tbefore=$(git rev-parse --verify refs/notes/commits) &&\n+\ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n+\ttest_must_fail git notes remove --missing-ok HEAD^^ HEAD^^^ NO-SUCH-COMMIT &&\n+\tafter=$(git rev-parse --verify refs/notes/commits) &&\n+\ttest \"$before\" = \"$after\"\n+'\n+\n test_expect_success 'list notes with \"git notes list\"' '\n \tgit notes list > output &&\n \ttest_cmp expect output\n-- \n1.7.5.1.414.gb4910\n"},{"id":"168179","messageId":"1305764061-21303-4-git-send-email-gitster@pobox.com","threadId":"27403","inReplyTo":"1305764061-21303-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/3] notes remove: --stdin reads from the standard input","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-19T00:14:21Z","receivedAt":"2011-05-19T00:14:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Teach the command to read object names to remove from the standard\ninput, in addition to the object names given from the command line.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * The logical conclusion of the series.\n---\n Documentation/git-notes.txt |    7 ++++++-\n builtin/notes.c             |   14 +++++++++++++-\n t/t3301-notes.sh            |   33 +++++++++++++++++++++++++++++++++\n 3 files changed, 52 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt\nindex 6b92060..42e4823 100644\n--- a/Documentation/git-notes.txt\n+++ b/Documentation/git-notes.txt\n@@ -17,7 +17,7 @@ SYNOPSIS\n 'git notes' merge [-v | -q] [-s <strategy> ] <notes_ref>\n 'git notes' merge --commit [-v | -q]\n 'git notes' merge --abort [-v | -q]\n-'git notes' remove [--missing-ok] [<object>...]\n+'git notes' remove [--missing-ok] [--stdin] [<object>...]\n 'git notes' prune [-n | -v]\n 'git notes' get-ref\n \n@@ -159,6 +159,11 @@ OPTIONS\n \tDo not consider it an error to request removing notes from an\n \tobject that does not have notes attached to it.\n \n+--stdin::\n+\tAlso read the object names to remove notes from from the standard\n+\tinput (there is no reason you cannot combine this with object\n+\tnames from the command line).\n+\n -n::\n --dry-run::\n \tDo not remove anything; just report the object names whose notes\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex ca045f8..23954e0 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -972,10 +972,13 @@ static int remove_one_note(struct notes_tree *t, const char *name, unsigned flag\n static int remove_cmd(int argc, const char **argv, const char *prefix)\n {\n \tunsigned flag = 0;\n+\tint from_stdin = 0;\n \tstruct option options[] = {\n \t\tOPT_BIT(0, \"missing-ok\", &flag,\n \t\t\t\"attempt to remove non-existent note is not an error\",\n \t\t\tMISSING_OK),\n+\t\tOPT_BOOLEAN(0, \"stdin\", &from_stdin,\n+\t\t\t    \"read object names from the standard input\"),\n \t\tOPT_END()\n \t};\n \tstruct notes_tree *t;\n@@ -986,7 +989,7 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)\n \n \tt = init_notes_check(\"remove\");\n \n-\tif (!argc) {\n+\tif (!argc && !from_stdin) {\n \t\tretval = remove_one_note(t, \"HEAD\", flag);\n \t} else {\n \t\twhile (*argv) {\n@@ -994,6 +997,15 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)\n \t\t\targv++;\n \t\t}\n \t}\n+\tif (from_stdin) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\twhile (strbuf_getwholeline(&sb, stdin, '\\n') != EOF) {\n+\t\t\tint len = sb.len;\n+\t\t\tif (len && sb.buf[len - 1] == '\\n')\n+\t\t\t\tsb.buf[--len] = '\\0';\n+\t\t\tretval |= remove_one_note(t, sb.buf, flag);\n+\t\t}\n+\t}\n \tif (!retval)\n \t\tcommit_notes(t, \"Notes removed by 'git notes remove'\");\n \tfree_notes(t);\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex bdd4036..2c52f21 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -473,6 +473,39 @@ test_expect_success 'removing with --missing-ok but bogus ref' '\n \ttest \"$before\" = \"$after\"\n '\n \n+test_expect_success 'remove reads from --stdin' '\n+\tbefore=$(git rev-parse --verify refs/notes/commits) &&\n+\ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n+\tgit rev-parse HEAD^^ HEAD^^^ >input &&\n+\tgit notes remove --stdin <input &&\n+\tgit diff --name-only refs/notes/commits^ refs/notes/commits >actual &&\n+\ttest 2 = $(wc -l <actual) &&\n+\tgit ls-tree -r --name-only refs/notes/commits >actual &&\n+\t>empty &&\n+\ttest_cmp empty actual\n+'\n+\n+test_expect_success 'remove --stdin is also atomic' '\n+\tbefore=$(git rev-parse --verify refs/notes/commits) &&\n+\ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n+\tgit rev-parse HEAD^^ HEAD^^^ HEAD^ >input &&\n+\ttest_must_fail git notes remove --stdin <input &&\n+\tafter=$(git rev-parse --verify refs/notes/commits) &&\n+\ttest \"$before\" = \"$after\"\n+'\n+\n+test_expect_success 'removing with --stdin --missing-ok' '\n+\tbefore=$(git rev-parse --verify refs/notes/commits) &&\n+\ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n+\tgit rev-parse HEAD^^ HEAD^^^ HEAD^ >input &&\n+\tgit notes remove --missing-ok --stdin <input &&\n+\tgit diff --name-only refs/notes/commits^ refs/notes/commits >actual &&\n+\ttest 2 = $(wc -l <actual) &&\n+\tgit ls-tree -r --name-only refs/notes/commits >actual &&\n+\t>empty &&\n+\ttest_cmp empty actual\n+'\n+\n test_expect_success 'list notes with \"git notes list\"' '\n \tgit notes list > output &&\n \ttest_cmp expect output\n-- \n1.7.5.1.414.gb4910\n"},{"id":"168182","messageId":"7vpqnfa1qu.fsf@alter.siamese.dyndns.org","threadId":"27403","inReplyTo":"1305764061-21303-3-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 2/3] notes remove: --missing-ok","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-19T02:00:09Z","receivedAt":"2011-05-19T02:00:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Just FYI, I'll rename this to --ignore-missing to match other commands in\nthe version I am going to queue in 'pu'.\n"},{"id":"168183","messageId":"7vliy3a1l5.fsf@alter.siamese.dyndns.org","threadId":"27403","inReplyTo":"1305764061-21303-1-git-send-email-gitster@pobox.com","subject":"[PATCH 4/3] show: --ignore-missing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-19T02:03:34Z","receivedAt":"2011-05-19T02:03:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Instead of barfing, simply ignore bad object names seen in the\ninput. This is useful when reading from \"git notes list\" output\nthat may refer to objects that have already been garbage collected.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This would let me do something like\n\n\tgit notes list |\n        sed -e 's/. *//' | # strip note-blob\n\tgit show --format='%ci %H ...' --ignore-missing --stdin |\n        ... perhaps sort by commit date, filter, etc... |\n        git notes remove --stdin --ignore-missing\n\n   Perhaps \"git notes list --target-only\" can be added to remove the\n   need for the second \"sed\" in the pipeline.\n\n Documentation/git-rev-list.txt     |    1 +\n Documentation/rev-list-options.txt |    4 ++++\n builtin/rev-parse.c                |    1 +\n revision.c                         |   15 +++++++++++++--\n revision.h                         |    3 ++-\n 5 files changed, 21 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt\nindex 415f4f0..38fafca 100644\n--- a/Documentation/git-rev-list.txt\n+++ b/Documentation/git-rev-list.txt\n@@ -29,6 +29,7 @@ SYNOPSIS\n \t     [ \\--tags[=<pattern>] ]\n \t     [ \\--remotes[=<pattern>] ]\n \t     [ \\--glob=<glob-pattern> ]\n+\t     [ \\--ignore-missing ]\n \t     [ \\--stdin ]\n \t     [ \\--quiet ]\n \t     [ \\--topo-order ]\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 52bae31..7e7ba68 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -139,6 +139,10 @@ parents) and `--max-parents=-1` (negative numbers denote no upper limit).\n \tis automatically prepended if missing. If pattern lacks '?', '*',\n \tor '[', '/*' at the end is implied.\n \n+--ignore-missing::\n+\n+\tUpon seeing an invalid object name in the input, pretend as if\n+\tthe bad input was not given.\n \n ifndef::git-rev-list[]\n --bisect::\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex adb1cae..4c19f84 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -44,6 +44,7 @@ static int is_rev_argument(const char *arg)\n \t\t\"--branches=\",\n \t\t\"--branches\",\n \t\t\"--header\",\n+\t\t\"--ignore-missing\",\n \t\t\"--max-age=\",\n \t\t\"--max-count=\",\n \t\t\"--min-age=\",\ndiff --git a/revision.c b/revision.c\nindex a7cf79b..5cb3a75 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -133,6 +133,8 @@ void mark_parents_uninteresting(struct commit *commit)\n \n static void add_pending_object_with_mode(struct rev_info *revs, struct object *obj, const char *name, unsigned mode)\n {\n+\tif (!obj)\n+\t\treturn;\n \tif (revs->no_walk && (obj->flags & UNINTERESTING))\n \t\trevs->no_walk = 0;\n \tif (revs->reflog_info && obj->type == OBJ_COMMIT) {\n@@ -174,8 +176,11 @@ static struct object *get_reference(struct rev_info *revs, const char *name, con\n \tstruct object *object;\n \n \tobject = parse_object(sha1);\n-\tif (!object)\n+\tif (!object) {\n+\t\tif (revs->ignore_missing)\n+\t\t\treturn object;\n \t\tdie(\"bad object %s\", name);\n+\t}\n \tobject->flags |= flags;\n \treturn object;\n }\n@@ -906,6 +911,8 @@ static int add_parents_only(struct rev_info *revs, const char *arg, int flags)\n \t\treturn 0;\n \twhile (1) {\n \t\tit = get_reference(revs, arg, sha1, 0);\n+\t\tif (!it && revs->ignore_missing)\n+\t\t\treturn 0;\n \t\tif (it->type != OBJ_TAG)\n \t\t\tbreak;\n \t\tif (!((struct tag*)it)->tagged)\n@@ -1044,6 +1051,8 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,\n \t\t\ta = lookup_commit_reference(from_sha1);\n \t\t\tb = lookup_commit_reference(sha1);\n \t\t\tif (!a || !b) {\n+\t\t\t\tif (revs->ignore_missing)\n+\t\t\t\t\treturn 0;\n \t\t\t\tdie(symmetric ?\n \t\t\t\t    \"Invalid symmetric difference expression %s...%s\" :\n \t\t\t\t    \"Invalid revision range %s..%s\",\n@@ -1090,7 +1099,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,\n \t\targ++;\n \t}\n \tif (get_sha1_with_mode(arg, sha1, &mode))\n-\t\treturn -1;\n+\t\treturn revs->ignore_missing ? 0 : -1;\n \tif (!cant_be_filename)\n \t\tverify_non_filename(revs->prefix, arg);\n \tobject = get_reference(revs, arg, sha1, flags ^ local_flags);\n@@ -1475,6 +1484,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--children\")) {\n \t\trevs->children.name = \"children\";\n \t\trevs->limited = 1;\n+\t} else if (!strcmp(arg, \"--ignore-missing\")) {\n+\t\trevs->ignore_missing = 1;\n \t} else {\n \t\tint opts = diff_opt_parse(&revs->diffopt, argv, argc);\n \t\tif (!opts)\ndiff --git a/revision.h b/revision.h\nindex bca9947..93f338d 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -36,7 +36,8 @@ struct rev_info {\n \tconst char *prefix;\n \tconst char *def;\n \tstruct pathspec prune_data;\n-\tunsigned int early_output;\n+\tunsigned int\tearly_output:1,\n+\t\t\tignore_missing:1;\n \n \t/* Traversal flags */\n \tunsigned int\tdense:1,\n-- \n1.7.5.1.414.gb4910\n"},{"id":"168191","messageId":"4DD4BC0D.2000805@drmicha.warpmail.net","threadId":"27403","inReplyTo":"1305764061-21303-2-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 1/3] notes remove: allow removing more than one","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-19T06:43:25Z","receivedAt":"2011-05-19T06:43:25Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 19.05.2011 02:14:\n> While \"xargs -n1 git notes rm\" is certainly a possible way to remove notes\n> from many objects, this would create one notes \"commit\" per removal, which\n> is not quite suitable for seasonal housekeeping.\n> \n> Allow taking more than one on the command line, and record their removal\n> as a single atomic event if everthing goes well.\n> \n> Even though the old code insisted that \"git notes rm\" must be given only\n> one object (or zero, in which case it would default to HEAD), this\n> condition was not tested. Add tests to handle the new case where we feed\n> multiple objects, and also make sure if there is a bad input, no change\n> is recorded.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  Documentation/git-notes.txt |    7 +++--\n>  builtin/notes.c             |   47 ++++++++++++++++++++++--------------------\n>  t/t3301-notes.sh            |   19 +++++++++++++++++\n>  3 files changed, 48 insertions(+), 25 deletions(-)\n> \n> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt\n> index 913ecd8..e2e1c4c 100644\n> --- a/Documentation/git-notes.txt\n> +++ b/Documentation/git-notes.txt\n> @@ -17,7 +17,7 @@ SYNOPSIS\n>  'git notes' merge [-v | -q] [-s <strategy> ] <notes_ref>\n>  'git notes' merge --commit [-v | -q]\n>  'git notes' merge --abort [-v | -q]\n> -'git notes' remove [<object>]\n> +'git notes' remove [<object>...]\n>  'git notes' prune [-n | -v]\n>  'git notes' get-ref\n>  \n> @@ -106,8 +106,9 @@ When done, the user can either finalize the merge with\n>  'git notes merge --abort'.\n>  \n>  remove::\n> -\tRemove the notes for a given object (defaults to HEAD).\n> -\tThis is equivalent to specifying an empty note message to\n> +\tRemove the notes for given objects (defaults to HEAD). When\n> +\tgiving zero or one object from the command line, this is\n> +\tequivalent to specifying an empty note message to\n>  \tthe `edit` subcommand.\n>  \n>  prune::\n> diff --git a/builtin/notes.c b/builtin/notes.c\n> index 1fb1f73..30cee0f 100644\n> --- a/builtin/notes.c\n> +++ b/builtin/notes.c\n> @@ -29,7 +29,7 @@ static const char * const git_notes_usage[] = {\n>  \t\"git notes [--ref <notes_ref>] merge [-v | -q] [-s <strategy> ] <notes_ref>\",\n>  \t\"git notes merge --commit [-v | -q]\",\n>  \t\"git notes merge --abort [-v | -q]\",\n> -\t\"git notes [--ref <notes_ref>] remove [<object>]\",\n> +\t\"git notes [--ref <notes_ref>] remove [<object>...]\",\n>  \t\"git notes [--ref <notes_ref>] prune [-n | -v]\",\n>  \t\"git notes [--ref <notes_ref>] get-ref\",\n>  \tNULL\n> @@ -953,40 +953,43 @@ static int merge(int argc, const char **argv, const char *prefix)\n>  \treturn result < 0; /* return non-zero on conflicts */\n>  }\n>  \n> +static int remove_one_note(struct notes_tree *t, const char *name)\n> +{\n> +\tint status;\n> +\tunsigned char sha1[20];\n> +\tif (get_sha1(name, sha1))\n> +\t\treturn error(_(\"Failed to resolve '%s' as a valid ref.\"), name);\n> +\tstatus = remove_note(t, sha1);\n> +\tif (status)\n> +\t\tfprintf(stderr, _(\"Object %s has no note\\n\"), name);\n> +\telse\n> +\t\tfprintf(stderr, _(\"Removing note for object %s\\n\"), name);\n> +\treturn status;\n> +}\n> +\n>  static int remove_cmd(int argc, const char **argv, const char *prefix)\n>  {\n>  \tstruct option options[] = {\n>  \t\tOPT_END()\n>  \t};\n> -\tconst char *object_ref;\n>  \tstruct notes_tree *t;\n> -\tunsigned char object[20];\n> -\tint retval;\n> +\tint retval = 0;\n>  \n>  \targc = parse_options(argc, argv, prefix, options,\n>  \t\t\t     git_notes_remove_usage, 0);\n>  \n> -\tif (1 < argc) {\n> -\t\terror(_(\"too many parameters\"));\n> -\t\tusage_with_options(git_notes_remove_usage, options);\n> -\t}\n> -\n> -\tobject_ref = argc ? argv[0] : \"HEAD\";\n> -\n> -\tif (get_sha1(object_ref, object))\n> -\t\tdie(_(\"Failed to resolve '%s' as a valid ref.\"), object_ref);\n> -\n>  \tt = init_notes_check(\"remove\");\n>  \n> -\tretval = remove_note(t, object);\n> -\tif (retval)\n> -\t\tfprintf(stderr, _(\"Object %s has no note\\n\"), sha1_to_hex(object));\n> -\telse {\n> -\t\tfprintf(stderr, _(\"Removing note for object %s\\n\"),\n> -\t\t\tsha1_to_hex(object));\n> -\n> -\t\tcommit_notes(t, \"Notes removed by 'git notes remove'\");\n> +\tif (!argc) {\n> +\t\tretval = remove_one_note(t, \"HEAD\");\n> +\t} else {\n> +\t\twhile (*argv) {\n> +\t\t\tretval |= remove_one_note(t, *argv);\n> +\t\t\targv++;\n> +\t\t}\n>  \t}\n> +\tif (!retval)\n> +\t\tcommit_notes(t, \"Notes removed by 'git notes remove'\");\n>  \tfree_notes(t);\n>  \treturn retval;\n>  }\n> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\n> index 28e17c8..6a6daa9 100755\n> --- a/t/t3301-notes.sh\n> +++ b/t/t3301-notes.sh\n> @@ -435,6 +435,25 @@ test_expect_success 'removing non-existing note should not create new commit' '\n>  \ttest_cmp before_commit after_commit\n>  '\n>  \n> +test_expect_success 'removing more than one' '\n> +\tbefore=$(git rev-parse --verify refs/notes/commits) &&\n> +\ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n> +\tgit notes remove HEAD^^ HEAD^^^ &&\n> +\tgit diff --name-only refs/notes/commits^ refs/notes/commits >actual &&\n> +\ttest 2 = $(wc -l <actual) &&\n> +\tgit ls-tree -r --name-only refs/notes/commits >actual &&\n> +\t>empty &&\n> +\ttest_cmp empty actual\n> +'\n\nYou're not really testing that this removes the correct notes. Actually,\nyou're not even testing that this removes only 2 notes when there are\nmore than 2, are you? I might be a bit fussy, though. I like the feature\nin general:\n\n git notes remove $(git rev-list <fancy-rev-spec>)\n\nrocks :)\n\nAlthough \"notes remove\" could grok a rev-spec and dwim --do-walk, come\nto think of it. Would be even more gittish.\n\n> +\n> +test_expect_success 'removing is atomic' '\n> +\tbefore=$(git rev-parse --verify refs/notes/commits) &&\n> +\ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n> +\ttest_must_fail git notes remove HEAD^^ HEAD^^^ HEAD^ &&\n> +\tafter=$(git rev-parse --verify refs/notes/commits) &&\n> +\ttest \"$before\" = \"$after\"\n> +'\n> +\n>  test_expect_success 'list notes with \"git notes list\"' '\n>  \tgit notes list > output &&\n>  \ttest_cmp expect output\n"},{"id":"168195","messageId":"4DD4BD98.6040805@drmicha.warpmail.net","threadId":"27403","inReplyTo":"1305764061-21303-4-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 3/3] notes remove: --stdin reads from the standard input","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-19T06:50:00Z","receivedAt":"2011-05-19T06:50:00Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 19.05.2011 02:14:\n> Teach the command to read object names to remove from the standard\n> input, in addition to the object names given from the command line.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> \n>  * The logical conclusion of the series.\n\nSo that makes 4/3 to be... ;)\n\nI'm torn between asking for a rev-list interface and being happy with\n--stdin. I guess we have other places where we allow lists of objects in\nargv and stdin (instead of rev-list), and since this targets all kinds\nof objects, it's more natural as is (than with with the rev-list interface).\n\n> ---\n>  Documentation/git-notes.txt |    7 ++++++-\n>  builtin/notes.c             |   14 +++++++++++++-\n>  t/t3301-notes.sh            |   33 +++++++++++++++++++++++++++++++++\n>  3 files changed, 52 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt\n> index 6b92060..42e4823 100644\n> --- a/Documentation/git-notes.txt\n> +++ b/Documentation/git-notes.txt\n> @@ -17,7 +17,7 @@ SYNOPSIS\n>  'git notes' merge [-v | -q] [-s <strategy> ] <notes_ref>\n>  'git notes' merge --commit [-v | -q]\n>  'git notes' merge --abort [-v | -q]\n> -'git notes' remove [--missing-ok] [<object>...]\n> +'git notes' remove [--missing-ok] [--stdin] [<object>...]\n>  'git notes' prune [-n | -v]\n>  'git notes' get-ref\n>  \n> @@ -159,6 +159,11 @@ OPTIONS\n>  \tDo not consider it an error to request removing notes from an\n>  \tobject that does not have notes attached to it.\n>  \n> +--stdin::\n> +\tAlso read the object names to remove notes from from the standard\n> +\tinput (there is no reason you cannot combine this with object\n> +\tnames from the command line).\n> +\n>  -n::\n>  --dry-run::\n>  \tDo not remove anything; just report the object names whose notes\n> diff --git a/builtin/notes.c b/builtin/notes.c\n> index ca045f8..23954e0 100644\n> --- a/builtin/notes.c\n> +++ b/builtin/notes.c\n> @@ -972,10 +972,13 @@ static int remove_one_note(struct notes_tree *t, const char *name, unsigned flag\n>  static int remove_cmd(int argc, const char **argv, const char *prefix)\n>  {\n>  \tunsigned flag = 0;\n> +\tint from_stdin = 0;\n>  \tstruct option options[] = {\n>  \t\tOPT_BIT(0, \"missing-ok\", &flag,\n>  \t\t\t\"attempt to remove non-existent note is not an error\",\n>  \t\t\tMISSING_OK),\n> +\t\tOPT_BOOLEAN(0, \"stdin\", &from_stdin,\n> +\t\t\t    \"read object names from the standard input\"),\n>  \t\tOPT_END()\n>  \t};\n>  \tstruct notes_tree *t;\n> @@ -986,7 +989,7 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)\n>  \n>  \tt = init_notes_check(\"remove\");\n>  \n> -\tif (!argc) {\n> +\tif (!argc && !from_stdin) {\n>  \t\tretval = remove_one_note(t, \"HEAD\", flag);\n>  \t} else {\n>  \t\twhile (*argv) {\n> @@ -994,6 +997,15 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)\n>  \t\t\targv++;\n>  \t\t}\n>  \t}\n> +\tif (from_stdin) {\n> +\t\tstruct strbuf sb = STRBUF_INIT;\n> +\t\twhile (strbuf_getwholeline(&sb, stdin, '\\n') != EOF) {\n> +\t\t\tint len = sb.len;\n> +\t\t\tif (len && sb.buf[len - 1] == '\\n')\n> +\t\t\t\tsb.buf[--len] = '\\0';\n> +\t\t\tretval |= remove_one_note(t, sb.buf, flag);\n> +\t\t}\n> +\t}\n>  \tif (!retval)\n>  \t\tcommit_notes(t, \"Notes removed by 'git notes remove'\");\n>  \tfree_notes(t);\n> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\n> index bdd4036..2c52f21 100755\n> --- a/t/t3301-notes.sh\n> +++ b/t/t3301-notes.sh\n> @@ -473,6 +473,39 @@ test_expect_success 'removing with --missing-ok but bogus ref' '\n>  \ttest \"$before\" = \"$after\"\n>  '\n>  \n> +test_expect_success 'remove reads from --stdin' '\n> +\tbefore=$(git rev-parse --verify refs/notes/commits) &&\n> +\ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n> +\tgit rev-parse HEAD^^ HEAD^^^ >input &&\n> +\tgit notes remove --stdin <input &&\n> +\tgit diff --name-only refs/notes/commits^ refs/notes/commits >actual &&\n> +\ttest 2 = $(wc -l <actual) &&\n> +\tgit ls-tree -r --name-only refs/notes/commits >actual &&\n> +\t>empty &&\n> +\ttest_cmp empty actual\n> +'\n\nSame comment as for 1/3 applies here.\n\n> +\n> +test_expect_success 'remove --stdin is also atomic' '\n> +\tbefore=$(git rev-parse --verify refs/notes/commits) &&\n> +\ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n> +\tgit rev-parse HEAD^^ HEAD^^^ HEAD^ >input &&\n> +\ttest_must_fail git notes remove --stdin <input &&\n> +\tafter=$(git rev-parse --verify refs/notes/commits) &&\n> +\ttest \"$before\" = \"$after\"\n> +'\n> +\n> +test_expect_success 'removing with --stdin --missing-ok' '\n> +\tbefore=$(git rev-parse --verify refs/notes/commits) &&\n> +\ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n> +\tgit rev-parse HEAD^^ HEAD^^^ HEAD^ >input &&\n> +\tgit notes remove --missing-ok --stdin <input &&\n> +\tgit diff --name-only refs/notes/commits^ refs/notes/commits >actual &&\n> +\ttest 2 = $(wc -l <actual) &&\n> +\tgit ls-tree -r --name-only refs/notes/commits >actual &&\n> +\t>empty &&\n> +\ttest_cmp empty actual\n> +'\n> +\n>  test_expect_success 'list notes with \"git notes list\"' '\n>  \tgit notes list > output &&\n>  \ttest_cmp expect output\n"},{"id":"168196","messageId":"7v1uzv9oab.fsf@alter.siamese.dyndns.org","threadId":"27403","inReplyTo":"4DD4BC0D.2000805@drmicha.warpmail.net","subject":"Re: [PATCH 1/3] notes remove: allow removing more than one","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-19T06:50:52Z","receivedAt":"2011-05-19T06:50:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n[jc: cull the part you did not comment on, thanks]\n\n>> +test_expect_success 'removing more than one' '\n>> +\tbefore=$(git rev-parse --verify refs/notes/commits) &&\n>> +\ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n>> +\tgit notes remove HEAD^^ HEAD^^^ &&\n>> +\tgit diff --name-only refs/notes/commits^ refs/notes/commits >actual &&\n>> +\ttest 2 = $(wc -l <actual) &&\n>> +\tgit ls-tree -r --name-only refs/notes/commits >actual &&\n>> +\t>empty &&\n>> +\ttest_cmp empty actual\n>> +'\n>\n> You're not really testing that this removes the correct notes. Actually,\n> you're not even testing that this removes only 2 notes when there are\n> more than 2, are you?\n\nIn the test vector, there are only two notes, and I am testing removal of\nmultiple.  What do you want me to do?  Test removing one and make sure the\nother one survives ;-)? \n\nPatches on top of it is very welcome, but I wouldn't bother.\n"},{"id":"168199","messageId":"201105190908.19080.johan@herland.net","threadId":"27403","inReplyTo":"1305764061-21303-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/3] \"git notes remove\" updates","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2011-05-19T07:08:18Z","receivedAt":"2011-05-19T07:08:18Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Thursday 19 May 2011, Junio C Hamano wrote:\n> I wanted to do a bit better than\n> \n> \tfor sha1 in ... list of old commit objects\n> \tdo\n> \t\tgit notes --ref refs/notes/amlog remove $sha1\n> \tdone\n> \n> to remove old entries in my \"where did this commit come from\" database.\n> \n> Junio C Hamano (3):\n>   notes remove: allow removing more than one\n>   notes remove: --missing-ok\n>   notes remove: --stdin reads from the standard input\n\n\nAfter a cursory reading the series looks good to me. Thanks! :)\n\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"168200","messageId":"4DD4C73C.7020600@drmicha.warpmail.net","threadId":"27403","inReplyTo":"7v1uzv9oab.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] notes remove: allow removing more than one","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-19T07:31:08Z","receivedAt":"2011-05-19T07:31:08Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 19.05.2011 08:50:\n> Michael J Gruber <git@drmicha.warpmail.net> writes:\n> \n> [jc: cull the part you did not comment on, thanks]\n> \n>>> +test_expect_success 'removing more than one' '\n>>> +\tbefore=$(git rev-parse --verify refs/notes/commits) &&\n>>> +\ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n>>> +\tgit notes remove HEAD^^ HEAD^^^ &&\n>>> +\tgit diff --name-only refs/notes/commits^ refs/notes/commits >actual &&\n>>> +\ttest 2 = $(wc -l <actual) &&\n>>> +\tgit ls-tree -r --name-only refs/notes/commits >actual &&\n>>> +\t>empty &&\n>>> +\ttest_cmp empty actual\n>>> +'\n>>\n>> You're not really testing that this removes the correct notes. Actually,\n>> you're not even testing that this removes only 2 notes when there are\n>> more than 2, are you?\n> \n> In the test vector, there are only two notes, and I am testing removal of\n> multiple.  What do you want me to do?  Test removing one and make sure the\n> other one survives ;-)? \n\nThe test has two notes because it was created when remove would remove\none note at a time only, and the test made sure it did not remove the\nother one (!).\n\nIf you want a meaningful test which tests that \"git notes remove A B\"\nremoves exactly 2 notes (not more) you need to add another note first.\nOtherwise it could be that your new code simply nukes the notes tree. (I\nknow it does not, but only by looking at the source; this test does not\ncheck it.)\n\nChecking that it actually removes *those* 2 notes would give extra\ncredit but may not be necessary (since we know it removes the correct\none in single mode).\n\n> Patches on top of it is very welcome, but I wouldn't bother.\n\nIs that the kind of response you accept from submitters to your (valid)\nrequests that they provide negative tests in addition to positive ones?\n\nI don't mind helping out with a test patch (later this UTC day) but I'm\nconcerned about the attitude someone may read into this and may want to\nfollow as an example. In terms of code-with-test habits, I would claim\nthat git.git is really an exemplary standard to follow (and to keep).\n\nCheers,\nMichael\n"},{"id":"168204","messageId":"20110519105009.GA11107@sigill.intra.peff.net","threadId":"27403","inReplyTo":"1305764061-21303-4-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 3/3] notes remove: --stdin reads from the standard input","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-05-19T10:50:09Z","receivedAt":"2011-05-19T10:50:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 18, 2011 at 05:14:21PM -0700, Junio C Hamano wrote:\n\n> +\tif (from_stdin) {\n> +\t\tstruct strbuf sb = STRBUF_INIT;\n> +\t\twhile (strbuf_getwholeline(&sb, stdin, '\\n') != EOF) {\n> +\t\t\tint len = sb.len;\n> +\t\t\tif (len && sb.buf[len - 1] == '\\n')\n> +\t\t\t\tsb.buf[--len] = '\\0';\n> +\t\t\tretval |= remove_one_note(t, sb.buf, flag);\n> +\t\t}\n> +\t}\n\nWouldn't strbuf_rtrim (or even strbuf_trim) be useful here?\n\nManipulating only a copy of the len parameter while changing the strbuf\nitself raised warning flags, though it is correct as-is because you\nnever actually use sb.len.\n\nUsing strbuf_trim is shorter, easier to read, and will helpfully eat any\nextraneous whitespace, too.\n\n-Peff\n"},{"id":"168236","messageId":"7vpqne7fge.fsf@alter.siamese.dyndns.org","threadId":"27403","inReplyTo":"4DD4C73C.7020600@drmicha.warpmail.net","subject":"Re: [PATCH 1/3] notes remove: allow removing more than one","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-19T17:44:33Z","receivedAt":"2011-05-19T17:44:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n> The test has two notes because it was created when remove would remove\n> one note at a time only, and the test made sure it did not remove the\n> other one (!).\n\nFair enough.  Here is an incremental update to be squashed in.\n\nI notice that \"notes list <commit>\" and \"notes list | grep <commit>\" give\nquite different results, and \"notes list <commit1> <commit2>...\" does not\neven work at all. Probably the \"notes\" interface was done with interactive\nuse in mind without realizing the need for batch operations.\n\n t/t3301-notes.sh |   11 ++++++-----\n 1 files changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 6a6daa9..6278fe8 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -438,12 +438,13 @@ test_expect_success 'removing non-existing note should not create new commit' '\n test_expect_success 'removing more than one' '\n \tbefore=$(git rev-parse --verify refs/notes/commits) &&\n \ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n+\n+\t# We have only two -- add another and make sure it stays\n+\tgit notes add -m \"extra\" &&\n+\tgit notes list HEAD >after-removal-expect &&\n \tgit notes remove HEAD^^ HEAD^^^ &&\n-\tgit diff --name-only refs/notes/commits^ refs/notes/commits >actual &&\n-\ttest 2 = $(wc -l <actual) &&\n-\tgit ls-tree -r --name-only refs/notes/commits >actual &&\n-\t>empty &&\n-\ttest_cmp empty actual\n+\tgit notes list | sed -e \"s/ .*//\" >actual &&\n+\ttest_cmp after-removal-expect actual\n '\n \n test_expect_success 'removing is atomic' '\n"},{"id":"168238","messageId":"7vliy27exx.fsf@alter.siamese.dyndns.org","threadId":"27403","inReplyTo":"20110519105009.GA11107@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] notes remove: --stdin reads from the standard input","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-19T17:55:38Z","receivedAt":"2011-05-19T17:55:38Z","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> On Wed, May 18, 2011 at 05:14:21PM -0700, Junio C Hamano wrote:\n>\n>> +\tif (from_stdin) {\n>> +\t\tstruct strbuf sb = STRBUF_INIT;\n>> +\t\twhile (strbuf_getwholeline(&sb, stdin, '\\n') != EOF) {\n>> +\t\t\tint len = sb.len;\n>> +\t\t\tif (len && sb.buf[len - 1] == '\\n')\n>> +\t\t\t\tsb.buf[--len] = '\\0';\n>> +\t\t\tretval |= remove_one_note(t, sb.buf, flag);\n>> +\t\t}\n>> +\t}\n>\n> Wouldn't strbuf_rtrim (or even strbuf_trim) be useful here?\n\nThanks for noticing.\n\nI just mimicked what was done to the result of strbuf_getwholeline() in\nother places (I think from revision.c but I am not sure).\n\nAn incremental correction, relative to what I had in 'pu' overnight, looks\nlike this.\n\n builtin/notes.c  |    5 ++---\n t/t3301-notes.sh |   26 ++++++++++++++------------\n 2 files changed, 16 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 164d605..f8e437d 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -1000,11 +1000,10 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)\n \tif (from_stdin) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n \t\twhile (strbuf_getwholeline(&sb, stdin, '\\n') != EOF) {\n-\t\t\tint len = sb.len;\n-\t\t\tif (len && sb.buf[len - 1] == '\\n')\n-\t\t\t\tsb.buf[--len] = '\\0';\n+\t\t\tstrbuf_rtrim(&sb);\n \t\t\tretval |= remove_one_note(t, sb.buf, flag);\n \t\t}\n+\t\tstrbuf_release(&sb);\n \t}\n \tif (!retval)\n \t\tcommit_notes(t, \"Notes removed by 'git notes remove'\");\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex e1b5619..16de05a 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -478,13 +478,14 @@ test_expect_success 'removing with --ignore-missing but bogus ref' '\n test_expect_success 'remove reads from --stdin' '\n \tbefore=$(git rev-parse --verify refs/notes/commits) &&\n \ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n+\n+\t# We have only two -- add another and make sure it stays\n+\tgit notes add -m \"extra\" &&\n+\tgit notes list HEAD >after-removal-expect &&\n \tgit rev-parse HEAD^^ HEAD^^^ >input &&\n \tgit notes remove --stdin <input &&\n-\tgit diff --name-only refs/notes/commits^ refs/notes/commits >actual &&\n-\ttest 2 = $(wc -l <actual) &&\n-\tgit ls-tree -r --name-only refs/notes/commits >actual &&\n-\t>empty &&\n-\ttest_cmp empty actual\n+\tgit notes list | sed -e \"s/ .*//\" >actual &&\n+\ttest_cmp after-removal-expect actual\n '\n \n test_expect_success 'remove --stdin is also atomic' '\n@@ -496,16 +497,17 @@ test_expect_success 'remove --stdin is also atomic' '\n \ttest \"$before\" = \"$after\"\n '\n \n-test_expect_success 'removing with --stdin --missing-ok' '\n+test_expect_success 'removing with --stdin --ignore-missing' '\n \tbefore=$(git rev-parse --verify refs/notes/commits) &&\n \ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n+\n+\t# We have only two -- add another and make sure it stays\n+\tgit notes add -m \"extra\" &&\n+\tgit notes list HEAD >after-removal-expect &&\n \tgit rev-parse HEAD^^ HEAD^^^ HEAD^ >input &&\n-\tgit notes remove --missing-ok --stdin <input &&\n-\tgit diff --name-only refs/notes/commits^ refs/notes/commits >actual &&\n-\ttest 2 = $(wc -l <actual) &&\n-\tgit ls-tree -r --name-only refs/notes/commits >actual &&\n-\t>empty &&\n-\ttest_cmp empty actual\n+\tgit notes remove --ignore-missing --stdin <input &&\n+\tgit notes list | sed -e \"s/ .*//\" >actual &&\n+\ttest_cmp after-removal-expect actual\n '\n \n test_expect_success 'list notes with \"git notes list\"' '\n"},{"id":"168240","messageId":"20110519180205.GA26564@sigill.intra.peff.net","threadId":"27403","inReplyTo":"7vliy27exx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] notes remove: --stdin reads from the standard input","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-05-19T18:02:05Z","receivedAt":"2011-05-19T18:02:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 19, 2011 at 10:55:38AM -0700, Junio C Hamano wrote:\n\n> >> +\tif (from_stdin) {\n> >> +\t\tstruct strbuf sb = STRBUF_INIT;\n> >> +\t\twhile (strbuf_getwholeline(&sb, stdin, '\\n') != EOF) {\n> >> +\t\t\tint len = sb.len;\n> >> +\t\t\tif (len && sb.buf[len - 1] == '\\n')\n> >> +\t\t\t\tsb.buf[--len] = '\\0';\n> >> +\t\t\tretval |= remove_one_note(t, sb.buf, flag);\n> >> +\t\t}\n> >> +\t}\n> >\n> > Wouldn't strbuf_rtrim (or even strbuf_trim) be useful here?\n> \n> Thanks for noticing.\n> \n> I just mimicked what was done to the result of strbuf_getwholeline() in\n> other places (I think from revision.c but I am not sure).\n\nI am tempted to say that other callsites should be \"upgraded\" to\nstrbuf_trim. Is there any reason we should not be liberal in accepting\nsomething like:\n\n  echo \"  refs/heads/foo  \" | git rev-list --stdin\n\n?\n\nI guess for pathspecs we would want to be more literal, though, so maybe\nit is a stupid idea.\n\n> An incremental correction, relative to what I had in 'pu' overnight, looks\n> like this.\n\nThanks. BTW, I was too busy picking this one nit to mention that the\ngeneral direction of the series is a nice improvement. :)\n\n-Peff\n"},{"id":"168245","messageId":"bdd14b3138e4d90de58945f25dcf33da027d9f07.1305830419.git.git@drmicha.warpmail.net","threadId":"27403","inReplyTo":"4DD4C73C.7020600@drmicha.warpmail.net","subject":"[PATCH 1/2] fixup! notes remove: --ignore-missing","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-19T18:47:02Z","receivedAt":"2011-05-19T18:47:02Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\n t/t3301-notes.sh |    5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 09cf6b2..1edb7bf 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -438,12 +438,13 @@ test_expect_success 'removing non-existing note should not create new commit' '\n test_expect_success 'removing more than one' '\n \tbefore=$(git rev-parse --verify refs/notes/commits) &&\n \ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n+\tMSG=4 git notes add HEAD^ &&\n \tgit notes remove HEAD^^ HEAD^^^ &&\n \tgit diff --name-only refs/notes/commits^ refs/notes/commits >actual &&\n \ttest 2 = $(wc -l <actual) &&\n \tgit ls-tree -r --name-only refs/notes/commits >actual &&\n-\t>empty &&\n-\ttest_cmp empty actual\n+\tgit rev-parse HEAD^ >expect1 &&\n+\ttest_cmp expect1 actual\n '\n \n test_expect_success 'removing is atomic' '\n-- \n1.7.5.1.558.gc8bec\n"},{"id":"168246","messageId":"19352322fc4fcbf59490ab2d812191a3fdc92acb.1305830419.git.git@drmicha.warpmail.net","threadId":"27403","inReplyTo":"4DD4C73C.7020600@drmicha.warpmail.net","subject":"[PATCH 2/2] fixup! notes remove: --stdin reads from the standard input","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-19T18:47:03Z","receivedAt":"2011-05-19T18:47:03Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Cheers :)\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\n t/t3301-notes.sh |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 1edb7bf..8462173 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -495,11 +495,11 @@ test_expect_success 'remove --stdin is also atomic' '\n \ttest \"$before\" = \"$after\"\n '\n \n-test_expect_success 'removing with --stdin --missing-ok' '\n+test_expect_success 'removing with --stdin --ignore-missing' '\n \tbefore=$(git rev-parse --verify refs/notes/commits) &&\n \ttest_when_finished \"git update-ref refs/notes/commits $before\" &&\n \tgit rev-parse HEAD^^ HEAD^^^ HEAD^ >input &&\n-\tgit notes remove --missing-ok --stdin <input &&\n+\tgit notes remove --ignore-missing --stdin <input &&\n \tgit diff --name-only refs/notes/commits^ refs/notes/commits >actual &&\n \ttest 2 = $(wc -l <actual) &&\n \tgit ls-tree -r --name-only refs/notes/commits >actual &&\n-- \n1.7.5.1.558.gc8bec\n"}]}