trailer: teach iterator about non-trailer lines

Previously the iterator did not iterate over non-trailer lines. This was
somewhat unfortunate, because trailer blocks could have non-trailer
lines in them since 146245063e (trailer: allow non-trailers in trailer
block, 2016-10-21), which was before the iterator was created in
f0939a0eb1 (trailer: add interface for iterating over commit trailers,
2020-09-27).

So if trailer API users wanted to iterate over all lines in a trailer
block (including non-trailer lines), they could not use the iterator and
were forced to use the lower-level trailer_info struct directly (which
provides a raw string array that includes all lines in the trailer
block).

Change the iterator's behavior so that we also iterate over non-trailer
lines, instead of skipping over them. The new "raw" member of the
iterator allows API users to access previously inaccessible non-trailer
lines. Reword the variable "trailer" to just "line" because this
variable can now hold both trailer lines _and_ non-trailer lines.

The new "raw" member is important because anyone currently not using the
iterator is using trailer_info's raw string array directly to access
lines to check what the combined key + value looks like. If we didn't
provide a "raw" member here, iterator users would have to re-construct
the unparsed line by concatenating the key and value back together again

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/unit-tests/t-trailer.c b/t/unit-tests/t-trailer.c
index c1f897235c7072ae43fbe77628c52d7399e145a7..4f640d2a4b85b427d63ccd2391472aa7bd29bfbb 100644 (file)
--- a/t/unit-tests/t-trailer.c
+++ b/t/unit-tests/t-trailer.c
@@ -1,7 +1,7 @@
 #include "test-lib.h"
 #include "trailer.h"
 
-static void t_trailer_iterator(const char *msg, size_t num_expected_trailers)
+static void t_trailer_iterator(const char *msg, size_t num_expected)
 {
        struct trailer_iterator iter;
        size_t i = 0;
@@ -11,7 +11,7 @@ static void t_trailer_iterator(const char *msg, size_t num_expected_trailers)
                i++;
        trailer_iterator_release(&iter);
 
-       check_uint(i, ==, num_expected_trailers);
+       check_uint(i, ==, num_expected);
 }
 
 static void run_t_trailer_iterator(void)
@@ -19,7 +19,7 @@ static void run_t_trailer_iterator(void)
        static struct test_cases {
                const char *name;
                const char *msg;
-               size_t num_expected_trailers;
+               size_t num_expected;
        } tc[] = {
                {
                        "empty input",
@@ -119,7 +119,13 @@ static void run_t_trailer_iterator(void)
                        "not a trailer line\n"
                        "not a trailer line\n"
                        "Signed-off-by: x\n",
-                       1
+                       /*
+                        * Even though there is only really 1 real "trailer"
+                        * (Signed-off-by), we still have 4 trailer objects
+                        * because we still want to iterate through the entire
+                        * block.
+                        */
+                       4
                },
                {
                        "with non-trailer lines (one too many) in trailer block",
@@ -162,7 +168,7 @@ static void run_t_trailer_iterator(void)
 
        for (int i = 0; i < sizeof(tc) / sizeof(tc[0]); i++) {
                TEST(t_trailer_iterator(tc[i].msg,
-                                       tc[i].num_expected_trailers),
+                                       tc[i].num_expected),
                     "%s", tc[i].name);
        }
 }

diff --git a/trailer.c b/trailer.c
index 3e4dab9c065f028914c6336b2256dfdbbe9e68f0..4700c4414427a1aa621f31478554c2c0620fe355 100644 (file)
--- a/trailer.c
+++ b/trailer.c
@@ -1146,17 +1146,15 @@ void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
 
 int trailer_iterator_advance(struct trailer_iterator *iter)
 {
-       while (iter->internal.cur < iter->internal.info.trailer_nr) {
-               char *trailer = iter->internal.info.trailers[iter->internal.cur++];
-               int separator_pos = find_separator(trailer, separators);
-
-               if (separator_pos < 1)
-                       continue; /* not a real trailer */
+       if (iter->internal.cur < iter->internal.info.trailer_nr) {
+               char *line = iter->internal.info.trailers[iter->internal.cur++];
+               int separator_pos = find_separator(line, separators);
 
+               iter->raw = line;
                strbuf_reset(&iter->key);
                strbuf_reset(&iter->val);
                parse_trailer(&iter->key, &iter->val, NULL,
-                             trailer, separator_pos);
+                             line, separator_pos);
                /* Always unfold values during iteration. */
                unfold_value(&iter->val);
                return 1;

diff --git a/trailer.h b/trailer.h
index 9f42aa75994f3f5b54243239b7ce839b21d7fc45..7e36da7d13c068e0ba4b52637a543868d5da7db5 100644 (file)
--- a/trailer.h
+++ b/trailer.h
@@ -125,6 +125,13 @@ void format_trailers_from_commit(const struct process_trailer_options *,
  *   trailer_iterator_release(&iter);
  */
 struct trailer_iterator {
+       /*
+        * Raw line (e.g., "foo: bar baz") before being parsed as a trailer
+        * key/val pair as part of a trailer block (as the "key" and "val"
+        * fields below). If a line fails to parse as a trailer, then the "key"
+        * will be the entire line and "val" will be the empty string.
+        */
+       const char *raw;
        struct strbuf key;
        struct strbuf val;