diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt index b8fafb1e8b..a5e8b36f62 100644 --- a/Documentation/git-interpret-trailers.txt +++ b/Documentation/git-interpret-trailers.txt @@ -56,8 +56,9 @@ least one Git-generated or user-configured trailer and consists of at least 25% trailers. The group must be preceded by one or more empty (or whitespace-only) lines. The group must either be at the end of the message or be the last -non-whitespace lines before a line that starts with '---'. Such three -minus signs start the patch part of the message. +non-whitespace lines before a line that starts with '---' (followed by a +space or the end of the line). Such three minus signs start the patch +part of the message. See also `--no-divider` below. When reading trailers, there can be whitespaces after the token, the separator and the value. There can also be whitespaces @@ -125,6 +126,11 @@ OPTIONS A convenience alias for `--only-trailers --only-input --unfold`. +--no-divider:: + Do not treat `---` as the end of the commit message. Use this + when you know your input contains just the commit message itself + (and not an email or the output of `git format-patch`). + CONFIGURATION VARIABLES ----------------------- diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c index b742539d4d..4b87e0dd2e 100644 --- a/builtin/interpret-trailers.c +++ b/builtin/interpret-trailers.c @@ -104,6 +104,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "unfold", &opts.unfold, N_("join whitespace-continued values")), { OPTION_CALLBACK, 0, "parse", &opts, NULL, N_("set parsing options"), PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_parse }, + OPT_BOOL(0, "no-divider", &opts.no_divider, N_("do not treat --- specially")), OPT_CALLBACK(0, "trailer", &trailers, N_("trailer"), N_("trailer(s) to add"), option_parse_trailer), OPT_END() diff --git a/commit.c b/commit.c index 5781f9bc67..d6f7f5e4d2 100644 --- a/commit.c +++ b/commit.c @@ -1426,10 +1426,10 @@ const char *find_commit_header(const char *msg, const char *key, size_t *out_len * Returns the number of bytes from the tail to ignore, to be fed as * the second parameter to append_signoff(). */ -int ignore_non_trailer(const char *buf, size_t len) +size_t ignore_non_trailer(const char *buf, size_t len) { - int boc = 0; - int bol = 0; + size_t boc = 0; + size_t bol = 0; int in_old_conflicts_block = 0; size_t cutoff = wt_status_locate_end(buf, len); diff --git a/commit.h b/commit.h index e2c99d9b04..2b1a734388 100644 --- a/commit.h +++ b/commit.h @@ -293,7 +293,7 @@ extern const char *find_commit_header(const char *msg, const char *key, size_t *out_len); /* Find the end of the log message, the right place for a new trailer. */ -extern int ignore_non_trailer(const char *buf, size_t len); +extern size_t ignore_non_trailer(const char *buf, size_t len); typedef int (*each_mergetag_fn)(struct commit *commit, struct commit_extra_header *extra, void *cb_data); diff --git a/pretty.c b/pretty.c index 98cf5228f9..8ca29e9281 100644 --- a/pretty.c +++ b/pretty.c @@ -1304,6 +1304,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ if (skip_prefix(placeholder, "(trailers", &arg)) { struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT; + + opts.no_divider = 1; + if (*arg == ':') { arg++; for (;;) { diff --git a/ref-filter.c b/ref-filter.c index 9bacd32f0a..e1bcb4ca8a 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -264,6 +264,8 @@ static int trailers_atom_parser(const struct ref_format *format, struct used_ato struct string_list params = STRING_LIST_INIT_DUP; int i; + atom->u.contents.trailer_opts.no_divider = 1; + if (arg) { string_list_split(¶ms, arg, ',', -1); for (i = 0; i < params.nr; i++) { diff --git a/sequencer.c b/sequencer.c index bd159991e9..9ba873b875 100644 --- a/sequencer.c +++ b/sequencer.c @@ -226,13 +226,16 @@ static const char *get_todo_path(const struct replay_opts *opts) * Returns 3 when sob exists within conforming footer as last entry */ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob, - int ignore_footer) + size_t ignore_footer) { + struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT; struct trailer_info info; - int i; + size_t i; int found_sob = 0, found_sob_last = 0; - trailer_info_get(&info, sb->buf); + opts.no_divider = 1; + + trailer_info_get(&info, sb->buf, &opts); if (info.trailer_start == info.trailer_end) return 0; @@ -3829,7 +3832,7 @@ int sequencer_pick_revisions(struct replay_opts *opts) return res; } -void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag) +void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) { unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP; struct strbuf sob = STRBUF_INIT; diff --git a/sequencer.h b/sequencer.h index c751c9d6e4..c986bc8251 100644 --- a/sequencer.h +++ b/sequencer.h @@ -90,7 +90,14 @@ int rearrange_squash(void); extern const char sign_off_header[]; -void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag); +/* + * Append a signoff to the commit message in "msgbuf". The ignore_footer + * parameter specifies the number of bytes at the end of msgbuf that should + * not be considered at all. I.e., they are not checked for existing trailers, + * and the new signoff will be spliced into the buffer before those bytes. + */ +void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag); + void append_conflicts_hint(struct strbuf *msgbuf); int message_is_empty(const struct strbuf *sb, enum commit_msg_cleanup_mode cleanup_mode); diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index 2052cadb11..978a8a66ff 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -598,4 +598,27 @@ test_expect_success ':only and :unfold work together' ' test_cmp expect actual ' +test_expect_success 'trailer parsing not fooled by --- line' ' + git commit --allow-empty -F - <<-\EOF && + this is the subject + + This is the body. The message has a "---" line which would confuse a + message+patch parser. But here we know we have only a commit message, + so we get it right. + + trailer: wrong + --- + This is more body. + + trailer: right + EOF + + { + echo "trailer: right" && + echo + } >expect && + git log --no-walk --format="%(trailers)" >actual && + test_cmp expect actual +' + test_done diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index 024f8c06f7..97bfbee6e8 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -715,6 +715,29 @@ test_expect_success 'basic atom: head contents:trailers' ' test_cmp expect actual.clean ' +test_expect_success 'trailer parsing not fooled by --- line' ' + git commit --allow-empty -F - <<-\EOF && + this is the subject + + This is the body. The message has a "---" line which would confuse a + message+patch parser. But here we know we have only a commit message, + so we get it right. + + trailer: wrong + --- + This is more body. + + trailer: right + EOF + + { + echo "trailer: right" && + echo + } >expect && + git for-each-ref --format="%(trailers)" refs/heads/master >actual && + test_cmp expect actual +' + test_expect_success 'Add symbolic ref for the following tests' ' git symbolic-ref refs/heads/sym refs/heads/master ' diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh index 4cae92804d..1a6773ee68 100755 --- a/t/t7501-commit.sh +++ b/t/t7501-commit.sh @@ -517,6 +517,22 @@ Myfooter: x" && test_cmp expected actual ' +test_expect_success 'signoff not confused by ---' ' + cat >expected <<-EOF && + subject + + body + --- + these dashes confuse the parser! + + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + EOF + # should be a noop, since we already signed + git commit --allow-empty --signoff -F expected && + git log -1 --pretty=format:%B >actual && + test_cmp expected actual +' + test_expect_success 'multiple -m' ' >negative && diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 164719d1c9..c441861331 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -1417,4 +1417,46 @@ test_expect_success 'unfold' ' test_cmp expected actual ' +test_expect_success 'handling of --- lines in input' ' + echo "real-trailer: just right" >expected && + + git interpret-trailers --parse >actual <<-\EOF && + subject + + body + + not-a-trailer: too soon + ------ this is just a line in the commit message with a bunch of + ------ dashes; it does not have any syntactic meaning. + + real-trailer: just right + --- + below the dashed line may be a patch, etc. + + not-a-trailer: too late + EOF + + test_cmp expected actual +' + +test_expect_success 'suppress --- handling' ' + echo "real-trailer: just right" >expected && + + git interpret-trailers --parse --no-divider >actual <<-\EOF && + subject + + This commit message has a "---" in it, but because we tell + interpret-trailers not to respect that, it has no effect. + + not-a-trailer: too soon + --- + + This is still the commit message body. + + real-trailer: just right + EOF + + test_cmp expected actual +' + test_done diff --git a/trailer.c b/trailer.c index 4e309460d1..0796f326b3 100644 --- a/trailer.c +++ b/trailer.c @@ -585,7 +585,7 @@ static const char *token_from_item(struct arg_item *item, char *tok) return item->conf.name; } -static int token_matches_item(const char *tok, struct arg_item *item, int tok_len) +static int token_matches_item(const char *tok, struct arg_item *item, size_t tok_len) { if (!strncasecmp(tok, item->conf.name, tok_len)) return 1; @@ -603,7 +603,7 @@ static int token_matches_item(const char *tok, struct arg_item *item, int tok_le * distinguished from the non-well-formed-line case (in which this function * returns -1) because some callers of this function need such a distinction. */ -static int find_separator(const char *line, const char *separators) +static ssize_t find_separator(const char *line, const char *separators) { int whitespace_found = 0; const char *c; @@ -630,10 +630,10 @@ static int find_separator(const char *line, const char *separators) */ static void parse_trailer(struct strbuf *tok, struct strbuf *val, const struct conf_info **conf, const char *trailer, - int separator_pos) + ssize_t separator_pos) { struct arg_item *item; - int tok_len; + size_t tok_len; struct list_head *pos; if (separator_pos != -1) { @@ -721,7 +721,7 @@ static void process_command_line_args(struct list_head *arg_head, list_for_each(pos, new_trailer_head) { struct new_trailer_item *tr = list_entry(pos, struct new_trailer_item, list); - int separator_pos = find_separator(tr->text, cl_separators); + ssize_t separator_pos = find_separator(tr->text, cl_separators); if (separator_pos == 0) { struct strbuf sb = STRBUF_INIT; @@ -763,9 +763,9 @@ static const char *next_line(const char *str) /* * Return the position of the start of the last line. If len is 0, return -1. */ -static int last_line(const char *buf, size_t len) +static ssize_t last_line(const char *buf, size_t len) { - int i; + ssize_t i; if (len == 0) return -1; if (len == 1) @@ -788,12 +788,14 @@ static int last_line(const char *buf, size_t len) * Return the position of the start of the patch or the length of str if there * is no patch in the message. */ -static int find_patch_start(const char *str) +static size_t find_patch_start(const char *str) { const char *s; for (s = str; *s; s = next_line(s)) { - if (starts_with(s, "---")) + const char *v; + + if (skip_prefix(s, "---", &v) && isspace(*v)) return s - str; } @@ -804,10 +806,11 @@ static int find_patch_start(const char *str) * Return the position of the first trailer line or len if there are no * trailers. */ -static int find_trailer_start(const char *buf, size_t len) +static size_t find_trailer_start(const char *buf, size_t len) { const char *s; - int end_of_title, l, only_spaces = 1; + ssize_t end_of_title, l; + int only_spaces = 1; int recognized_prefix = 0, trailer_lines = 0, non_trailer_lines = 0; /* * Number of possible continuation lines encountered. This will be @@ -838,7 +841,7 @@ static int find_trailer_start(const char *buf, size_t len) l = last_line(buf, l)) { const char *bol = buf + l; const char **p; - int separator_pos; + ssize_t separator_pos; if (bol[0] == comment_line_char) { non_trailer_lines += possible_continuation_lines; @@ -899,14 +902,14 @@ static int find_trailer_start(const char *buf, size_t len) } /* Return the position of the end of the trailers. */ -static int find_trailer_end(const char *buf, size_t len) +static size_t find_trailer_end(const char *buf, size_t len) { return len - ignore_non_trailer(buf, len); } static int ends_with_blank_line(const char *buf, size_t len) { - int ll = last_line(buf, len); + ssize_t ll = last_line(buf, len); if (ll < 0) return 0; return is_blank_line(buf + ll); @@ -939,17 +942,17 @@ static void unfold_value(struct strbuf *val) strbuf_release(&out); } -static int process_input_file(FILE *outfile, - const char *str, - struct list_head *head, - const struct process_trailer_options *opts) +static size_t process_input_file(FILE *outfile, + const char *str, + struct list_head *head, + const struct process_trailer_options *opts) { struct trailer_info info; struct strbuf tok = STRBUF_INIT; struct strbuf val = STRBUF_INIT; - int i; + size_t i; - trailer_info_get(&info, str); + trailer_info_get(&info, str, opts); /* Print lines before the trailers as is */ if (!opts->only_trailers) @@ -1032,7 +1035,7 @@ void process_trailers(const char *file, { LIST_HEAD(head); struct strbuf sb = STRBUF_INIT; - int trailer_end; + size_t trailer_end; FILE *outfile = stdout; ensure_configured(); @@ -1066,7 +1069,8 @@ void process_trailers(const char *file, strbuf_release(&sb); } -void trailer_info_get(struct trailer_info *info, const char *str) +void trailer_info_get(struct trailer_info *info, const char *str, + const struct process_trailer_options *opts) { int patch_start, trailer_end, trailer_start; struct strbuf **trailer_lines, **ptr; @@ -1076,7 +1080,11 @@ void trailer_info_get(struct trailer_info *info, const char *str) ensure_configured(); - patch_start = find_patch_start(str); + if (opts->no_divider) + patch_start = strlen(str); + else + patch_start = find_patch_start(str); + trailer_end = find_trailer_end(str, patch_start); trailer_start = find_trailer_start(str, trailer_end); @@ -1111,7 +1119,7 @@ void trailer_info_get(struct trailer_info *info, const char *str) void trailer_info_release(struct trailer_info *info) { - int i; + size_t i; for (i = 0; i < info->trailer_nr; i++) free(info->trailers[i]); free(info->trailers); @@ -1121,7 +1129,7 @@ static void format_trailer_info(struct strbuf *out, const struct trailer_info *info, const struct process_trailer_options *opts) { - int i; + size_t i; /* If we want the whole block untouched, we can take the fast path. */ if (!opts->only_trailers && !opts->unfold) { @@ -1132,7 +1140,7 @@ static void format_trailer_info(struct strbuf *out, for (i = 0; i < info->trailer_nr; i++) { char *trailer = info->trailers[i]; - int separator_pos = find_separator(trailer, separators); + ssize_t separator_pos = find_separator(trailer, separators); if (separator_pos >= 1) { struct strbuf tok = STRBUF_INIT; @@ -1158,7 +1166,7 @@ void format_trailers_from_commit(struct strbuf *out, const char *msg, { struct trailer_info info; - trailer_info_get(&info, msg); + trailer_info_get(&info, msg, opts); format_trailer_info(out, &info, opts); trailer_info_release(&info); } diff --git a/trailer.h b/trailer.h index 9c10026c35..b997739649 100644 --- a/trailer.h +++ b/trailer.h @@ -71,6 +71,7 @@ struct process_trailer_options { int only_trailers; int only_input; int unfold; + int no_divider; }; #define PROCESS_TRAILER_OPTIONS_INIT {0} @@ -79,7 +80,8 @@ void process_trailers(const char *file, const struct process_trailer_options *opts, struct list_head *new_trailer_head); -void trailer_info_get(struct trailer_info *info, const char *str); +void trailer_info_get(struct trailer_info *info, const char *str, + const struct process_trailer_options *opts); void trailer_info_release(struct trailer_info *info);