Re: [PATCH v6 08/10] commit: add for_each_mergetag()

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

Christian Couder <chriscool@tuxfamily.org> writes:

> In the same way as there is for_each_ref() to
> iterate on refs, it might be useful to have
> for_each_mergetag() to iterate on the mergetags
> of a given commit.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---

Heh, "might be useful" is an understatement ;-) We won't apply a
change that "might be useful" very lightly, but this refactoring
already uses the helper in existing code, showing that it *is*
useful, no?

Let's have this early in the series, or perhaps even independent of
the "replace" series.

Thanks.

>  commit.c   | 13 +++++++++++++
>  commit.h   |  5 +++++
>  log-tree.c | 15 ++++-----------
>  3 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/commit.c b/commit.c
> index 54e157d..0f83ff7 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -1348,6 +1348,19 @@ struct commit_extra_header *read_commit_extra_headers(struct commit *commit,
>  	return extra;
>  }
>  
> +void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data)
> +{
> +	struct commit_extra_header *extra, *to_free;
> +
> +	to_free = read_commit_extra_headers(commit, NULL);
> +	for (extra = to_free; extra; extra = extra->next) {
> +		if (strcmp(extra->key, "mergetag"))
> +			continue; /* not a merge tag */
> +		fn(commit, extra, data);
> +	}
> +	free_commit_extra_headers(to_free);
> +}
> +
>  static inline int standard_header_field(const char *field, size_t len)
>  {
>  	return ((len == 4 && !memcmp(field, "tree ", 5)) ||
> diff --git a/commit.h b/commit.h
> index 4234dae..c81ba85 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -312,6 +312,11 @@ extern struct commit_extra_header *read_commit_extra_headers(struct commit *, co
>  
>  extern void free_commit_extra_headers(struct commit_extra_header *extra);
>  
> +typedef void (*each_mergetag_fn)(struct commit *commit, struct commit_extra_header *extra,
> +				 void *cb_data);
> +
> +extern void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data);
> +
>  struct merge_remote_desc {
>  	struct object *obj; /* the named object, could be a tag */
>  	const char *name;
> diff --git a/log-tree.c b/log-tree.c
> index 10e6844..706ed4c 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -413,10 +413,11 @@ static int is_common_merge(const struct commit *commit)
>  		&& !commit->parents->next->next);
>  }
>  
> -static void show_one_mergetag(struct rev_info *opt,
> +static void show_one_mergetag(struct commit *commit,
>  			      struct commit_extra_header *extra,
> -			      struct commit *commit)
> +			      void *data)
>  {
> +	struct rev_info *opt = (struct rev_info *)data;
>  	unsigned char sha1[20];
>  	struct tag *tag;
>  	struct strbuf verify_message;
> @@ -463,15 +464,7 @@ static void show_one_mergetag(struct rev_info *opt,
>  
>  static void show_mergetag(struct rev_info *opt, struct commit *commit)
>  {
> -	struct commit_extra_header *extra, *to_free;
> -
> -	to_free = read_commit_extra_headers(commit, NULL);
> -	for (extra = to_free; extra; extra = extra->next) {
> -		if (strcmp(extra->key, "mergetag"))
> -			continue; /* not a merge tag */
> -		show_one_mergetag(opt, extra, commit);
> -	}
> -	free_commit_extra_headers(to_free);
> +	for_each_mergetag(show_one_mergetag, commit, opt);
>  }
>  
>  void show_log(struct rev_info *opt)