git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>,
	 "Windl, Ulrich" <u.windl@ukr.de>,
	 Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: [PATCH v2 2/5] add-patch: document that option J rolls over
Date: Sun, 05 Oct 2025 14:30:43 -0700	[thread overview]
Message-ID: <xmqq7bx9rq7w.fsf@gitster.g> (raw)
In-Reply-To: <187aac4d-18b6-41df-a181-7f42e3cbc0d4@web.de> ("René Scharfe"'s message of "Sun, 5 Oct 2025 17:55:24 +0200")

René Scharfe <l.s.r@web.de> writes:

> The variable "permitted" is only not reset after moving to a different

"only not" -> "not".

> hunk, so it only accumulates permission and doesn't necessarily reflect
> those of the current hunk.  This may be a bug, but is actually useful
> with the option J, which can be used at the last hunk to roll over to
> the first hunk.  Make this particular behavior official.
>
> Suggested-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  Documentation/git-add.adoc |  2 +-
>  add-patch.c                |  4 ++--
>  t/t3701-add-interactive.sh | 18 ++++++++++++++----
>  3 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/git-add.adoc b/Documentation/git-add.adoc
> index 3266ccf105..5c05a3a7f9 100644
> --- a/Documentation/git-add.adoc
> +++ b/Documentation/git-add.adoc
> @@ -343,7 +343,7 @@ patch::
>         g - select a hunk to go to
>         / - search for a hunk matching the given regex
>         j - go to the next undecided hunk
> -       J - go to the next hunk
> +       J - go to the next hunk, roll over at the bottom
>         k - go to the previous undecided hunk
>         K - go to the previous hunk
>         s - split the current hunk into smaller hunks
> diff --git a/add-patch.c b/add-patch.c
> index 912266a3f8..bef2ba7a25 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -1398,7 +1398,7 @@ static size_t display_hunks(struct add_p_state *s,
>  
>  static const char help_patch_remainder[] =
>  N_("j - go to the next undecided hunk\n"
> -   "J - go to the next hunk\n"
> +   "J - go to the next hunk, roll over at the bottom\n"
>     "k - go to the previous undecided hunk\n"
>     "K - go to the previous hunk\n"
>     "g - select a hunk to go to\n"
> @@ -1493,7 +1493,7 @@ static int patch_update_file(struct add_p_state *s,
>  				permitted |= ALLOW_GOTO_NEXT_UNDECIDED_HUNK;
>  				strbuf_addstr(&s->buf, ",j");
>  			}
> -			if (hunk_index + 1 < file_diff->hunk_nr) {
> +			if (file_diff->hunk_nr > 1) {
>  				permitted |= ALLOW_GOTO_NEXT_HUNK;
>  				strbuf_addstr(&s->buf, ",J");
>  			}
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index d9fe289a7a..d5d2e120ab 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -334,7 +334,7 @@ test_expect_success 'different prompts for mode change/deleted' '
>  	cat >expect <<-\EOF &&
>  	(1/1) Stage deletion [y,n,q,a,d,p,?]?
>  	(1/2) Stage mode change [y,n,q,a,d,j,J,g,/,p,?]?
> -	(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]?
> +	(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,?]?
>  	EOF
>  	test_cmp expect actual.filtered
>  '
> @@ -521,7 +521,7 @@ test_expect_success 'split hunk setup' '
>  test_expect_success 'goto hunk 1 with "g 1"' '
>  	test_when_finished "git reset" &&
>  	tr _ " " >expect <<-EOF &&
> -	(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? + 1:  -1,2 +1,3          +15
> +	(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,?]? + 1:  -1,2 +1,3          +15
>  	_ 2:  -2,4 +3,8          +21
>  	go to which hunk? @@ -1,2 +1,3 @@
>  	_10
> @@ -550,7 +550,7 @@ test_expect_success 'goto hunk 1 with "g1"' '
>  test_expect_success 'navigate to hunk via regex /pattern' '
>  	test_when_finished "git reset" &&
>  	tr _ " " >expect <<-EOF &&
> -	(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? @@ -1,2 +1,3 @@
> +	(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,?]? @@ -1,2 +1,3 @@
>  	_10
>  	+15
>  	_20
> @@ -805,7 +805,7 @@ test_expect_success 'colors can be overridden' '
>  	<YELLOW>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? <RESET><MAGENTA>@@ -3 +3,2 @@<RESET>
>  	<CYAN> more-context<RESET>
>  	<BLUE>+<RESET><BLUE>another-one<RESET>
> -	<YELLOW>(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? <RESET><MAGENTA>@@ -1,3 +1,3 @@<RESET>
> +	<YELLOW>(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,?]? <RESET><MAGENTA>@@ -1,3 +1,3 @@<RESET>
>  	<CYAN> context<RESET>
>  	<BOLD>-old<RESET>
>  	<BLUE>+new<RESET>
> @@ -1354,4 +1354,14 @@ do
>  	'
>  done
>  
> +test_expect_success 'option J rolls over' '
> +	test_write_lines a b c d e f g h i >file &&
> +	git add file &&
> +	test_write_lines X b c d e f g h X >file &&
> +	test_write_lines J J q | git add -p >out &&
> +	test_write_lines 1 2 1 >expect &&
> +	sed -n -e "s-/.*--" -e "s/^(//p" <out >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_done

  reply	other threads:[~2025-10-05 21:30 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-02  9:23 Broken handling of "J" hunks for "add --interactive"? Windl, Ulrich
2025-10-03 12:16 ` [PATCH] add-patch: roll over to next undecided hunk René Scharfe
2025-10-03 13:41   ` Phillip Wood
2025-10-03 14:10     ` René Scharfe
2025-10-08 13:47       ` Phillip Wood
2025-10-03 16:11   ` Junio C Hamano
2025-10-03 19:53     ` René Scharfe
2025-10-03 20:39       ` Junio C Hamano
2025-10-03 20:42       ` Junio C Hamano
2025-10-03 21:18   ` Junio C Hamano
2025-10-05 15:45 ` [PATCH v2 0/5] " René Scharfe
2025-10-05 15:55   ` [PATCH v2 1/5] add-patch: improve help for options j, J, k, and K René Scharfe
2025-10-05 21:30     ` Junio C Hamano
2025-10-06 17:17       ` René Scharfe
2025-10-06 17:58         ` Junio C Hamano
2025-10-31 10:08     ` [EXT] " Windl, Ulrich
2025-11-01  8:18       ` Junio C Hamano
2025-11-03 12:43         ` [EXT] " Windl, Ulrich
2025-10-05 15:55   ` [PATCH v2 2/5] add-patch: document that option J rolls over René Scharfe
2025-10-05 21:30     ` Junio C Hamano [this message]
2025-10-05 15:55   ` [PATCH v2 3/5] add-patch: let options y, n, j, and e roll over to next undecided René Scharfe
2025-10-05 15:55   ` [PATCH v2 4/5] add-patch: let options k and K roll over like j and J René Scharfe
2025-10-05 20:55     ` Junio C Hamano
2025-10-06 17:18       ` René Scharfe
2025-10-05 15:55   ` [PATCH v2 5/5] add-patch: reset "permitted" at loop start René Scharfe
2025-10-06 17:18 ` [PATCH v3 0/6] add-patch: roll over to next undecided hunk René Scharfe
2025-10-06 17:19   ` [PATCH v3 1/6] add-patch: improve help for options j, J, k, and K René Scharfe
2025-10-06 17:20   ` [PATCH v3 2/6] add-patch: document that option J rolls over René Scharfe
2025-10-06 17:21   ` [PATCH v3 3/6] add-patch: let options y, n, j, and e roll over to next undecided René Scharfe
2025-10-06 17:22   ` [PATCH v3 4/6] add-patch: let options k and K roll over like j and J René Scharfe
2025-10-06 17:23   ` [PATCH v3 5/6] add-patch: let options a and d roll over like y and n René Scharfe
2025-10-06 17:24   ` [PATCH v3 6/6] add-patch: reset "permitted" at loop start René Scharfe
2025-10-31 10:28     ` [EXT] " Windl, Ulrich
2025-10-31 15:16       ` Junio C Hamano
2025-10-06 18:00   ` [PATCH v3 0/6] add-patch: roll over to next undecided hunk Junio C Hamano
2025-10-06 20:05     ` René Scharfe
2025-10-06 22:01       ` Junio C Hamano

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=xmqq7bx9rq7w.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=u.windl@ukr.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).