Eric Sunshine writes:
> > Situation of note "removing" shouldn't happen in 'append_edit()',
> > unless it's a bug. So, let's drop the unreachable "else" code
> > in "append_edit()".
> >
> > The notes operation "append" is different with "add", the latter
> > supports to overwrite the existing note then let the "removing"
> > happen (e.g. execute `git notes add -f -F /dev/null` on an existing
> > note), but the former will not because it only does "appends" but
> > not doing "overwrites".
> >
> > Signed-off-by: Teng Long <dyroneteng@xxxxxxxxx>
> > ---
> > diff --git a/builtin/notes.c b/builtin/notes.c
> > @@ -630,13 +630,8 @@ static int append_edit(int argc, const char **argv, const char *prefix)
> > if (add_note(t, &object, &new_note, combine_notes_overwrite))
> > BUG("combine_notes_overwrite failed");
> > logmsg = xstrfmt("Notes added by 'git notes %s'", argv[0]);
> > - } else {
> > - fprintf(stderr, _("Removing note for object %s\n"),
> > - oid_to_hex(&object));
> > - remove_note(t, object.hash);
> > - logmsg = xstrfmt("Notes removed by 'git notes %s'", argv[0]);
> > + commit_notes(the_repository, t, logmsg);
> > }
> > - commit_notes(the_repository, t, logmsg);
>
> This change breaks removal of notes using "git notes edit". Prior to
> this change, if you delete the content of a note using "git notes
> edit", then the note is removed. Following this change, the note
> remains, which contradicts documented[1] behavior.
>
> [1]: Unfortunately, perhaps, this behavior is documented under the
> "remove" subcommand rather than the "edit" subcommand, but it is
> nevertheless documented and has worked this way for ages, so this
> patch causes a regression.
As the commit msg describes, the subcommands I understand should have clear
responsibilities as possible (documentaion may have some effect in my mind). So,
the removal opertion under "append subcommand" here is little wired to me, but
your suggestion makes sense, this may have compatibility issues. Although I
think it's weird that someone would use this in the presence of the remove
subcommand, and my feeling is that this code is actually copied from the add
method (introduced by 52694cdabbf68f19c8289416e7bb3bbef41d8d27), but I'm not
sure.
So, it's ok for me to drop this one.
Thanks.
