Re: [GSoC PATCH v6] add -p: show user's hunk decision when selecting hunks

From: Samuel Abraham <abrahamadekunle50@gmail.com>
Date: Jan 6, 2026, 22:02 UTC
Message-ID: <CADYq+fafH-cwc7Ego1fDdNRgSnrOeGzCz_vCqszajVYEx1FV-w@mail.gmail.com>
In-Reply-To: <54e48ac4-7151-4378-b95f-8f22279d6761@gmail.com>

On Tue, Jan 6, 2026 at 5:10 PM Phillip Wood <phillip.wood123@gmail.com> wrote:

>
> Hi Abraham

Hello Phillip,

Show 18 quoted lines

>
> On 06/01/2026 12:01, Abraham Samuel Adekunle wrote:
> > When a user is interactively deciding which hunks to use or skip for
> > staging, unstaging, stashing etc, there is no way to know the
> > decision previously chosen for a hunk when navigating through the
> > previous and next hunks using K/J respectively.
> >
> > Improve the UI to explicitly show if a user has previously decided to
> > use a hunk (by pressing 'y') or skip the hunk (by pressing 'n').
> > This will improve clarity and aid the navigation process for the
> > user.
>
> I like the idea of telling the user if the hunk is currently selected
> but say "(previous decision: use)" makes the prompt rather long (some of
> the prompts in the tests below are 80 characters long). I wonder if we
> can find a more compact notation. "(currently selected)" is a bit
> shorter and takes us under 80 characters but is still longer than I'd
> like - maybe someone reading this will have a better suggestion.

Thank you for the review So I previously used selected/deselected. But Junio was not okay with those choice of words because they did not clearly tell If the user selected to skip or or selected to use the hunk.

But how about

Stage this mode change (you chose: use) [y,n,q,a,d%s,?]? Stage this mode change (you chose: skip) [y,n,q,a,d%s,?]? Stage this deletion (you chose: use) [y,n,q,a,d%sm,?]?

or Stage this mode change (choice: use) [y,n,q,a,d%s,?]? Stage this mode change (choice: skip)[y,n,q,a,d%s,?]? Stage this deletion (choice: skip)" [y,n,q,a,d%sm,?]?

or Stage this mode change (use: yes) [y,n,q,a,d%s,?]? Stage this mode change (use: no) [y,n,q,a,d%s,?]? Stage this deletion (use: no) [y,n,q,a,d%sm,?]?

Though I feel the last one does not fully tell what is happening at a glance. I can wait for more suggestions from other members if these do not suffice.

Show 20 quoted lines

>
> > diff --git a/add-patch.c b/add-patch.c
> > index 173a53241e..a383ea7f45 100644
> > --- a/add-patch.c
> > +++ b/add-patch.c
> > @@ -42,10 +42,10 @@ static struct patch_mode patch_mode_add = {
> >       .apply_args = { "--cached", NULL },
> >       .apply_check_args = { "--cached", NULL },
> >       .prompt_mode = {
> > -             N_("Stage mode change [y,n,q,a,d%s,?]? "),
> > -             N_("Stage deletion [y,n,q,a,d%s,?]? "),
> > -             N_("Stage addition [y,n,q,a,d%s,?]? "),
> > -             N_("Stage this hunk [y,n,q,a,d%s,?]? ")
> > +             N_("Stage mode change%s[y,n,q,a,d%s,?]? "),
> > +             N_("Stage deletion%s[y,n,q,a,d%s,?]? "),
> > +             N_("Stage addition%s[y,n,q,a,d%s,?]? "),
> > +             N_("Stage this hunk%s[y,n,q,a,d%s,?]? ")
>
> I'd find these strings easier to read if we kept the space and just
> passed an empty string when the hunk is undecided.

Okay I understand. Thank you. I will do that

Show 8 quoted lines

>
> > @@ -1564,8 +1565,14 @@ static int patch_update_file(struct add_p_state *s,
> >                             (uintmax_t)(file_diff->hunk_nr
> >                                               ? file_diff->hunk_nr
> >                                               : 1));
> > +             if (file_diff->hunk_nr && hunk->use != UNDECIDED_HUNK) {
>
> Why do we need to check hunk_nr here?

Okay it is actually not necessary to check `hunk_nr` since `hunk` is set to `file_diff->head` if `file_diff->hunk_nr` is zero Thank you for the observation.

Abraham.