Re: [PATCH v4] notes: teach the -e option to edit messages in editor

>
> On Mon, Oct 21, 2024 at 06:12:20PM +0000, Samuel Adekunle Abraham via GitGitGadget wrote:
> > From: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com>
> > diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> > index 99137fb2357..813dfd8db97 100755
> > --- a/t/t3301-notes.sh
> > +++ b/t/t3301-notes.sh
> > @@ -1567,4 +1567,67 @@ test_expect_success 'empty notes do not invoke the editor' '
> >       git notes remove HEAD
> >  '
> >
> > +test_expect_success 'git notes add with -m/-F invokes editor with -e' '
> > +     test_commit 19th &&
> > +     echo "edited" >expect &&
> > +     MSG="$(cat expect)" git notes add -m "initial" -e &&
> > +     git notes show >actual &&
> > +     test_cmp expect actual &&
> > +     git notes remove HEAD &&
> > +
> > +     # Add a note using -F and edit it
> > +     echo "initial" >note_file &&
> > +     MSG="$(cat expect)" git notes add -F note_file -e &&
> > +     git notes show >actual &&
> > +     test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'git notes append with -m/-F invokes the editor with -e' '
> > +     test_commit 20th &&
> > +     cat >expect <<-EOF &&
> > +             initial
> > +
> > +             edited
> > +     EOF
>
> Nit: we typically align the contents of HERE docs with `cat`. I'm not a
> huge fan of it and had been struggling with it initially, as well, but
> coding style is subjective anyway and it's totally fine that one doesn't
> agree with everything.
>
> In any case, I don't think this warrants a reroll of this patch, just
> to keep it in mind for future patches you may send.
>

> [snip]
> > +test_expect_success 'git notes append aborts when editor fails with -e' '
> > +     test_commit 22nd &&
> > +     echo "foo-file-1" >note_1 &&
> > +
> > +     # Try to append a note with -F and -e, but make the editor fail
> > +     test_env GIT_EDITOR="false" test_must_fail git notes append -F note_1 -e &&
> > +
> > +     # Verify that no note was added due to editor failure
> > +     test_must_fail git notes show
> > +'
> > +
>
> Nice.
>
> Thanks, this looks good to me overall.
>
> Patrick