Re: [PATCH v3 1/3] interactive -p: add new `--rework-with-file` flag to interactive machinery

>
> 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 because the session automatically
> > advances to the next file or ends if we have just one file, immediately all hunks
> > in a file are decided on.
>
> The last part of the sentence after the last comma does not read
> very well, at least to me.

>
> We recommend to fold lines in such a way that after a few e-mail
> exchange and quoting it will still stay within 80-column, so a
> practical fill-column value lies somewhere around ~70.  Your lines a
> slightly longer.

>
> > Introduce the flag "--rework-with-file" when interactively selecting patches with the
> > '--patch' option, which does not auto advance, thereby allowing users the option
> > to rework with files.
> > This ensures the current auto-advance method stays as the default method.
>
> OK.  There may be suggestions for better option names from others; I
> do not think of any right now.

>
> > diff --git a/add-interactive.h b/add-interactive.h
> > index da49502b76..aef2feca56 100644
> > --- a/add-interactive.h
> > +++ b/add-interactive.h
> > @@ -6,9 +6,10 @@
> >  struct add_p_opt {
> >       int context;
> >       int interhunkcontext;
> > +     int no_auto_advance;
> >  };
>
> We add a new risk of double-negation confusion, e.g.,
>
>     if (!opt->no_auto_advance)
>         ... do the auto-advance thing ...
>
> where it may be easier to follow if it were written
>
>     if (opt->auto_advance)
>         ... do the auto-advance thing ...
>
> Would it make it harder to arrange the code if we made this member
> "auto_advance" that defaults to "true"?  We have ADD_P_OPT_INIT that
> everybody is supposed to call already, like this
>
> > -#define ADD_P_OPT_INIT { .context = -1, .interhunkcontext = -1 }
> > +#define ADD_P_OPT_INIT { .context = -1, .interhunkcontext = -1, .no_auto_advance = 0 }
>
> so I do not imagine it would be too much hassle.

>
> > @@ -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, no_auto_advance;
> >  };
>
> Likewise.

>
> > diff --git a/builtin/add.c b/builtin/add.c
> > index 32709794b3..408827cf54 100644
> > --- a/builtin/add.c
> > +++ b/builtin/add.c
> > @@ -256,6 +256,8 @@ static struct option builtin_add_options[] = {
> >       OPT_GROUP(""),
> >       OPT_BOOL('i', "interactive", &add_interactive, N_("interactive picking")),
> >       OPT_BOOL('p', "patch", &patch_interactive, N_("select hunks interactively")),
> > +     OPT_BOOL(0, "rework-with-file", &add_p_opt.no_auto_advance,
> > +              N_("rework with files when selecting hunks interactively")),
>
> Likewise.