Re: [PATCH v4 2/2] add-patch: do not print hunks repeatedly

From: Rubén Justo <rjusto@gmail.com>
Date: Mar 29, 2024, 11:37 UTC
Message-ID: <05d295d7-8db1-4d07-87ea-db8725cbf5af@gmail.com>
In-Reply-To: <11f4cba9-e12e-4804-ac04-259adee4cfae@gmail.com>

On Fri, Mar 29, 2024 at 10:41:05AM +0000, phillip.wood123@gmail.com wrote:

Show 28 quoted lines

> Hi Rubén
> 
> On 29/03/2024 03:58, Rubén Justo wrote:
> 
> Thanks for re-rolling, this looks pretty good - I've left a couple of small
> comments.
> 
> > @@ -1448,10 +1448,15 @@ static int patch_update_file(struct add_p_state *s,
> >   		strbuf_reset(&s->buf);
> >   		if (file_diff->hunk_nr) {
> > -			render_hunk(s, hunk, 0, colored, &s->buf);
> > -			fputs(s->buf.buf, stdout);
> > +			if (rendered_hunk_index != hunk_index) {
> > +				render_hunk(s, hunk, 0, colored, &s->buf);
> > +				fputs(s->buf.buf, stdout);
> > +
> > +				rendered_hunk_index = hunk_index;
> 
> This line could be grouped with the rest of this block without the blank
> line if you wanted.
> 
> > +			}
> >   			strbuf_reset(&s->buf);
> > +
> 
> I'm not sure what this new blank line is for - previously it was clear that
> the call strbuf_reset() was grouped with the code that then reuses the
> buffer. The rest of the changes look fine

OK.

Junio has already queued this iteration. I wonder if we could reduce the noise in the list by squashing:

--- >8 ---

diff --git a/add-patch.c b/add-patch.c
index 5f11692911..778f168298 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -1451,12 +1451,10 @@ static int patch_update_file(struct add_p_state *s,
 			if (rendered_hunk_index != hunk_index) {
 				render_hunk(s, hunk, 0, colored, &s->buf);
 				fputs(s->buf.buf, stdout);
-
 				rendered_hunk_index = hunk_index;
 			}
 
 			strbuf_reset(&s->buf);
-
 			if (undecided_previous >= 0) {
 				permitted |= ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK;
 				strbuf_addstr(&s->buf, ",k");