From: Phillip Wood Date: Tue, 06 Jan 2026 16:10:24 GMT Subject: Re: [GSoC PATCH v6] add -p: show user's hunk decision when selecting hunks Message-ID: <54e48ac4-7151-4378-b95f-8f22279d6761@gmail.com> In-Reply-To: Hi Abraham 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. > 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. > @@ -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? Thanks Phillip > + if (hunk->use == USE_HUNK) > + hunk_use_decision = _(" (previous decision: use) "); > + else > + hunk_use_decision = _(" (previous decision: skip) "); > + } > printf(_(s->mode->prompt_mode[prompt_mode_type]), > - s->buf.buf); > + hunk_use_decision, s->buf.buf); > if (*s->s.reset_color_interactive) > fputs(s->s.reset_color_interactive, stdout); > fflush(stdout); > diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh > index 4285314f35..cc3986a9d7 100755 > --- a/t/t3701-add-interactive.sh > +++ b/t/t3701-add-interactive.sh > @@ -527,7 +527,7 @@ test_expect_success 'goto hunk 1 with "g 1"' ' > _10 > +15 > _20 > - (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_ > + (1/2) Stage this hunk (previous decision: use) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_ > EOF > test_write_lines s y g 1 | git add -p >actual && > tail -n 7 actual.trimmed && > @@ -540,7 +540,7 @@ test_expect_success 'goto hunk 1 with "g1"' ' > _10 > +15 > _20 > - (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_ > + (1/2) Stage this hunk (previous decision: use) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_ > EOF > test_write_lines s y g1 | git add -p >actual && > tail -n 4 actual.trimmed && > @@ -554,7 +554,7 @@ test_expect_success 'navigate to hunk via regex /pattern' ' > _10 > +15 > _20 > - (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_ > + (1/2) Stage this hunk (previous decision: use) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_ > EOF > test_write_lines s y /1,2 | git add -p >actual && > tail -n 5 actual.trimmed && > @@ -567,7 +567,7 @@ test_expect_success 'navigate to hunk via regex / pattern' ' > _10 > +15 > _20 > - (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_ > + (1/2) Stage this hunk (previous decision: use) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_ > EOF > test_write_lines s y / 1,2 | git add -p >actual && > tail -n 4 actual.trimmed && > @@ -579,11 +579,11 @@ test_expect_success 'print again the hunk' ' > tr _ " " >expect <<-EOF && > +15 > 20 > - (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? @@ -1,2 +1,3 @@ > + (1/2) Stage this hunk (previous decision: use) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? @@ -1,2 +1,3 @@ > 10 > +15 > 20 > - (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_ > + (1/2) Stage this hunk (previous decision: use) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_ > EOF > test_write_lines s y g 1 p | git add -p >actual && > tail -n 7 actual.trimmed && > @@ -595,11 +595,11 @@ test_expect_success TTY 'print again the hunk (PAGER)' ' > cat >expect <<-EOF && > +15 > 20 > - (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? PAGER @@ -1,2 +1,3 @@ > + (1/2) Stage this hunk (previous decision: use) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? PAGER @@ -1,2 +1,3 @@ > PAGER 10 > PAGER +15 > PAGER 20 > - (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? > + (1/2) Stage this hunk (previous decision: use) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? > EOF > test_write_lines s y g 1 P | > ( > @@ -810,7 +810,7 @@ test_expect_success 'colors can be overridden' ' > -old > +new > more-context > - (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? > + (1/2) Stage this hunk (previous decision: use) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? > EOF > test_cmp expect actual > '