public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Christian Couder <chriscool@tuxfamily•org>
Cc: git@vger•kernel.org, Jeff King <peff@peff•net>,
	Michael Haggerty <mhagger@alum•mit.edu>,
	Jakub Narebski <jnareb@gmail•com>,
	Eric Sunshine <sunshine@sunshineco•com>
Subject: Re: [PATCH v6 08/10] commit: add for_each_mergetag()
Date: Mon, 07 Jul 2014 15:30:54 -0700	[thread overview]
Message-ID: <xmqqegxwby3l.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20140707063540.3708.39506.chriscool@tuxfamily.org> (Christian Couder's message of "Mon, 07 Jul 2014 08:35:37 +0200")

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)

  reply	other threads:[~2014-07-07 22:31 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-07  6:35 [PATCH v6 00/10] Add --graft option to git replace Christian Couder
2014-07-07  6:35 ` [PATCH v6 01/10] replace: cleanup redirection style in tests Christian Couder
2014-07-07  6:35 ` [PATCH v6 02/10] replace: add --graft option Christian Couder
2014-07-09 14:59   ` Junio C Hamano
2014-07-10  9:30     ` Christian Couder
2014-07-10 16:51       ` Junio C Hamano
2014-07-10 17:36         ` Junio C Hamano
2014-07-11  8:59           ` Christian Couder
2014-07-11 14:22             ` Junio C Hamano
2014-07-11 16:24               ` Christian Couder
2014-07-11 18:25                 ` Junio C Hamano
2014-07-11 18:29                   ` Jeff King
2014-07-07  6:35 ` [PATCH v6 03/10] replace: add test for --graft Christian Couder
2014-07-07 22:16   ` Junio C Hamano
2014-07-08  5:36     ` Christian Couder
2014-07-07  6:35 ` [PATCH v6 04/10] Documentation: replace: add --graft option Christian Couder
2014-07-07  6:35 ` [PATCH v6 05/10] contrib: add convert-grafts-to-replace-refs.sh Christian Couder
2014-07-07  6:35 ` [PATCH v6 06/10] replace: remove signature when using --graft Christian Couder
2014-07-07  6:35 ` [PATCH v6 07/10] replace: add test for --graft with signed commit Christian Couder
2014-07-07  6:35 ` [PATCH v6 08/10] commit: add for_each_mergetag() Christian Couder
2014-07-07 22:30   ` Junio C Hamano [this message]
2014-07-08  5:53     ` Christian Couder
2014-07-07  6:35 ` [PATCH v6 09/10] replace: check mergetags when using --graft Christian Couder
2014-07-07 22:28   ` Junio C Hamano
2014-07-08  5:35     ` Christian Couder
2014-07-07  6:35 ` [PATCH v6 10/10] replace: add test for --graft with a mergetag Christian Couder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqegxwby3l.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=chriscool@tuxfamily$(echo .)org \
    --cc=git@vger$(echo .)kernel.org \
    --cc=jnareb@gmail$(echo .)com \
    --cc=mhagger@alum$(echo .)mit.edu \
    --cc=peff@peff$(echo .)net \
    --cc=sunshine@sunshineco$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox