trailer: support values folded to multiple lines

Currently, interpret-trailers requires that a trailer be only on 1 line.
For example:

a: first line
second line

would be interpreted as one trailer line followed by one non-trailer line.

Make interpret-trailers support RFC 822-style folding, treating those
lines as one logical trailer.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
index 4966b5b104b15fe93af384c4714486f250697b3d..e99bda6addbb84ddc13bc3d8f9662cf5139aa3a4 100644 (file)
--- a/Documentation/git-interpret-trailers.txt
+++ b/Documentation/git-interpret-trailers.txt
@@ -57,11 +57,12 @@ minus signs start the patch part of the message.
 
 When reading trailers, there can be whitespaces after the
 token, the separator and the value. There can also be whitespaces
 
 When reading trailers, there can be whitespaces after the
 token, the separator and the value. There can also be whitespaces

-inside the token and the value.
+inside the token and the value. The value may be split over multiple lines with
+each subsequent line starting with whitespace, like the "folding" in RFC 822.

 
 Note that 'trailers' do not follow and are not intended to follow many
 
 Note that 'trailers' do not follow and are not intended to follow many

-rules for RFC 822 headers. For example they do not follow the line
-folding rules, the encoding rules and probably many other rules.
+rules for RFC 822 headers. For example they do not follow
+the encoding rules and probably many other rules.

 
 OPTIONS
 -------
 
 OPTIONS
 -------

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 3d94b3a2f76c9cd04635733bd852df4495987a0f..4dd1d7c52085d25546f64692acd0b351a7dddd72 100755 (executable)
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -256,6 +256,175 @@ test_expect_success 'line with leading whitespace is not trailer' '
        test_cmp expected actual
 '
 
        test_cmp expected actual
 '
 

+test_expect_success 'multiline field treated as one trailer for 25% check' '
+       q_to_tab >patch <<-\EOF &&
+
+               Signed-off-by: a <a@example.com>
+               name: value on
+               Qmultiple lines
+               this is not a trailer
+               this is not a trailer
+               this is not a trailer
+               this is not a trailer
+               this is not a trailer
+               this is not a trailer
+       EOF
+       q_to_tab >expected <<-\EOF &&
+
+               Signed-off-by: a <a@example.com>
+               name: value on
+               Qmultiple lines
+               this is not a trailer
+               this is not a trailer
+               this is not a trailer
+               this is not a trailer
+               this is not a trailer
+               this is not a trailer
+               name: value
+       EOF
+       git interpret-trailers --trailer "name: value" patch >actual &&
+       test_cmp expected actual
+'
+
+test_expect_success 'multiline field treated as atomic for placement' '
+       q_to_tab >patch <<-\EOF &&
+
+               another: trailer
+               name: value on
+               Qmultiple lines
+               another: trailer
+       EOF
+       q_to_tab >expected <<-\EOF &&
+
+               another: trailer
+               name: value on
+               Qmultiple lines
+               name: value
+               another: trailer
+       EOF
+       test_config trailer.name.where after &&
+       git interpret-trailers --trailer "name: value" patch >actual &&
+       test_cmp expected actual
+'
+
+test_expect_success 'multiline field treated as atomic for replacement' '
+       q_to_tab >patch <<-\EOF &&
+
+               another: trailer
+               name: value on
+               Qmultiple lines
+               another: trailer
+       EOF
+       q_to_tab >expected <<-\EOF &&
+
+               another: trailer
+               another: trailer
+               name: value
+       EOF
+       test_config trailer.name.ifexists replace &&
+       git interpret-trailers --trailer "name: value" patch >actual &&
+       test_cmp expected actual
+'
+
+test_expect_success 'multiline field treated as atomic for difference check' '
+       q_to_tab >patch <<-\EOF &&
+
+               another: trailer
+               name: first line
+               Qsecond line
+               another: trailer
+       EOF
+       test_config trailer.name.ifexists addIfDifferent &&
+
+       q_to_tab >trailer <<-\EOF &&
+               name: first line
+               Qsecond line
+       EOF
+       q_to_tab >expected <<-\EOF &&
+
+               another: trailer
+               name: first line
+               Qsecond line
+               another: trailer
+       EOF
+       git interpret-trailers --trailer "$(cat trailer)" patch >actual &&
+       test_cmp expected actual &&
+
+       q_to_tab >trailer <<-\EOF &&
+               name: first line
+               QQQQQsecond line
+       EOF
+       q_to_tab >expected <<-\EOF &&
+
+               another: trailer
+               name: first line
+               Qsecond line
+               another: trailer
+               name: first line
+               QQQQQsecond line
+       EOF
+       git interpret-trailers --trailer "$(cat trailer)" patch >actual &&
+       test_cmp expected actual &&
+
+       q_to_tab >trailer <<-\EOF &&
+               name: first line *DIFFERENT*
+               Qsecond line
+       EOF
+       q_to_tab >expected <<-\EOF &&
+
+               another: trailer
+               name: first line
+               Qsecond line
+               another: trailer
+               name: first line *DIFFERENT*
+               Qsecond line
+       EOF
+       git interpret-trailers --trailer "$(cat trailer)" patch >actual &&
+       test_cmp expected actual
+'
+
+test_expect_success 'multiline field treated as atomic for neighbor check' '
+       q_to_tab >patch <<-\EOF &&
+
+               another: trailer
+               name: first line
+               Qsecond line
+               another: trailer
+       EOF
+       test_config trailer.name.where after &&
+       test_config trailer.name.ifexists addIfDifferentNeighbor &&
+
+       q_to_tab >trailer <<-\EOF &&
+               name: first line
+               Qsecond line
+       EOF
+       q_to_tab >expected <<-\EOF &&
+
+               another: trailer
+               name: first line
+               Qsecond line
+               another: trailer
+       EOF
+       git interpret-trailers --trailer "$(cat trailer)" patch >actual &&
+       test_cmp expected actual &&
+
+       q_to_tab >trailer <<-\EOF &&
+               name: first line
+               QQQQQsecond line
+       EOF
+       q_to_tab >expected <<-\EOF &&
+
+               another: trailer
+               name: first line
+               Qsecond line
+               name: first line
+               QQQQQsecond line
+               another: trailer
+       EOF
+       git interpret-trailers --trailer "$(cat trailer)" patch >actual &&
+       test_cmp expected actual
+'
+

 test_expect_success 'with config setup' '
        git config trailer.ack.key "Acked-by: " &&
        cat >expected <<-\EOF &&
 test_expect_success 'with config setup' '
        git config trailer.ack.key "Acked-by: " &&
        cat >expected <<-\EOF &&

diff --git a/trailer.c b/trailer.c
index 84105ac181f657b587d922ecd119ed59303adcd1..d19a92cd1c4448bf4c3f34f387554ba8719520a7 100644 (file)
--- a/trailer.c
+++ b/trailer.c
@@ -619,12 +619,14 @@ static void parse_trailer(struct strbuf *tok, struct strbuf *val,
        }
 }
 
        }
 }
 

-static void add_trailer_item(struct list_head *head, char *tok, char *val)
+static struct trailer_item *add_trailer_item(struct list_head *head, char *tok,
+                                            char *val)

 {
        struct trailer_item *new = xcalloc(sizeof(*new), 1);
        new->token = tok;
        new->value = val;
        list_add_tail(&new->list, head);
 {
        struct trailer_item *new = xcalloc(sizeof(*new), 1);
        new->token = tok;
        new->value = val;
        list_add_tail(&new->list, head);

+       return new;

 }
 
 static void add_arg_item(struct list_head *arg_head, char *tok, char *val,
 }
 
 static void add_arg_item(struct list_head *arg_head, char *tok, char *val,


@@ -732,6 +734,14 @@ static int find_trailer_start(struct strbuf **lines, int count)
 {
        int start, end_of_title, only_spaces = 1;
        int recognized_prefix = 0, trailer_lines = 0, non_trailer_lines = 0;
 {
        int start, end_of_title, only_spaces = 1;
        int recognized_prefix = 0, trailer_lines = 0, non_trailer_lines = 0;

+       /*
+        * Number of possible continuation lines encountered. This will be
+        * reset to 0 if we encounter a trailer (since those lines are to be
+        * considered continuations of that trailer), and added to
+        * non_trailer_lines if we encounter a non-trailer (since those lines
+        * are to be considered non-trailers).
+        */
+       int possible_continuation_lines = 0;

 
        /* The first paragraph is the title and cannot be trailers */
        for (start = 0; start < count; start++) {
 
        /* The first paragraph is the title and cannot be trailers */
        for (start = 0; start < count; start++) {


@@ -752,11 +762,15 @@ static int find_trailer_start(struct strbuf **lines, int count)
                const char **p;
                int separator_pos;
 
                const char **p;
                int separator_pos;
 

-               if (lines[start]->buf[0] == comment_line_char)
+               if (lines[start]->buf[0] == comment_line_char) {
+                       non_trailer_lines += possible_continuation_lines;
+                       possible_continuation_lines = 0;

                        continue;
                        continue;

+               }

                if (contains_only_spaces(lines[start]->buf)) {
                        if (only_spaces)
                                continue;
                if (contains_only_spaces(lines[start]->buf)) {
                        if (only_spaces)
                                continue;

+                       non_trailer_lines += possible_continuation_lines;

                        if (recognized_prefix &&
                            trailer_lines * 3 >= non_trailer_lines)
                                return start + 1;
                        if (recognized_prefix &&
                            trailer_lines * 3 >= non_trailer_lines)
                                return start + 1;


@@ -769,16 +783,18 @@ static int find_trailer_start(struct strbuf **lines, int count)
                for (p = git_generated_prefixes; *p; p++) {
                        if (starts_with(lines[start]->buf, *p)) {
                                trailer_lines++;
                for (p = git_generated_prefixes; *p; p++) {
                        if (starts_with(lines[start]->buf, *p)) {
                                trailer_lines++;

+                               possible_continuation_lines = 0;

                                recognized_prefix = 1;
                                goto continue_outer_loop;
                        }
                }
 
                                recognized_prefix = 1;
                                goto continue_outer_loop;
                        }
                }
 

-               separator_pos = find_separator(lines[start]->buf);
+               separator_pos = find_separator(lines[start]->buf, separators);

                if (separator_pos >= 1 && !isspace(lines[start]->buf[0])) {
                        struct list_head *pos;
 
                        trailer_lines++;
                if (separator_pos >= 1 && !isspace(lines[start]->buf[0])) {
                        struct list_head *pos;
 
                        trailer_lines++;

+                       possible_continuation_lines = 0;

                        if (recognized_prefix)
                                continue;
                        list_for_each(pos, &conf_head) {
                        if (recognized_prefix)
                                continue;
                        list_for_each(pos, &conf_head) {


@@ -790,8 +806,13 @@ static int find_trailer_start(struct strbuf **lines, int count)
                                        break;
                                }
                        }
                                        break;
                                }
                        }

-               } else
+               } else if (isspace(lines[start]->buf[0]))
+                       possible_continuation_lines++;
+               else {

                        non_trailer_lines++;
                        non_trailer_lines++;

+                       non_trailer_lines += possible_continuation_lines;
+                       possible_continuation_lines = 0;
+               }

 continue_outer_loop:
                ;
        }
 continue_outer_loop:
                ;
        }


@@ -840,6 +861,7 @@ static int process_input_file(FILE *outfile,
        int patch_start, trailer_start, trailer_end, i;
        struct strbuf tok = STRBUF_INIT;
        struct strbuf val = STRBUF_INIT;
        int patch_start, trailer_start, trailer_end, i;
        struct strbuf tok = STRBUF_INIT;
        struct strbuf val = STRBUF_INIT;

+       struct trailer_item *last = NULL;

 
        /* Get the line count */
        while (lines[count])
 
        /* Get the line count */
        while (lines[count])


@@ -860,19 +882,28 @@ static int process_input_file(FILE *outfile,
                int separator_pos;
                if (lines[i]->buf[0] == comment_line_char)
                        continue;
                int separator_pos;
                if (lines[i]->buf[0] == comment_line_char)
                        continue;

+               if (last && isspace(lines[i]->buf[0])) {
+                       struct strbuf sb = STRBUF_INIT;
+                       strbuf_addf(&sb, "%s\n%s", last->value, lines[i]->buf);
+                       strbuf_strip_suffix(&sb, "\n");
+                       free(last->value);
+                       last->value = strbuf_detach(&sb, NULL);
+                       continue;
+               }

                separator_pos = find_separator(lines[i]->buf, separators);
                if (separator_pos >= 1) {
                        parse_trailer(&tok, &val, NULL, lines[i]->buf,
                                      separator_pos);
                separator_pos = find_separator(lines[i]->buf, separators);
                if (separator_pos >= 1) {
                        parse_trailer(&tok, &val, NULL, lines[i]->buf,
                                      separator_pos);

-                       add_trailer_item(head,
-                                        strbuf_detach(&tok, NULL),
-                                        strbuf_detach(&val, NULL));
+                       last = add_trailer_item(head,
+                                               strbuf_detach(&tok, NULL),
+                                               strbuf_detach(&val, NULL));

                } else {
                        strbuf_addbuf(&val, lines[i]);
                        strbuf_strip_suffix(&val, "\n");
                        add_trailer_item(head,
                                         NULL,
                                         strbuf_detach(&val, NULL));
                } else {
                        strbuf_addbuf(&val, lines[i]);
                        strbuf_strip_suffix(&val, "\n");
                        add_trailer_item(head,
                                         NULL,
                                         strbuf_detach(&val, NULL));

+                       last = NULL;

                }
        }
 
                }
        }