notes remove: allow removing more than one
While "xargs -n1 git notes rm" is certainly a possible way to remove notes from many objects, this would create one notes "commit" per removal, which is not quite suitable for seasonal housekeeping. Allow taking more than one on the command line, and record their removal as a single atomic event if everthing goes well. Even though the old code insisted that "git notes rm" must be given only one object (or zero, in which case it would default to HEAD), this condition was not tested. Add tests to handle the new case where we feed multiple objects, and also make sure if there is a bad input, no change is recorded. Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano committed
May 18, 2011 at 15:44 UTC
c3ab1a8e4cb8a84967efcf45c5e6bee41b17f9f9
3 files changed
+49
-25
Documentation/git-notes.txt
+4
-3
@@ -17,7 +17,7 @@ SYNOPSIS
17
'git notes' merge [-v | -q] [-s <strategy> ] <notes_ref>
18
'git notes' merge --commit [-v | -q]
19
'git notes' merge --abort [-v | -q]
20
-'git notes' remove [<object>]
20
+'git notes' remove [<object>...]
21
'git notes' prune [-n | -v]
22
'git notes' get-ref
23
@@ -106,8 +106,9 @@ When done, the user can either finalize the merge with
106
'git notes merge --abort'.
107
108
remove::
109
- Remove the notes for a given object (defaults to HEAD).
110
- This is equivalent to specifying an empty note message to
109
+ Remove the notes for given objects (defaults to HEAD). When
110
+ giving zero or one object from the command line, this is
111
+ equivalent to specifying an empty note message to
112
the `edit` subcommand.
113
114
prune::
builtin/notes.c
+25
-22
@@ -29,7 +29,7 @@ static const char * const git_notes_usage[] = {
29
"git notes [--ref <notes_ref>] merge [-v | -q] [-s <strategy> ] <notes_ref>",
30
"git notes merge --commit [-v | -q]",
31
"git notes merge --abort [-v | -q]",
32
- "git notes [--ref <notes_ref>] remove [<object>]",
32
+ "git notes [--ref <notes_ref>] remove [<object>...]",
33
"git notes [--ref <notes_ref>] prune [-n | -v]",
34
"git notes [--ref <notes_ref>] get-ref",
35
NULL
@@ -953,40 +953,43 @@ static int merge(int argc, const char **argv, const char *prefix)
953
return result < 0; /* return non-zero on conflicts */
954
}
955
956
+static int remove_one_note(struct notes_tree *t, const char *name)
957
+{
958
+ int status;
959
+ unsigned char sha1[20];
960
+ if (get_sha1(name, sha1))
961
+ return error(_("Failed to resolve '%s' as a valid ref."), name);
962
+ status = remove_note(t, sha1);
963
+ if (status)
964
+ fprintf(stderr, _("Object %s has no note\n"), name);
965
+ else
966
+ fprintf(stderr, _("Removing note for object %s\n"), name);
967
+ return status;
968
+}
969
+
970
static int remove_cmd(int argc, const char **argv, const char *prefix)
971
{
972
struct option options[] = {
973
OPT_END()
974
};
961
- const char *object_ref;
975
struct notes_tree *t;
963
- unsigned char object[20];
964
- int retval;
976
+ int retval = 0;
977
978
argc = parse_options(argc, argv, prefix, options,
979
git_notes_remove_usage, 0);
980
969
- if (1 < argc) {
970
- error(_("too many parameters"));
971
- usage_with_options(git_notes_remove_usage, options);
972
- }
973
-
974
- object_ref = argc ? argv[0] : "HEAD";
975
-
976
- if (get_sha1(object_ref, object))
977
- die(_("Failed to resolve '%s' as a valid ref."), object_ref);
978
-
981
t = init_notes_check("remove");
982
981
- retval = remove_note(t, object);
982
- if (retval)
983
- fprintf(stderr, _("Object %s has no note\n"), sha1_to_hex(object));
984
- else {
985
- fprintf(stderr, _("Removing note for object %s\n"),
986
- sha1_to_hex(object));
987
-
988
- commit_notes(t, "Notes removed by 'git notes remove'");
983
+ if (!argc) {
984
+ retval = remove_one_note(t, "HEAD");
985
+ } else {
986
+ while (*argv) {
987
+ retval |= remove_one_note(t, *argv);
988
+ argv++;
989
+ }
990
}
991
+ if (!retval)
992
+ commit_notes(t, "Notes removed by 'git notes remove'");
993
free_notes(t);
994
return retval;
995
}
t/t3301-notes.sh
+20
@@ -435,6 +435,26 @@ test_expect_success 'removing non-existing note should not create new commit' '
435
test_cmp before_commit after_commit
436
'
437
438
+test_expect_success 'removing more than one' '
439
+ before=$(git rev-parse --verify refs/notes/commits) &&
440
+ test_when_finished "git update-ref refs/notes/commits $before" &&
441
+
442
+ # We have only two -- add another and make sure it stays
443
+ git notes add -m "extra" &&
444
+ git notes list HEAD >after-removal-expect &&
445
+ git notes remove HEAD^^ HEAD^^^ &&
446
+ git notes list | sed -e "s/ .*//" >actual &&
447
+ test_cmp after-removal-expect actual
448
+'
449
+
450
+test_expect_success 'removing is atomic' '
451
+ before=$(git rev-parse --verify refs/notes/commits) &&
452
+ test_when_finished "git update-ref refs/notes/commits $before" &&
453
+ test_must_fail git notes remove HEAD^^ HEAD^^^ HEAD^ &&
454
+ after=$(git rev-parse --verify refs/notes/commits) &&
455
+ test "$before" = "$after"
456
+'
457
+
458
test_expect_success 'list notes with "git notes list"' '
459
git notes list > output &&
460
test_cmp expect output