Re: [PATCH 1/4] log-tree: create for_each_decoration()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 3, 2022, 17:39 UTC
- Message-ID
- <xmqqa6athcif.fsf@gitster.g>
- In-Reply-To
- <4f9f34876413927d819313a70fcdefcad5b35689.1654263472.git.gitgitgadget@gmail.com>
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 18 quoted lines
> const struct name_decoration *decoration;
> + struct format_decorations_context ctx = {
> + .sb = sb,
> + .use_color = use_color,
> + .prefix = prefix,
> + .separator = separator,
> + .suffix = suffix,
> + .color_commit = diff_get_color(use_color, DIFF_COMMIT),
> + .color_reset = decorate_get_color(use_color, DECORATION_NONE),
> + };
>
> decoration = get_name_decoration(&commit->object);
> if (!decoration)
> return;
>
> + ctx.current_and_HEAD = current_pointed_by_HEAD(decoration);
>
> + for_each_decoration(commit, append_decoration, &ctx);The function for_each_decoration() that does not take decoration but a commit felt iffy, especially because we already have called get_name_decoration() to obtain one for commit, and the API forces us to do that again at the beginning of for_each_decoration().
Show 19 quoted lines
> + strbuf_addstr(sb, ctx.color_commit);
> + strbuf_addstr(sb, ctx.suffix);
> + strbuf_addstr(sb, ctx.color_reset);
> +}
> +
> +int for_each_decoration(const struct commit *c, decoration_fn fn, void *data)
> +{
> + const struct name_decoration *decoration;
> +
> + decoration = get_name_decoration(&c->object);
> + while (decoration) {
> + int res;
> + if ((res = fn(decoration, data)))
> + return res;
> decoration = decoration->next;
> }
> +
> + return 0;
> }We'll know if this small waste is worth it when we see the new caller(s), I guess, but even if they start from commit, allowing them the same early return trick would require this piece of code:
decoration = get_name_decoration(&commit->object); if (!decoration) return;
in them, and even if they do not need it, it would be trivial for them to say
for_each_decoration(get_name_decoration(&commit->object),
do_whatever_they_need_to_do, &ctx);so, we may want to revisit this after we finish reading the series through. Making the iterator take name_decoration does not look too bad.
Show 14 quoted lines
> void show_decorations(struct rev_info *opt, struct commit *commit) > diff --git a/log-tree.h b/log-tree.h > index e7e4641cf83..ea07da2625b 100644 > --- a/log-tree.h > +++ b/log-tree.h > @@ -35,4 +35,8 @@ void fmt_output_commit(struct strbuf *, struct commit *, struct rev_info *); > void fmt_output_subject(struct strbuf *, const char *subject, struct rev_info *); > void fmt_output_email_subject(struct strbuf *, struct rev_info *); > > +typedef int decoration_fn(const struct name_decoration *d, > + void *data); > +int for_each_decoration(const struct commit *c, decoration_fn fn, void *data); > + > #endif