[PATCH] add -p: show hunk selection state when selecting hunks

[PATCH] add -p: show hunk selection state when selecting hunks

[Abraham Samuel Adekunle]

When selecting hunks to stage or not to stage, there is no way to know
if a hunk has been selected or not when navigating through the previous
and next hunks using K/J respectively.

Improve the UI to show whether a particular hunk has been selected
or deselected to improve clarity and aid the navigation process.

Reported-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com>
---
 add-patch.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/add-patch.c b/add-patch.c
index 173a53241e..e70e390506 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -45,7 +45,7 @@ static struct patch_mode patch_mode_add = {
 		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 this hunk [y,n,q,a,d%s,?] %s? ")
 	},
 	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
 			     "will immediately be marked for staging."),
@@ -1564,7 +1564,19 @@ static int patch_update_file(struct add_p_state *s,
 			      (uintmax_t)(file_diff->hunk_nr
 						? file_diff->hunk_nr
 						: 1));
-		printf(_(s->mode->prompt_mode[prompt_mode_type]),
+		if (prompt_mode_type == PROMPT_HUNK) {
+			const char *state = "";
+			if (file_diff->hunk_nr) {
+				if (hunk->use == USE_HUNK)
+					state = _("[selected]");
+				else if (hunk->use == SKIP_HUNK)
+					state = _("[deselected]");
+			}
+			printf(_(s->mode->prompt_mode[prompt_mode_type]),
+				s->buf.buf, state);
+		}
+		else
+			printf(_(s->mode->prompt_mode[prompt_mode_type]),
 		       s->buf.buf);
 		if (*s->s.reset_color_interactive)
 			fputs(s->s.reset_color_interactive, stdout);
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] add -p: show hunk selection state when selecting hunks

[Junio C Hamano]

Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> writes:

> diff --git a/add-patch.c b/add-patch.c
> index 173a53241e..e70e390506 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -45,7 +45,7 @@ static struct patch_mode patch_mode_add = {
>  		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 this hunk [y,n,q,a,d%s,?] %s? ")
>  	},

Three comments:

 * These sets of prompts exist for each front-end that uses the
   interactive patch machinery, and we are looking at the set used
   by "git add -p".  But the "I came back here with K, or I do not
   remember which between k and K I came back here with, and I
   cannot easily tell if the hunk I am looking at is already
   selected" issue is shared with other users like "git reset -p".

 * "chmod +x Makefile && echo >>Makefile && git add -p" would ask if
   you want to stage the mode change of the path and content change
   for the path separately.  You may skip, and later come back with
   K to this question.  The same "hmph, have I selected to use
   this?" issue exists, no?

 * The existing "[choices]? " was designed to be at the very end of
   the question, so that the answer given by the user will come
   immediately after the offered choices.  Adding an overly long
   "selected" or "deselected" to make it "[choices] selected?" does
   not give us a pleasant end-user experience.

Also, after you decided on one hunk when you have two hunks, typing
'j' or 'k' would tell you "No other undecided hunk".  The phrase
used here, "undecided", refers to the choice between USE or SKIP.
To convey the intent clearly, "Select"/"Deselect" feels a rather
indirect way (i.e. "selected for use" vs "selected to skip") to say
what is happening.

Ideally, if we can convey

    Stage this mode change (you previously decided to use it) [y,n,q,a,d%s,?]?
    Stage this mode change (you previously decided to skip it) [y,n,q,a,d%s,?]?
    Stage this deletion (you previously decided to use it) [y,n,q,a,d%sm,?]?
    ...

without wasting too many extra display width, that would be great,
but this patch is not quite there, I am afraid to say.

> @@ -1564,7 +1564,19 @@ static int patch_update_file(struct add_p_state *s,
>  			      (uintmax_t)(file_diff->hunk_nr
>  						? file_diff->hunk_nr
>  						: 1));
> -		printf(_(s->mode->prompt_mode[prompt_mode_type]),
> +		if (prompt_mode_type == PROMPT_HUNK) {
> +			const char *state = "";
> +			if (file_diff->hunk_nr) {
> +				if (hunk->use == USE_HUNK)
> +					state = _("[selected]");
> +				else if (hunk->use == SKIP_HUNK)
> +					state = _("[deselected]");
> +			}
> +			printf(_(s->mode->prompt_mode[prompt_mode_type]),
> +				s->buf.buf, state);
> +		}
> +		else
> +			printf(_(s->mode->prompt_mode[prompt_mode_type]),
>  		       s->buf.buf);
>  		if (*s->s.reset_color_interactive)
>  			fputs(s->s.reset_color_interactive, stdout);

Re: [PATCH] add -p: show hunk selection state when selecting hunks

[Abraham Samuel Adekunle]

On Sun, Nov 30, 2025 at 10:32:56AM -0800, Junio C Hamano wrote:
> Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> writes:
> 
> > diff --git a/add-patch.c b/add-patch.c
> > index 173a53241e..e70e390506 100644
> > --- a/add-patch.c
> > +++ b/add-patch.c
> > @@ -45,7 +45,7 @@ static struct patch_mode patch_mode_add = {
> >  		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 this hunk [y,n,q,a,d%s,?] %s? ")
> >  	},
> 
> Three comments:
> 
>  * These sets of prompts exist for each front-end that uses the
>    interactive patch machinery, and we are looking at the set used
>    by "git add -p".  But the "I came back here with K, or I do not
>    remember which between k and K I came back here with, and I
>    cannot easily tell if the hunk I am looking at is already
>    selected" issue is shared with other users like "git reset -p".

Hello Junio,
Thank you for your review.

Okay, are you suggesting I apply the tweak in all prompt_mode arrays
used by other front-ends. I can see all the others modes (patch_mode_*)
shown in the file.

> 
>  * "chmod +x Makefile && echo >>Makefile && git add -p" would ask if
>    you want to stage the mode change of the path and content change
>    for the path separately.  You may skip, and later come back with
>    K to this question.  The same "hmph, have I selected to use
>    this?" issue exists, no?

Yes true, the issue does exist.
I will fix the change for the others
> 
>  * The existing "[choices]? " was designed to be at the very end of
>    the question, so that the answer given by the user will come
>    immediately after the offered choices.  Adding an overly long
>    "selected" or "deselected" to make it "[choices] selected?" does
>    not give us a pleasant end-user experience.

Okay.

> 
> Also, after you decided on one hunk when you have two hunks, typing
> 'j' or 'k' would tell you "No other undecided hunk".  The phrase
> used here, "undecided", refers to the choice between USE or SKIP.
> To convey the intent clearly, "Select"/"Deselect" feels a rather
> indirect way (i.e. "selected for use" vs "selected to skip") to say
> what is happening.
> 
> Ideally, if we can convey
> 
>     Stage this mode change (you previously decided to use it) [y,n,q,a,d%s,?]?
>     Stage this mode change (you previously decided to skip it) [y,n,q,a,d%s,?]?
>     Stage this deletion (you previously decided to use it) [y,n,q,a,d%sm,?]?
>     ...
> 
> without wasting too many extra display width, that would be great,

Okay this makes sense. But since the display width is something to watch out for,
would something like below siffice?

	Stage this mode change (previous decision: stage) [y,n,q,a,d%s,?]?
	Stage this mode change (previous decision: skip) [y,n,q,a,d%s,?]?
	Stage this deletion (previous decision: stage) [y,n,q,a,d%s,?]?

> but this patch is not quite there, I am afraid to say.

Thank you Junio, I will work towards getting it there.
[...]

Abraham