All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.