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

[Junio C Hamano <gitster@pobox.com>]

Rubén Justo <rjusto@gmail.com> writes:

> @@ -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) {

So, the one previously rendered is compared with the current one,
which raises an obvious question, what happens to the first new hunk
resulting from splitting a hunk?  The answer is below ...

> +				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");
> @@ -1649,10 +1654,12 @@ static int patch_update_file(struct add_p_state *s,
>  			if (!(permitted & ALLOW_SPLIT))
>  				err(s, _("Sorry, cannot split this hunk"));
>  			else if (!split_hunk(s, file_diff,
> -					     hunk - file_diff->hunk))
> +					     hunk - file_diff->hunk)) {
>  				color_fprintf_ln(stdout, s->s.header_color,
>  						 _("Split into %d hunks."),
>  						 (int)splittable_into);
> +				rendered_hunk_index = -1;
> +			}

... we explicitly say "we always want to show the current one after
this operation", which makes sense.

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

And that matches what is done for 'p', which is the base case that
wants to say "no matter what, show the current one".  Doubly makes
sense.

>  		} else {
>  			const char *p = _(help_patch_remainder), *eol = p;

Looking good.  As we are not doing anything dynamic to the help
text, I think dropping "again" in [1/2] would make sense.

Will queue.  Thanks.