From: Phillip Wood <phillip.wood123@gmail•com>
To: Li Chen <me@linux•beauty>, git@vger•kernel.org
Cc: Junio C Hamano <gitster@pobox•com>,
Phillip Wood <phillip.wood@dunelm•org.uk>,
Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail•com>
Subject: Re: [PATCH v7 1/5] interpret-trailers: factor trailer rewriting
Date: Mon, 2 Mar 2026 14:56:04 +0000 [thread overview]
Message-ID: <56b1b6b6-94c2-4a2f-b473-9b4d09d6f52e@gmail.com> (raw)
In-Reply-To: <20260224070552.148591-2-me@linux.beauty>
Hi Li
On 24/02/2026 07:05, Li Chen wrote:
> 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.
This is still missing my sign off c.f.
https://lore.kernel.org/f5152523-f7ff-4dee-a685-fb0b74cd6a56@gmail.com
> Signed-off-by: Li Chen <me@linux•beauty>
> ---
> v7:
> Use strbuf_write() when emitting buffered output.
Also renamed "sb" to "input"
Apart from the missing sign off this all looks good.
Thanks
Phillip
>
> builtin/interpret-trailers.c | 53 ++++++++++++++++++++----------------
> 1 file changed, 30 insertions(+), 23 deletions(-)
>
> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
> index 41b0750e5a..69f9d67ec0 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,
next prev parent reply other threads:[~2026-03-02 14:56 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-24 7:05 [PATCH v7 0/5] rebase: support --trailer Li Chen
2026-02-24 7:05 ` [PATCH v7 1/5] interpret-trailers: factor trailer rewriting Li Chen
2026-03-02 14:56 ` Phillip Wood [this message]
2026-03-02 15:00 ` Li Chen
2026-02-24 7:05 ` [PATCH v7 2/5] trailer: move process_trailers to trailer.h Li Chen
2026-03-02 14:56 ` phillip.wood123
2026-02-24 7:05 ` [PATCH v7 3/5] trailer: append trailers without fork/exec Li Chen
2026-03-02 14:56 ` Phillip Wood
2026-02-24 7:05 ` [PATCH v7 4/5] commit, tag: parse --trailer with OPT_STRVEC Li Chen
2026-03-02 14:56 ` Phillip Wood
2026-02-24 7:05 ` [PATCH v7 5/5] rebase: support --trailer Li Chen
2026-03-03 15:05 ` Phillip Wood
2026-03-03 20:36 ` Kristoffer Haugsbakk
2026-03-03 21:18 ` Junio C Hamano
2026-03-04 15:53 ` Phillip Wood
2026-03-04 17:22 ` Junio C Hamano
2026-02-26 16:52 ` [PATCH v7 0/5] " Junio C Hamano
2026-02-26 18:15 ` Phillip Wood
2026-02-26 21:12 ` Kristoffer Haugsbakk
2026-03-04 14:29 ` Phillip Wood
2026-03-05 13:49 ` Li Chen
2026-03-06 14:55 ` Phillip Wood
2026-03-06 14:53 ` [PATCH v8 0/6] " Phillip Wood
2026-03-06 14:53 ` [PATCH v8 1/6] interpret-trailers: factor trailer rewriting Phillip Wood
2026-03-06 21:04 ` Junio C Hamano
2026-03-09 10:36 ` Phillip Wood
2026-03-06 14:53 ` [PATCH v8 2/6] interpret-trailers: refactor create_in_place_tempfile() Phillip Wood
2026-03-06 21:05 ` Junio C Hamano
2026-03-06 14:53 ` [PATCH v8 3/6] trailer: libify a couple of functions Phillip Wood
2026-03-06 14:53 ` [PATCH v8 4/6] trailer: append trailers without fork/exec Phillip Wood
2026-03-06 14:53 ` [PATCH v8 5/6] commit, tag: parse --trailer with OPT_STRVEC Phillip Wood
2026-03-06 14:53 ` [PATCH v8 6/6] rebase: support --trailer Phillip Wood
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56b1b6b6-94c2-4a2f-b473-9b4d09d6f52e@gmail.com \
--to=phillip.wood123@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=kristofferhaugsbakk@fastmail$(echo .)com \
--cc=me@linux$(echo .)beauty \
--cc=phillip.wood@dunelm$(echo .)org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox