From: Samuel Abraham <abrahamadekunle50@gmail.com>
Date: Tue, 06 Jan 2026 22:02:28 GMT
Subject: Re: [GSoC PATCH v6] add -p: show user's hunk decision when selecting hunks
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,

>
> 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.

>
> > 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

>
> > @@ -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.