Junio C Hamano venit, vidit, dixit 19.05.2011 02:14:
> 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@xxxxxxxxx>
> ---
> Documentation/git-notes.txt | 7 +++--
> builtin/notes.c | 47 ++++++++++++++++++++++--------------------
> t/t3301-notes.sh | 19 +++++++++++++++++
> 3 files changed, 48 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
> index 913ecd8..e2e1c4c 100644
> --- a/Documentation/git-notes.txt
> +++ b/Documentation/git-notes.txt
> @@ -17,7 +17,7 @@ SYNOPSIS
> 'git notes' merge [-v | -q] [-s <strategy> ] <notes_ref>
> 'git notes' merge --commit [-v | -q]
> 'git notes' merge --abort [-v | -q]
> -'git notes' remove [<object>]
> +'git notes' remove [<object>...]
> 'git notes' prune [-n | -v]
> 'git notes' get-ref
>
> @@ -106,8 +106,9 @@ When done, the user can either finalize the merge with
> 'git notes merge --abort'.
>
> remove::
> - Remove the notes for a given object (defaults to HEAD).
> - This is equivalent to specifying an empty note message to
> + Remove the notes for given objects (defaults to HEAD). When
> + giving zero or one object from the command line, this is
> + equivalent to specifying an empty note message to
> the `edit` subcommand.
>
> prune::
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 1fb1f73..30cee0f 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -29,7 +29,7 @@ static const char * const git_notes_usage[] = {
> "git notes [--ref <notes_ref>] merge [-v | -q] [-s <strategy> ] <notes_ref>",
> "git notes merge --commit [-v | -q]",
> "git notes merge --abort [-v | -q]",
> - "git notes [--ref <notes_ref>] remove [<object>]",
> + "git notes [--ref <notes_ref>] remove [<object>...]",
> "git notes [--ref <notes_ref>] prune [-n | -v]",
> "git notes [--ref <notes_ref>] get-ref",
> NULL
> @@ -953,40 +953,43 @@ static int merge(int argc, const char **argv, const char *prefix)
> return result < 0; /* return non-zero on conflicts */
> }
>
> +static int remove_one_note(struct notes_tree *t, const char *name)
> +{
> + int status;
> + unsigned char sha1[20];
> + if (get_sha1(name, sha1))
> + return error(_("Failed to resolve '%s' as a valid ref."), name);
> + status = remove_note(t, sha1);
> + if (status)
> + fprintf(stderr, _("Object %s has no note\n"), name);
> + else
> + fprintf(stderr, _("Removing note for object %s\n"), name);
> + return status;
> +}
> +
> static int remove_cmd(int argc, const char **argv, const char *prefix)
> {
> struct option options[] = {
> OPT_END()
> };
> - const char *object_ref;
> struct notes_tree *t;
> - unsigned char object[20];
> - int retval;
> + int retval = 0;
>
> argc = parse_options(argc, argv, prefix, options,
> git_notes_remove_usage, 0);
>
> - if (1 < argc) {
> - error(_("too many parameters"));
> - usage_with_options(git_notes_remove_usage, options);
> - }
> -
> - object_ref = argc ? argv[0] : "HEAD";
> -
> - if (get_sha1(object_ref, object))
> - die(_("Failed to resolve '%s' as a valid ref."), object_ref);
> -
> t = init_notes_check("remove");
>
> - retval = remove_note(t, object);
> - if (retval)
> - fprintf(stderr, _("Object %s has no note\n"), sha1_to_hex(object));
> - else {
> - fprintf(stderr, _("Removing note for object %s\n"),
> - sha1_to_hex(object));
> -
> - commit_notes(t, "Notes removed by 'git notes remove'");
> + if (!argc) {
> + retval = remove_one_note(t, "HEAD");
> + } else {
> + while (*argv) {
> + retval |= remove_one_note(t, *argv);
> + argv++;
> + }
> }
> + if (!retval)
> + commit_notes(t, "Notes removed by 'git notes remove'");
> free_notes(t);
> return retval;
> }
> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> index 28e17c8..6a6daa9 100755
> --- a/t/t3301-notes.sh
> +++ b/t/t3301-notes.sh
> @@ -435,6 +435,25 @@ test_expect_success 'removing non-existing note should not create new commit' '
> test_cmp before_commit after_commit
> '
>
> +test_expect_success 'removing more than one' '
> + before=$(git rev-parse --verify refs/notes/commits) &&
> + test_when_finished "git update-ref refs/notes/commits $before" &&
> + git notes remove HEAD^^ HEAD^^^ &&
> + git diff --name-only refs/notes/commits^ refs/notes/commits >actual &&
> + test 2 = $(wc -l <actual) &&
> + git ls-tree -r --name-only refs/notes/commits >actual &&
> + >empty &&
> + test_cmp empty actual
> +'
You're not really testing that this removes the correct notes. Actually,
you're not even testing that this removes only 2 notes when there are
more than 2, are you? I might be a bit fussy, though. I like the feature
in general:
git notes remove $(git rev-list <fancy-rev-spec>)
rocks :)
Although "notes remove" could grok a rev-spec and dwim --do-walk, come
to think of it. Would be even more gittish.
> +
> +test_expect_success 'removing is atomic' '
> + before=$(git rev-parse --verify refs/notes/commits) &&
> + test_when_finished "git update-ref refs/notes/commits $before" &&
> + test_must_fail git notes remove HEAD^^ HEAD^^^ HEAD^ &&
> + after=$(git rev-parse --verify refs/notes/commits) &&
> + test "$before" = "$after"
> +'
> +
> test_expect_success 'list notes with "git notes list"' '
> git notes list > output &&
> test_cmp expect output
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html