public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail•com>
To: Abraham Samuel Adekunle <abrahamadekunle50@gmail•com>,
	git@vger•kernel.org
Cc: "Patrick Steinhardt" <ps@pks•im>,
	"Junio C Hamano" <gitster@pobox•com>,
	"SZEDER Gábor" <szeder.dev@gmail•com>,
	"Christian Couder" <christian.couder@gmail•com>,
	"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail•com>
Subject: Re: [GSoC PATCH v6] add -p: show user's hunk decision when selecting hunks
Date: Tue, 6 Jan 2026 16:10:24 +0000	[thread overview]
Message-ID: <54e48ac4-7151-4378-b95f-8f22279d6761@gmail.com> (raw)
In-Reply-To: <aVz5kf6eLsMZ6WQQ@Adekunles-MacBook-Air.local>

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 >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 >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 >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 >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 >actual.trimmed &&
> @@ -595,11 +595,11 @@ test_expect_success TTY 'print again the hunk (PAGER)' '
>   	cat >expect <<-EOF &&
>   	<GREEN>+<RESET><GREEN>15<RESET>
>   	 20<RESET>
> -	<BOLD;BLUE>(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? <RESET>PAGER <CYAN>@@ -1,2 +1,3 @@<RESET>
> +	<BOLD;BLUE>(1/2) Stage this hunk (previous decision: use) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? <RESET>PAGER <CYAN>@@ -1,2 +1,3 @@<RESET>
>   	PAGER  10<RESET>
>   	PAGER <GREEN>+<RESET><GREEN>15<RESET>
>   	PAGER  20<RESET>
> -	<BOLD;BLUE>(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? <RESET>
> +	<BOLD;BLUE>(1/2) Stage this hunk (previous decision: use) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? <RESET>
>   	EOF
>   	test_write_lines s y g 1 P |
>   	(
> @@ -810,7 +810,7 @@ test_expect_success 'colors can be overridden' '
>   	<BOLD>-old<RESET>
>   	<BLUE>+new<RESET>
>   	<CYAN> more-context<RESET>
> -	<YELLOW>(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? <RESET>
> +	<YELLOW>(1/2) Stage this hunk (previous decision: use) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? <RESET>
>   	EOF
>   	test_cmp expect actual
>   '


  reply	other threads:[~2026-01-06 16:10 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-30 15:06 [PATCH] add -p: show hunk selection state when selecting hunks Abraham Samuel Adekunle
2025-11-30 18:32 ` Junio C Hamano
2025-12-01 10:01   ` Abraham Samuel Adekunle
2026-01-01 21:04 ` [GSoC PATCH v2] add -p: show user's hunk decision " Abraham Samuel Adekunle
2026-01-01 22:54   ` Junio C Hamano
2026-01-02  7:20     ` Samuel Abraham
2026-01-02 18:51   ` [GSoC PATCH v3] " Abraham Samuel Adekunle
2026-01-04  6:03     ` Junio C Hamano
2026-01-04 10:36       ` Samuel Abraham
2026-01-04 11:02     ` [GSoC PATCH v4] " Abraham Samuel Adekunle
2026-01-05 19:35       ` SZEDER Gábor
2026-01-05 21:52         ` Samuel Abraham
2026-01-06 11:05       ` [GSoC PATCH v5] " Abraham Samuel Adekunle
2026-01-06 11:08         ` Kristoffer Haugsbakk
2026-01-06 11:46           ` Samuel Abraham
2026-01-06 12:01         ` [GSoC PATCH v6] " Abraham Samuel Adekunle
2026-01-06 16:10           ` Phillip Wood [this message]
2026-01-06 19:01             ` Ben Knoble
2026-01-07  0:13               ` Junio C Hamano
2026-01-06 22:02             ` Samuel Abraham
2026-01-06 22:19               ` Samuel Abraham
2026-01-07  8:49                 ` Samuel Abraham
2026-01-08 15:07           ` [GSoC PATCH v7] " Abraham Samuel Adekunle
2026-01-11  3:57             ` Junio C Hamano
2026-01-11 11:04               ` Samuel Abraham

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54e48ac4-7151-4378-b95f-8f22279d6761@gmail.com \
    --to=phillip.wood123@gmail$(echo .)com \
    --cc=abrahamadekunle50@gmail$(echo .)com \
    --cc=christian.couder@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=kristofferhaugsbakk@fastmail$(echo .)com \
    --cc=phillip.wood@dunelm$(echo .)org.uk \
    --cc=ps@pks$(echo .)im \
    --cc=szeder.dev@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox