From: Phillip Wood Date: Fri, 26 Sep 2025 10:12:22 GMT Subject: Re: [PATCH v3 1/2] add -p: mark split hunks as undecided Message-ID: <58689c52-d692-4a5f-8d55-478325bbd39e@gmail.com> In-Reply-To: Hi Junio On 25/09/2025 19:21, Junio C Hamano wrote: > "Phillip Wood via GitGitGadget" writes: > >> ... In the case >> where the user only wants to change the selection of the first of >> the split hunks they will now have to do more work re-selecting the >> remaining split hunks. > > In general, that is not just "first", as you can jump ahead, or just > keep typing "J" which is not all that hard, until you find the hunk > you are looking for, at which point you flip its status. Fair enough, I think that only applies to a single split hunk though. One you select or deselect it you can't walk though the remaining split hunks J anymore because you'll have been dumped at the next undecided hunk after entering 'y' or 'n' > [...] > So I like the updated behaviour very much, but I am reluctant to > pretend as if we are siding one camps of folks who think that > splitting a selected hunk is done with an intention to deselect most > of the minihunks most of the time, playing favors. I think that > is a wrong way to frame the problem this patch solved. > > In any case, I no longer have problems with the updated behaviour > with these two patches. Thanks for working on them. > > Will queue. Thanks, do you want a different commit message or are you happy to take them as-is? Phillip > > >> Signed-off-by: Phillip Wood >> --- >> add-patch.c | 3 ++- >> t/t3701-add-interactive.sh | 10 ++++++++++ >> 2 files changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/add-patch.c b/add-patch.c >> index 302e6ba7d9..61f42de9ea 100644 >> --- a/add-patch.c >> +++ b/add-patch.c >> @@ -956,6 +956,7 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff, >> * sizeof(*hunk)); >> hunk = file_diff->hunk + hunk_index; >> hunk->splittable_into = 1; >> + hunk->use = UNDECIDED_HUNK; >> memset(hunk + 1, 0, (splittable_into - 1) * sizeof(*hunk)); >> >> header = &hunk->header; >> @@ -1057,7 +1058,7 @@ next_hunk_line: >> >> hunk++; >> hunk->splittable_into = 1; >> - hunk->use = hunk[-1].use; >> + hunk->use = UNDECIDED_HUNK; >> header = &hunk->header; >> >> header->old_count = header->new_count = context_line_count; >> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh >> index 04d2a19835..a6829fd085 100755 >> --- a/t/t3701-add-interactive.sh >> +++ b/t/t3701-add-interactive.sh >> @@ -1301,4 +1301,14 @@ do >> ' >> done >> >> +test_expect_success 'splitting previous hunk marks split hunks as undecided' ' >> + test_write_lines a " " b c d e f g h i j k >file && >> + git add file && >> + test_write_lines x " " b y d e f g h i j x >file && >> + test_write_lines n K s n y q | git add -p file && >> + git cat-file blob :file >actual && >> + test_write_lines a " " b y d e f g h i j k >expect && >> + test_cmp expect actual >> +' >> + >> test_done