All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: Git List <git@vger.kernel.org>
Subject: Re: [PATCH 1/2] add-patch: quit without skipping undecided hunks
Date: Sat, 25 Oct 2025 08:42:00 -0700	[thread overview]
Message-ID: <xmqqv7k3ng3b.fsf@gitster.g> (raw)
In-Reply-To: <0985f775-fb01-4de0-99a8-4775b602829a@web.de> ("René Scharfe"'s message of "Sat, 25 Oct 2025 07:46:42 +0200")

René Scharfe <l.s.r@web.de> writes:

> Option q implies d, i.e., it marks any undecided hunks towards the
> bottom of the hunk array as skipped.  This is unnecessary; later code
> treats undecided and skipped hunks the same: The only functions that
> use UNDECIDED_HUNK and SKIP_HUNK are patch_update_file() itself (but
> not after its big for loop) and its helpers get_first_undecided() and
> display_hunks().
>
> Streamline the handling of option q by quitting immediately.
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  add-patch.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)

You are really into "add -p" for the past few days, aren't you?

I thought I knew this code fairly well (after all, I wrote the
original version before it got ported to C), and cannot believe an
idiotic mistake like this one remained in the code X-<.

I very much appreciate your careful reading.  Will queue.

>
> diff --git a/add-patch.c b/add-patch.c
> index ae9a20d8f2..a70def1f81 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -1601,7 +1601,7 @@ static int patch_update_file(struct add_p_state *s,
>  			} else if (hunk->use == UNDECIDED_HUNK) {
>  				hunk->use = USE_HUNK;
>  			}
> -		} else if (ch == 'd' || ch == 'q') {
> +		} else if (ch == 'd') {
>  			if (file_diff->hunk_nr) {
>  				for (; hunk_index < file_diff->hunk_nr; hunk_index++) {
>  					hunk = file_diff->hunk + hunk_index;
> @@ -1613,10 +1613,9 @@ static int patch_update_file(struct add_p_state *s,
>  			} else if (hunk->use == UNDECIDED_HUNK) {
>  				hunk->use = SKIP_HUNK;
>  			}
> -			if (ch == 'q') {
> -				quit = 1;
> -				break;
> -			}
> +		} else if (ch == 'q') {
> +			quit = 1;
> +			break;
>  		} else if (s->answer.buf[0] == 'K') {
>  			if (permitted & ALLOW_GOTO_PREVIOUS_HUNK)
>  				hunk_index = dec_mod(hunk_index,

      parent reply	other threads:[~2025-10-25 15:42 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-25  5:46 [PATCH 1/2] add-patch: quit without skipping undecided hunks René Scharfe
2025-10-25  5:48 ` [PATCH 2/2] add-patch: quit on EOF René Scharfe
2025-10-25 16:20   ` Junio C Hamano
2025-10-25 17:23     ` René Scharfe
2025-10-26  0:37       ` Junio C Hamano
2025-10-26  0:46   ` Junio C Hamano
2025-10-26 16:11     ` René Scharfe
2025-10-25 15:42 ` Junio C Hamano [this message]

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=xmqqv7k3ng3b.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.