Re: [PATCH v2 3/3] reflog: add --historical-branch to name the branch of each entry

["Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com>]

On Sun, Oct 18, 2026, at 20:27, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>[snip]
> diff --git a/reflog-walk.c b/reflog-walk.c
>[snip]
>  static struct complete_reflogs *read_complete_reflog(const char *ref)
>  {
>  	struct complete_reflogs *reflogs =
> @@ -178,13 +257,16 @@ void init_reflog_walk(struct reflog_walk_info **info)
>  	(*info)->complete_reflogs.strdup_strings = 1;
>  }
>
> -void reflog_walk_mark(struct reflog_walk_info *info, bool operations)
> +void reflog_walk_mark(struct reflog_walk_info *info, bool operations,
> +		      bool historical_branch)
>  {
>  	struct string_list_item *item;
>
>  	for_each_string_list_item(item, &info->complete_reflogs) {
>  		if (operations)
>  			mark_operations(item->util);

This was from the previous commit. A function that uses a `bool` param,
iterates/walks over a list, and then *if* `operations` is `true` (1)
calls a function, that looks weird to me. You’re either going to do it
to every item or just walk the list without doing anything.

> +		if (historical_branch)
> +			mark_branches(item->util);

But now you have two `bool` and you do two checks.

This is less weird to my eyes but these conditions inside a for-each
construct/macro still looks like a bad approach. Can’t you check each
condition and then call for-each with an unconditional `mark_branches`
for each item?

    if (cond)
        for-each item in items
            fun(item)

>  	}
>  }
>
> @@ -287,6 +369,16 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
>  	return 0;
>  }
>[snip]