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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 26, 2025, 17:37 UTC
Message-ID: <xmqqseg9azdc.fsf@gitster.g>
In-Reply-To: <58689c52-d692-4a5f-8d55-478325bbd39e@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:

Show 17 quoted lines

> Hi Junio
>
> On 25/09/2025 19:21, Junio C Hamano wrote:
>> "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'

If you have a single hunk for a file, you cannot split after you selected that single hunk in the first place, and the change we are discussing would not help the situation.

If you have two hunks, you select one (now the other one is shown), you can "K" back to the selected one, "s"plit it, and then say 'y/n' to deal with the first of the split hunks, which would take you to that "originally second" hunk. You can "K" back again to the last of the minihunk.

I think the room for improvement of the UI we are seeing is mostly outside the theme of this change under discussion, but enumerating them may help somebody pick them up as #leftoverbits in the future.

 * There is no way to rework with a file once you decided sel/desel
   on all its hunks.  Even when you have multiple modified files,
   you cannot switch from the current file to another file whose
   hunks have already been decided.

   Perhaps introduce "Y" and "N", that allows a choice like "y" and
   "n", but does not auto-advance?  I think the current code takes
   "N" and performs "n", so this would be a backward incompatible
   change that needs to be designed better.  Perhaps when no hunks
   are in undecided state, show a prompt "What now?" that allows you
   to still navigate with "J" and "K", or go to next file?

 * There is no way to see what state a hunk that is on the screen is
   in.  If you remember that you came with "j" or "k" or deciding on
   all hunks in another file and you are automatically taken to this
   file, then the hunk is undecided, but if you came here with "J"
   or "K", you cannot tell if you decided to select it or deselect it.

   Perhaps tweak the "(3/5) Stage this hunk [choices]?" prompt and
   give this information?

The change under discussion somewhot helps the first one but only when you have at least one undecided hunk in the file.

The latter problem is greatly alleviated with the change under discussion on this thread.

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?

Hmph, what would an improved commit log message would say? Reword the second UI problem above and explain how this change improves the situation?

Thanks.