From: phillip.wood123@gmail.com
Date: Fri, 29 Mar 2024 10:41:05 GMT
Subject: Re: [PATCH v4 2/2] add-patch: do not print hunks repeatedly
Message-ID: <11f4cba9-e12e-4804-ac04-259adee4cfae@gmail.com>
In-Reply-To: <1f737f82-9bc5-43b4-b78b-bf1b4393efc8@gmail.com>

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;
>