On 11/06/2025 01:09, Junio C Hamano wrote:
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?
Exactly - I was expecting to see a refactoring of interpret_trailers()
in builtin/interpret-trailers.c that moved most of the function body
into a new function in trailer.c that added the trailers to an strbuf.
This seems to be a parallel implementation which doesn't sound like the
best plan.
I'm going to be off the list for a couple of weeks, I'll take a more
detailed look at this series when I'm back
Best Wishes
Phillip
* `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?
