On Fri, Mar 29, 2024 at 10:41:05AM +0000, phillip.wood123@xxxxxxxxx wrote:
> 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");
