diff mbox series

[3/8] trailer: add interface for iterating over commit trailers

Message ID 20200925070249.GC62741@coredump.intra.peff.net (mailing list archive)
State Superseded
Headers show
Series parsing trailers with shortlog | expand

Commit Message

Jeff King Sept. 25, 2020, 7:02 a.m. UTC
The trailer code knows how to parse out the trailers and re-format them,
but there's no easy way to iterate over the trailers (you can use
trailer_info, but you have to then do a bunch of extra parsing).

Let's add an iteration interface that makes this easy to do.

Signed-off-by: Jeff King <peff@peff.net>
---
 trailer.c | 36 ++++++++++++++++++++++++++++++++++++
 trailer.h | 44 ++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 78 insertions(+), 2 deletions(-)

Comments

Martin Ågren Sept. 26, 2020, 12:39 p.m. UTC | #1
On Fri, 25 Sep 2020 at 09:04, Jeff King <peff@peff.net> wrote:
>
> The trailer code knows how to parse out the trailers and re-format them,
> but there's no easy way to iterate over the trailers (you can use
> trailer_info, but you have to then do a bunch of extra parsing).
>
> Let's add an iteration interface that makes this easy to do.

> +void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
> +{
> +       struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
> +       strbuf_init(&iter->key, 0);
> +       strbuf_init(&iter->val, 0);
> +       opts.no_divider = 1;
> +       trailer_info_get(&iter->info, msg, &opts);
> +       iter->cur = 0;
> +}

Ok, this does initialize everything...

> +void trailer_iterator_release(struct trailer_iterator *iter)
> +{
> +       trailer_info_release(&iter->info);
> +       strbuf_release(&iter->val);
> +       strbuf_release(&iter->key);
> +}

... and this side takes care of everything, too.

>  #define TRAILER_H
>
>  #include "list.h"
> -
> -struct strbuf;
> +#include "strbuf.h"

Spotting and removing the forward declaration, ok.

> +/*
> + * An interface for iterating over the trailers found in a particular commit
> + * message. Use like:
> + *
> + *   struct trailer_iterator iter;
> + *   trailer_iterator_init(&iter, msg);
> + *   while (trailer_iterator_advance(&iter))
> + *      ... do something with iter.key and iter.val ...
> + *   trailer_iterator_release(&iter);
> + */
> +struct trailer_iterator {
> +       struct strbuf key;
> +       struct strbuf val;
> +
> +       /* private */
> +       struct trailer_info info;
> +       size_t cur;
> +};

Ok.

> +/*
> + * Initialize "iter" in preparation for walking over the trailers in the commit
> + * message "msg". The "msg" pointer must remain valid until the iterator is
> + * released.
> + *
> + * After initializing, we are not yet pointing
> + */

Truncated sentence. "... not yet pointing at any trailer"?

Martin
Jeff King Sept. 27, 2020, 8:20 a.m. UTC | #2
On Sat, Sep 26, 2020 at 02:39:32PM +0200, Martin Ågren wrote:

> > +/*
> > + * Initialize "iter" in preparation for walking over the trailers in the commit
> > + * message "msg". The "msg" pointer must remain valid until the iterator is
> > + * released.
> > + *
> > + * After initializing, we are not yet pointing
> > + */
> 
> Truncated sentence. "... not yet pointing at any trailer"?

Oops. Yes, though I expanded it to:

  * After initializing, note that key/val will not yet point to any trailer.
  * Call advance() to parse the first one (if any).

Obviously another convention would be:

  for (trailer_iterator_init(&iter, msg);
       !trailer_iterator_done();
       trailer_iterator_advance()) {
  }

but I don't think it matters much either way as long as it's clearly
documented.

-Peff
diff mbox series

Patch

diff --git a/trailer.c b/trailer.c
index 68dabc2556..3f7391d793 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1183,3 +1183,39 @@  void format_trailers_from_commit(struct strbuf *out, const char *msg,
 	format_trailer_info(out, &info, opts);
 	trailer_info_release(&info);
 }
+
+void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
+{
+	struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
+	strbuf_init(&iter->key, 0);
+	strbuf_init(&iter->val, 0);
+	opts.no_divider = 1;
+	trailer_info_get(&iter->info, msg, &opts);
+	iter->cur = 0;
+}
+
+int trailer_iterator_advance(struct trailer_iterator *iter)
+{
+	while (iter->cur < iter->info.trailer_nr) {
+		char *trailer = iter->info.trailers[iter->cur++];
+		int separator_pos = find_separator(trailer, separators);
+
+		if (separator_pos < 1)
+			continue; /* not a real trailer */
+
+		strbuf_reset(&iter->key);
+		strbuf_reset(&iter->val);
+		parse_trailer(&iter->key, &iter->val, NULL,
+			      trailer, separator_pos);
+		unfold_value(&iter->val);
+		return 1;
+	}
+	return 0;
+}
+
+void trailer_iterator_release(struct trailer_iterator *iter)
+{
+	trailer_info_release(&iter->info);
+	strbuf_release(&iter->val);
+	strbuf_release(&iter->key);
+}
diff --git a/trailer.h b/trailer.h
index 203acf4ee1..af9949363a 100644
--- a/trailer.h
+++ b/trailer.h
@@ -2,8 +2,7 @@ 
 #define TRAILER_H
 
 #include "list.h"
-
-struct strbuf;
+#include "strbuf.h"
 
 enum trailer_where {
 	WHERE_DEFAULT,
@@ -103,4 +102,45 @@  void trailer_info_release(struct trailer_info *info);
 void format_trailers_from_commit(struct strbuf *out, const char *msg,
 				 const struct process_trailer_options *opts);
 
+/*
+ * An interface for iterating over the trailers found in a particular commit
+ * message. Use like:
+ *
+ *   struct trailer_iterator iter;
+ *   trailer_iterator_init(&iter, msg);
+ *   while (trailer_iterator_advance(&iter))
+ *      ... do something with iter.key and iter.val ...
+ *   trailer_iterator_release(&iter);
+ */
+struct trailer_iterator {
+	struct strbuf key;
+	struct strbuf val;
+
+	/* private */
+	struct trailer_info info;
+	size_t cur;
+};
+
+/*
+ * Initialize "iter" in preparation for walking over the trailers in the commit
+ * message "msg". The "msg" pointer must remain valid until the iterator is
+ * released.
+ *
+ * After initializing, we are not yet pointing
+ */
+void trailer_iterator_init(struct trailer_iterator *iter, const char *msg);
+
+/*
+ * Advance to the next trailer of the iterator. Returns 0 if there is no such
+ * trailer, and 1 otherwise. The key and value of the trailer can be
+ * fetched from the iter->key and iter->value fields (which are valid
+ * only until the next advance).
+ */
+int trailer_iterator_advance(struct trailer_iterator *iter);
+
+/*
+ * Release all resources associated with the trailer iteration.
+ */
+void trailer_iterator_release(struct trailer_iterator *iter);
+
 #endif /* TRAILER_H */