git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Phillip Wood <phillip.wood123@gmail.com>
Cc: Phillip Wood via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,  Justin Tobler <jltobler@gmail.com>,
	 Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: [PATCH v3 1/2] add -p: mark split hunks as undecided
Date: Fri, 26 Sep 2025 10:37:51 -0700	[thread overview]
Message-ID: <xmqqseg9azdc.fsf@gitster.g> (raw)
In-Reply-To: <58689c52-d692-4a5f-8d55-478325bbd39e@gmail.com> (Phillip Wood's message of "Fri, 26 Sep 2025 11:12:22 +0100")

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

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

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



  reply	other threads:[~2025-09-26 17:37 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
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 [this message]
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=xmqqseg9azdc.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=jltobler@gmail.com \
    --cc=phillip.wood123@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).