| Message ID | f5b7ba08aa7c80a3bd5bcbf5563eac8896fe7054.1707196348.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enrich Trailer API | expand |
"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Linus Arver <linusa@google.com> > > This is another preparatory refactor to unify the trailer formatters. > > Make format_trailer_info() operate on trailer_item objects, not the raw > string array. > > This breaks t4205 and t6300. We will continue to make improvements until > the test suite passes again, ultimately renaming format_trailer_info() > to format_trailers(), at which point the unification of these formatters > will be complete. > > Signed-off-by: Linus Arver <linusa@google.com> > --- > trailer.c | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) I would have expected a (tentative) flip from test_expect_success to test_expect_failure in the affected tests, to illustrate what behaviour change this step introduces and why. But the huge single step in the previous round broken out into smaller steps like this round makes them much easier to follow. Thanks. > diff --git a/trailer.c b/trailer.c > index e6665c99cc3..6333dfe1c11 100644 > --- a/trailer.c > +++ b/trailer.c > @@ -1086,21 +1086,21 @@ void trailer_info_release(struct trailer_info *info) > } > > static void format_trailer_info(const struct process_trailer_options *opts, > - const struct trailer_info *info, > + struct list_head *trailers, > struct strbuf *out) > { > size_t origlen = out->len; > - size_t i; > - > - for (i = 0; i < info->trailer_nr; i++) { > - char *trailer = info->trailers[i]; > - ssize_t separator_pos = find_separator(trailer, separators); > + struct list_head *pos; > + struct trailer_item *item; > > - if (separator_pos >= 1) { > + list_for_each(pos, trailers) { > + item = list_entry(pos, struct trailer_item, list); > + if (item->token) { > struct strbuf tok = STRBUF_INIT; > struct strbuf val = STRBUF_INIT; > + strbuf_addstr(&tok, item->token); > + strbuf_addstr(&val, item->value); > > - parse_trailer(&tok, &val, NULL, trailer, separator_pos); > if (!opts->filter || opts->filter(&tok, opts->filter_data)) { > if (opts->unfold) > unfold_value(&val); > @@ -1127,13 +1127,12 @@ static void format_trailer_info(const struct process_trailer_options *opts, > if (opts->separator && out->len != origlen) { > strbuf_addbuf(out, opts->separator); > } > - strbuf_addstr(out, trailer); > + strbuf_addstr(out, item->value); > if (opts->separator) { > strbuf_rtrim(out); > } > } > } > - > } > > void format_trailers_from_commit(const struct process_trailer_options *opts, > @@ -1152,7 +1151,7 @@ void format_trailers_from_commit(const struct process_trailer_options *opts, > strbuf_add(out, msg + info.trailer_block_start, > info.trailer_block_end - info.trailer_block_start); > } else > - format_trailer_info(opts, &info, out); > + format_trailer_info(opts, &trailers, out); > > free_trailers(&trailers); > trailer_info_release(&info);
Junio C Hamano <gitster@pobox.com> writes: > "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> From: Linus Arver <linusa@google.com> >> >> This is another preparatory refactor to unify the trailer formatters. >> >> Make format_trailer_info() operate on trailer_item objects, not the raw >> string array. >> >> This breaks t4205 and t6300. We will continue to make improvements until >> the test suite passes again, ultimately renaming format_trailer_info() >> to format_trailers(), at which point the unification of these formatters >> will be complete. >> >> Signed-off-by: Linus Arver <linusa@google.com> >> --- >> trailer.c | 21 ++++++++++----------- >> 1 file changed, 10 insertions(+), 11 deletions(-) > > I would have expected a (tentative) flip from test_expect_success to > test_expect_failure in the affected tests, to illustrate what behaviour > change this step introduces and why. Somehow, such a simple idea did not cross my mind (even though I admit I didn't like how I was breaking tests, albeit temporarily). Will update on next reroll. > But the huge single step in the previous round broken out into > smaller steps like this round makes them much easier to follow. Great, I'm doing something right for once! ;)
diff --git a/trailer.c b/trailer.c index e6665c99cc3..6333dfe1c11 100644 --- a/trailer.c +++ b/trailer.c @@ -1086,21 +1086,21 @@ void trailer_info_release(struct trailer_info *info) } static void format_trailer_info(const struct process_trailer_options *opts, - const struct trailer_info *info, + struct list_head *trailers, struct strbuf *out) { size_t origlen = out->len; - size_t i; - - for (i = 0; i < info->trailer_nr; i++) { - char *trailer = info->trailers[i]; - ssize_t separator_pos = find_separator(trailer, separators); + struct list_head *pos; + struct trailer_item *item; - if (separator_pos >= 1) { + list_for_each(pos, trailers) { + item = list_entry(pos, struct trailer_item, list); + if (item->token) { struct strbuf tok = STRBUF_INIT; struct strbuf val = STRBUF_INIT; + strbuf_addstr(&tok, item->token); + strbuf_addstr(&val, item->value); - parse_trailer(&tok, &val, NULL, trailer, separator_pos); if (!opts->filter || opts->filter(&tok, opts->filter_data)) { if (opts->unfold) unfold_value(&val); @@ -1127,13 +1127,12 @@ static void format_trailer_info(const struct process_trailer_options *opts, if (opts->separator && out->len != origlen) { strbuf_addbuf(out, opts->separator); } - strbuf_addstr(out, trailer); + strbuf_addstr(out, item->value); if (opts->separator) { strbuf_rtrim(out); } } } - } void format_trailers_from_commit(const struct process_trailer_options *opts, @@ -1152,7 +1151,7 @@ void format_trailers_from_commit(const struct process_trailer_options *opts, strbuf_add(out, msg + info.trailer_block_start, info.trailer_block_end - info.trailer_block_start); } else - format_trailer_info(opts, &info, out); + format_trailer_info(opts, &trailers, out); free_trailers(&trailers); trailer_info_release(&info);