Re: [PATCH v4 1/4] interactive -p: add new `--auto-advance` flag

>
> Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> writes:
>
> > When using the interactive add, reset, stash or checkout machinery,
> > we do not have the option of reworking with a file when selecting
> > hunks, because the session automatically advances to the next file
> > or ends if we have just one file.
> >
> > Introduce the flag `--auto-advance` which auto advances by default,
> > when interactively selecting patches with the '--patch' option.
> > However, the `--no-auto-advance` option does not auto advance, thereby
> > allowing users the option to rework with files.
> >
> > Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com>
> > ---
> >  add-interactive.c     | 4 ++++
> >  add-interactive.h     | 5 +++--
> >  builtin/add.c         | 4 ++++
> >  builtin/checkout.c    | 7 +++++++
> >  builtin/reset.c       | 4 ++++
> >  builtin/stash.c       | 8 ++++++++
> >  t/t9902-completion.sh | 1 +
> >  7 files changed, 31 insertions(+), 2 deletions(-)
> >
> > diff --git a/add-interactive.c b/add-interactive.c
> > index 95ec5a89f8..c3a36cd11f 100644
> > --- a/add-interactive.c
> > +++ b/add-interactive.c
> > @@ -64,6 +64,7 @@ void init_add_i_state(struct add_i_state *s, struct repository *r,
> >       s->r = r;
> >       s->context = -1;
> >       s->interhunkcontext = -1;
> > +     s->auto_advance = 1;
> >
> >       s->use_color_interactive = check_color_config(r, "color.interactive");
> >
> > @@ -124,6 +125,8 @@ void init_add_i_state(struct add_i_state *s, struct repository *r,
> >                       die(_("%s cannot be negative"), "--inter-hunk-context");
> >               s->interhunkcontext = add_p_opt->interhunkcontext;
> >       }
> > +     if (!add_p_opt->auto_advance)
> > +             s->auto_advance = 0;
> >  }
>
> I am confused.  Why do we need above two hunks in this function?
> Wouldn't it suffice to do
>
>         s->auto_advance = add_p_opt->auto_advance;
>
> in the first hunk, instead of assigning 1 to it?

>
> >  struct add_i_state {
> >       struct repository *r;
> > @@ -28,7 +29,7 @@ struct add_i_state {
> >
> >       int use_single_key;
> >       char *interactive_diff_filter, *interactive_diff_algorithm;
> > -     int context, interhunkcontext;
> > +     int context, interhunkcontext, auto_advance;
>
> Please don't do this.
>
> The original is already bad to have two members on the same line,
> but is tolerated as they represent somewhat related concepts.  The
> auto_advance member has nothing to do with these two.
>