Subject: Re: [PATCH 1/2] add-patch: quit without skipping undecided hunks

From: Junio C Hamano <[email-masked]>
Date: Sat, 25 Oct 2025 08:42:00 -0700

René Scharfe <[email-masked]> 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 <[email-masked]>
> ---
>  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,