Re: [PATCH v8 1/6] interpret-trailers: factor trailer rewriting
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 6, 2026, 21:04 UTC
- Message-ID
- <xmqqo6l0zn2p.fsf@gitster.g>
- In-Reply-To
- <0d08b361995f4d117b3c80a7e403c1a821d10a5f.1772808594.git.phillip.wood@dunelm.org.uk>
Phillip Wood <phillip.wood123@gmail.com> writes:
Show 14 quoted lines
> From: Li Chen <me@linux.beauty> > > Extract the trailer rewriting logic into a helper that appends to an > output strbuf. > > Update interpret_trailers() to handle file I/O only: read input once, > call the helper, and write the buffered result. > > This separation makes it easier to move the helper into trailer.c in the > next commit. > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > Signed-off-by: Li Chen <me@linux.beauty> > ---
If "Li wrote it and signed it off, Phillip is relaying" is what is happening, the sign-offs are in reverse order. If "Li incorporated what Phillip wrote earlier in his larger work", then the sign-offs may be fine, but it would be necessary to hint that that is what happened in the proposed log message. I cannot quite tell which is the case.
Show 95 quoted lines
> builtin/interpret-trailers.c | 57 ++++++++++++++++++++----------------
> 1 file changed, 32 insertions(+), 25 deletions(-)
>
> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
> index 41b0750e5af..69f9d67ec0e 100644
> --- a/builtin/interpret-trailers.c
> +++ b/builtin/interpret-trailers.c
> @@ -136,32 +136,21 @@ static void read_input_file(struct strbuf *sb, const char *file)
> strbuf_complete_line(sb);
> }
>
> -static void interpret_trailers(const struct process_trailer_options *opts,
> - struct list_head *new_trailer_head,
> - const char *file)
> +static void process_trailers(const struct process_trailer_options *opts,
> + struct list_head *new_trailer_head,
> + struct strbuf *input, struct strbuf *out)
> {
> LIST_HEAD(head);
> - struct strbuf sb = STRBUF_INIT;
> - struct strbuf trailer_block_sb = STRBUF_INIT;
> struct trailer_block *trailer_block;
> - FILE *outfile = stdout;
> -
> - trailer_config_init();
> -
> - read_input_file(&sb, file);
> -
> - if (opts->in_place)
> - outfile = create_in_place_tempfile(file);
> -
> - trailer_block = parse_trailers(opts, sb.buf, &head);
> +
> + trailer_block = parse_trailers(opts, input->buf, &head);
>
> /* Print the lines before the trailer block */
> if (!opts->only_trailers)
> - fwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile);
> + strbuf_add(out, input->buf, trailer_block_start(trailer_block));
>
> if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
> - fprintf(outfile, "\n");
> -
> + strbuf_addch(out, '\n');
>
> if (!opts->only_input) {
> LIST_HEAD(config_head);
> @@ -173,22 +162,40 @@ static void interpret_trailers(const struct process_trailer_options *opts,
> }
>
> /* Print trailer block. */
> - format_trailers(opts, &head, &trailer_block_sb);
> + format_trailers(opts, &head, out);
> free_trailers(&head);
> - fwrite(trailer_block_sb.buf, 1, trailer_block_sb.len, outfile);
> - strbuf_release(&trailer_block_sb);
>
> /* Print the lines after the trailer block as is. */
> if (!opts->only_trailers)
> - fwrite(sb.buf + trailer_block_end(trailer_block), 1,
> - sb.len - trailer_block_end(trailer_block), outfile);
> + strbuf_add(out, input->buf + trailer_block_end(trailer_block),
> + input->len - trailer_block_end(trailer_block));
> trailer_block_release(trailer_block);
> -
> +}
> +
> +static void interpret_trailers(const struct process_trailer_options *opts,
> + struct list_head *new_trailer_head,
> + const char *file)
> +{
> + struct strbuf input = STRBUF_INIT;
> + struct strbuf out = STRBUF_INIT;
> + FILE *outfile = stdout;
> +
> + trailer_config_init();
> +
> + read_input_file(&input, file);
> +
> + if (opts->in_place)
> + outfile = create_in_place_tempfile(file);
> +
> + process_trailers(opts, new_trailer_head, &input, &out);
> +
> + strbuf_write(&out, outfile);
> if (opts->in_place)
> if (rename_tempfile(&trailers_tempfile, file))
> die_errno(_("could not rename temporary file to %s"), file);
>
> - strbuf_release(&sb);
> + strbuf_release(&input);
> + strbuf_release(&out);
> }
>
> int cmd_interpret_trailers(int argc,