Re: [PATCH v2 1/2] trailer: append trailers in-process and drop the fork to `interpret-trailers`

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

Li Chen <me@linux.beauty> writes:

> * `amend_strbuf_with_trailers()`
>   – parses the existing message,
>   – merges trailers from the command line and config,
>   – formats the final trailer block, and
>   – rewrites the supplied `struct strbuf`
>   without ever leaving the current process.

If such a helper function exists, shouldn't "interpret-trailers" be
able to lose quite a lot of lines, at least nearly as many as the
new lines introduced to this new function, by making it call this
function as well?  And that would ensure that the internal call can
safely replace the external call and produce exactly the same
output?  If so, that can be a pure refactoring patch that can become
a commit on its own, I presume?

> * `amend_file_with_trailers()` becomes a thin wrapper that reads a file
>   into a `strbuf`, calls the new helper, and writes the result back.
> * `builtin/rebase.c` now calls `amend_file_with_trailers()` instead of
>   executing `interpret-trailers`.

And then these two changes can become a separate patch on top?

> -int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)

This lone "removed line" can be avoided if the patch did not touch
this line ...

> +static size_t first_comment_pos(const struct strbuf *buf)
>  {
> -	struct child_process run_trailer = CHILD_PROCESS_INIT;
> -
> -	run_trailer.git_cmd = 1;
> -	strvec_pushl(&run_trailer.args, "interpret-trailers",
> -		     "--in-place", "--no-divider",
> -		     path, NULL);
> -	strvec_pushv(&run_trailer.args, trailer_args->v);
> -	return run_command(&run_trailer);
> +	const char *p = buf->buf;
> +	const char *end = buf->buf + buf->len;
> +
> +	while (p < end) {
> +		const char *line = p;
> +		const char *nl = memchr(p, '\n', end - p);
> +		size_t len = nl ? (size_t)(nl - p) : (size_t)(end - p);
> +
> +		/* skip leading whitespace */
> +		size_t i = 0;
> +		while (i < len && isspace((unsigned char)line[i]))
> +			i++;
> +
> +		if (i < len && line[i] == '#')
> +			return (size_t)(line - buf->buf); /* comment starts here */
> +
> +		if (!nl)              /* last line without newline */
> +			break;
> +		p = nl + 1;
> +	}
> +	return buf->len;          /* no comment line found */
> +}
> +
> +int amend_strbuf_with_trailers(struct strbuf *buf,
> +							   const struct strvec *trailer_args)

... like this?