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

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Sep 16, 2025, 09:36 UTC
Message-ID: <cc2a8fa7-be27-48a6-8699-c500d894404b@gmail.com>
In-Reply-To: <xmqqbjnbhapy.fsf@gitster.g>

Hi Junio

On 15/09/2025 18:44, Junio C Hamano wrote:

Show 34 quoted lines

> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> When a hunk is split, each of the new hunks inherits whether it is
>> selected or not from the original hunk. If a selected hunk is split
>> all of the new hunks are marked as "selected" and the user is only
>> prompted with the first of the split hunks. The user is not asked
>> whether or not they wish to select the rest of the new hunks. This
>> means that if they wish to deselect any of the new hunks apart from
>> the first one they have to navigate back to the hunk they want to
>> deselect before they can deselect it. This is unfortunate as the user
>> is presumably splitting the original hunk because they only want to
>> select some sub-set of it.
>>
>> Instead mark all the new hunks as "undecided" so that the user is
>> prompted whether they wish to select each one in turn. 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. However, changing the selection of any of the
>> other newly created hunks is now much simpler as the user no-longer has
>> to navigate back to them in order to change their selected state.
> 
> That is great, but ...
> 
>> Due
>> to concerns that users may be relying on the current behaviour [1]
>> this change is guarded by WITH_BREAKING_CHANGES.
> 
> ... this does not really sound like a good candidate for "before
> version X we used to do this, but after X we no longer do so".
> 
> Unless it is a mere bugfix, in which case such a change does not
> deserve a huge-version-bump switchover like this.

To me, the current behavior is strange enough to be considered a bug but when we discussed this before you were not so sure and said [1]

     Given our user base has grown quite a bit over the years, it
     almost is a given that any change to existing behaviour is a
     regression to somebody.  Certainly a safe material for Git 3.0
     but I do not know if it is safe enough for 2.50 for example.
     The strategy to leave it longer in 'next' did not work well
     to catch potential issues for another topic during this cycle,
     but we could try it out again.

So I re-rolled using WITH_BREAKING_CHANGES. If you're happy just to change the behavior unconditionally I can go back to V1

Show 6 quoted lines

> On the other hand, assuming it is not a bugfix but introducing a
> different behaviour, where both the original and the new ones are
> useful depending on the situation, wouldn't it be better to give
> users choices at runtime instead, with a configuration variable at
> least, but possibly with a interactive command to choose which
> behaviour is used on demand?

I'm not really convinced the current behavior is that useful and it is such an esoteric use of 'add -p' that I'm not sure it makes sense to add a config setting for it.

Thanks

Phillip

[1] https://lore.kernel.org/git/xmqqjz9b6xr1.fsf@gitster.g/