Re: [PATCH] sequencer: fix edit handling for cherry-pick and revert messages

[Elijah Newren <newren@gmail.com>]

On Mon, Mar 29, 2021 at 2:23 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 26/03/2021 07:16, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@gmail.com>
> >
> > save_opts() should save any non-default values.  It was intended to do
> > this, but since most options in struct replay_opts default to 0, it only
> > saved non-zero values.  Unfortunatley,
>
> s/Unfortunatley/Unfortunately/

Thanks, will fix.

> also s/iff/if/ in a few places below.

I want behavior for both isatty(0) and !isatty(0) to be specified by
the table, so iff is correct; see
https://www.lexico.com/definition/iff

[...]
> > Make continue_single_pick() (which is the function responsible for
> > resuming after conflict cases)
>
> It might be worth emphasizing that despite its name
> continue_single_pick() is used to commit conflict resolutions regardless
> of the number of picks - I had to check the code to see what it was
> doing in the multi-pick case.

I'll edit the description to make this more clear.

[...]
> These tests check that the options are saved but do not check what we do
> with them. It would be good to have a test that checked we actually open
> the editor when we should to test the new code in
> continue_single_pick(), however as that code calls isatty() that may be
> tricky
>
>
> I was surprised how big a change was required to the existing code but
> it seems this is surprising tricky to get right - I cannot think of any
> simplifications.

Thanks for taking a look.  :-)