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

From: [email-masked]
Date: Fri, 29 Mar 2024 10:41:05 +0000

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

Best Wishes

Phillip

>   			if (undecided_previous >= 0) {
>   				permitted |= ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK;
>   				strbuf_addstr(&s->buf, ",k");
> @@ -1646,13 +1651,15 @@ static int patch_update_file(struct add_p_state *s,
>   			hunk_index = i;
>   		} else if (s->answer.buf[0] == 's') {
>   			size_t splittable_into = hunk->splittable_into;
> -			if (!(permitted & ALLOW_SPLIT))
> +			if (!(permitted & ALLOW_SPLIT)) {
>   				err(s, _("Sorry, cannot split this hunk"));
> -			else if (!split_hunk(s, file_diff,
> -					     hunk - file_diff->hunk))
> +			} else if (!split_hunk(s, file_diff,
> +					     hunk - file_diff->hunk)) {
>   				color_fprintf_ln(stdout, s->s.header_color,
>   						 _("Split into %d hunks."),
>   						 (int)splittable_into);
> +				rendered_hunk_index = -1;
> +			}
>   		} else if (s->answer.buf[0] == 'e') {
>   			if (!(permitted & ALLOW_EDIT))
>   				err(s, _("Sorry, cannot edit this hunk"));
> @@ -1661,7 +1668,7 @@ static int patch_update_file(struct add_p_state *s,
>   				goto soft_increment;
>   			}
>   		} else if (s->answer.buf[0] == 'p') {
> -			/* nothing special is needed */
> +			rendered_hunk_index = -1;
>   		} else {
>   			const char *p = _(help_patch_remainder), *eol = p;
>