From: Junio C Hamano Date: Sat, 25 Oct 2025 15:42:00 GMT Subject: Re: [PATCH 1/2] add-patch: quit without skipping undecided hunks Message-ID: In-Reply-To: <0985f775-fb01-4de0-99a8-4775b602829a@web.de> René Scharfe 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 > --- > 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,