git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Junio C Hamano <gitster@pobox.com>,
	Phillip Wood via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Justin Tobler <jltobler@gmail.com>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: [PATCH v2 1/2] add -p: mark split hunks as undecided
Date: Tue, 16 Sep 2025 10:36:10 +0100	[thread overview]
Message-ID: <cc2a8fa7-be27-48a6-8699-c500d894404b@gmail.com> (raw)
In-Reply-To: <xmqqbjnbhapy.fsf@gitster.g>

Hi Junio

On 15/09/2025 18:44, Junio C Hamano wrote:
> "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

> 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/


  reply	other threads:[~2025-09-16  9:36 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-21 14:57 [PATCH 0/2] add -p: a couple of hunk splitting fixes Phillip Wood via GitGitGadget
2025-02-21 14:57 ` [PATCH 1/2] add -p: mark split hunks as undecided Phillip Wood via GitGitGadget
2025-02-21 19:52   ` Justin Tobler
2025-02-21 21:31   ` Junio C Hamano
2025-02-26 14:40     ` phillip.wood123
2025-02-26 16:49       ` Junio C Hamano
2025-02-27 16:22         ` phillip.wood123
2025-02-27 18:36           ` Junio C Hamano
2025-02-28 16:19             ` Phillip Wood
2025-02-28 17:06               ` Junio C Hamano
2025-03-04 10:25                 ` Phillip Wood
2025-02-21 14:57 ` [PATCH 2/2] add-patch: update hunk splitability after editing Phillip Wood via GitGitGadget
2025-02-21 20:29   ` Justin Tobler
2025-02-21 21:42   ` Junio C Hamano
2025-09-15 15:29 ` [PATCH v2 0/2] add -p: a couple of hunk splitting fixes Phillip Wood via GitGitGadget
2025-09-15 15:29   ` [PATCH v2 1/2] add -p: mark split hunks as undecided Phillip Wood via GitGitGadget
2025-09-15 17:44     ` Junio C Hamano
2025-09-16  9:36       ` Phillip Wood [this message]
2025-09-16 16:03         ` Junio C Hamano
2025-09-15 15:29   ` [PATCH v2 2/2] add-patch: update hunk splitability after editing Phillip Wood via GitGitGadget
2025-09-25 15:10   ` [PATCH v3 0/2] add -p: a couple of hunk splitting fixes Phillip Wood via GitGitGadget
2025-09-25 15:10     ` [PATCH v3 1/2] add -p: mark split hunks as undecided Phillip Wood via GitGitGadget
2025-09-25 18:21       ` Junio C Hamano
2025-09-26 10:12         ` Phillip Wood
2025-09-26 17:37           ` Junio C Hamano
2025-10-08 13:51             ` Phillip Wood
2025-09-25 15:10     ` [PATCH v3 2/2] add-patch: update hunk splitability after editing Phillip Wood via GitGitGadget

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=cc2a8fa7-be27-48a6-8699-c500d894404b@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=jltobler@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    /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).