Re: [PATCH v3 1/2] add -p: mark split hunks as undecided

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Sep 26, 2025, 10:12 UTC
Message-ID: <58689c52-d692-4a5f-8d55-478325bbd39e@gmail.com>
In-Reply-To: <xmqq348agzpk.fsf@gitster.g>

Hi Junio

On 25/09/2025 19:21, Junio C Hamano wrote:

Show 10 quoted lines

> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> 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'

Show 11 quoted lines

> [...]
> 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

Show 48 quoted lines

> 
> 
>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> ---
>>   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