diff options
author | Junio C Hamano <gitster@pobox.com> | 2021-03-02 01:02:57 +0300 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2021-03-02 01:02:58 +0300 |
commit | 28714238c8d482345ca17e8dd98d8cc328de24a8 (patch) | |
tree | 9e6d014a41e4298f4bb8793f1a51f906e44d4728 | |
parent | 18aabfaee50058237829c0aea8d4c50efd8078aa (diff) | |
parent | ee82a487f68a7c86551fb59f6176812d63be61b6 (diff) |
Merge branch 'hv/trailer-formatting'
The logic to handle "trailer" related placeholders in the
"--format=" mechanisms in the "log" family and "for-each-ref"
family is getting unified.
* hv/trailer-formatting:
ref-filter: use pretty.c logic for trailers
pretty.c: capture invalid trailer argument
pretty.c: refactor trailer logic to `format_set_trailers_options()`
t6300: use function to test trailer options
-rw-r--r-- | Documentation/git-for-each-ref.txt | 8 | ||||
-rw-r--r-- | pretty.c | 98 | ||||
-rw-r--r-- | pretty.h | 12 | ||||
-rw-r--r-- | ref-filter.c | 36 | ||||
-rwxr-xr-x | t/t6300-for-each-ref.sh | 185 |
5 files changed, 236 insertions, 103 deletions
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 2962f85a50..2ae2478de7 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -260,11 +260,9 @@ contents:lines=N:: The first `N` lines of the message. Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1] -are obtained as `trailers` (or by using the historical alias -`contents:trailers`). Non-trailer lines from the trailer block can be omitted -with `trailers:only`. Whitespace-continuations can be removed from trailers so -that each trailer appears on a line by itself with its full content with -`trailers:unfold`. Both can be used together as `trailers:unfold,only`. +are obtained as `trailers[:options]` (or by using the historical alias +`contents:trailers[:options]`). For valid [:option] values see `trailers` +section of linkgit:git-log[1]. For sorting purposes, fields with numeric values sort in numeric order (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`). @@ -1149,6 +1149,63 @@ static int format_trailer_match_cb(const struct strbuf *key, void *ud) return 0; } +int format_set_trailers_options(struct process_trailer_options *opts, + struct string_list *filter_list, + struct strbuf *sepbuf, + struct strbuf *kvsepbuf, + const char **arg, + char **invalid_arg) +{ + for (;;) { + const char *argval; + size_t arglen; + + if (**arg == ')') + break; + + if (match_placeholder_arg_value(*arg, "key", arg, &argval, &arglen)) { + uintptr_t len = arglen; + + if (!argval) + return -1; + + if (len && argval[len - 1] == ':') + len--; + string_list_append(filter_list, argval)->util = (char *)len; + + opts->filter = format_trailer_match_cb; + opts->filter_data = filter_list; + opts->only_trailers = 1; + } else if (match_placeholder_arg_value(*arg, "separator", arg, &argval, &arglen)) { + char *fmt; + + strbuf_reset(sepbuf); + fmt = xstrndup(argval, arglen); + strbuf_expand(sepbuf, fmt, strbuf_expand_literal_cb, NULL); + free(fmt); + opts->separator = sepbuf; + } else if (match_placeholder_arg_value(*arg, "key_value_separator", arg, &argval, &arglen)) { + char *fmt; + + strbuf_reset(kvsepbuf); + fmt = xstrndup(argval, arglen); + strbuf_expand(kvsepbuf, fmt, strbuf_expand_literal_cb, NULL); + free(fmt); + opts->key_value_separator = kvsepbuf; + } else if (!match_placeholder_bool_arg(*arg, "only", arg, &opts->only_trailers) && + !match_placeholder_bool_arg(*arg, "unfold", arg, &opts->unfold) && + !match_placeholder_bool_arg(*arg, "keyonly", arg, &opts->key_only) && + !match_placeholder_bool_arg(*arg, "valueonly", arg, &opts->value_only)) { + if (invalid_arg) { + size_t len = strcspn(*arg, ",)"); + *invalid_arg = xstrndup(*arg, len); + } + return -1; + } + } + return 0; +} + static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ const char *placeholder, void *context) @@ -1429,45 +1486,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ if (*arg == ':') { arg++; - for (;;) { - const char *argval; - size_t arglen; - - if (match_placeholder_arg_value(arg, "key", &arg, &argval, &arglen)) { - uintptr_t len = arglen; - - if (!argval) - goto trailer_out; - - if (len && argval[len - 1] == ':') - len--; - string_list_append(&filter_list, argval)->util = (char *)len; - - opts.filter = format_trailer_match_cb; - opts.filter_data = &filter_list; - opts.only_trailers = 1; - } else if (match_placeholder_arg_value(arg, "separator", &arg, &argval, &arglen)) { - char *fmt; - - strbuf_reset(&sepbuf); - fmt = xstrndup(argval, arglen); - strbuf_expand(&sepbuf, fmt, strbuf_expand_literal_cb, NULL); - free(fmt); - opts.separator = &sepbuf; - } else if (match_placeholder_arg_value(arg, "key_value_separator", &arg, &argval, &arglen)) { - char *fmt; - - strbuf_reset(&kvsepbuf); - fmt = xstrndup(argval, arglen); - strbuf_expand(&kvsepbuf, fmt, strbuf_expand_literal_cb, NULL); - free(fmt); - opts.key_value_separator = &kvsepbuf; - } else if (!match_placeholder_bool_arg(arg, "only", &arg, &opts.only_trailers) && - !match_placeholder_bool_arg(arg, "unfold", &arg, &opts.unfold) && - !match_placeholder_bool_arg(arg, "keyonly", &arg, &opts.key_only) && - !match_placeholder_bool_arg(arg, "valueonly", &arg, &opts.value_only)) - break; - } + if (format_set_trailers_options(&opts, &filter_list, &sepbuf, &kvsepbuf, &arg, NULL)) + goto trailer_out; } if (*arg == ')') { format_trailers_from_commit(sb, msg + c->subject_off, &opts); @@ -6,6 +6,7 @@ struct commit; struct strbuf; +struct process_trailer_options; /* Commit formats */ enum cmit_fmt { @@ -142,4 +143,15 @@ int commit_format_is_empty(enum cmit_fmt); /* Make subject of commit message suitable for filename */ void format_sanitized_subject(struct strbuf *sb, const char *msg, size_t len); +/* + * Set values of fields in "struct process_trailer_options" + * according to trailers arguments. + */ +int format_set_trailers_options(struct process_trailer_options *opts, + struct string_list *filter_list, + struct strbuf *sepbuf, + struct strbuf *kvsepbuf, + const char **arg, + char **invalid_arg); + #endif /* PRETTY_H */ diff --git a/ref-filter.c b/ref-filter.c index bade6528ee..e84efb53db 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -67,6 +67,12 @@ struct refname_atom { int lstrip, rstrip; }; +static struct ref_trailer_buf { + struct string_list filter_list; + struct strbuf sepbuf; + struct strbuf kvsepbuf; +} ref_trailer_buf = {STRING_LIST_INIT_NODUP, STRBUF_INIT, STRBUF_INIT}; + static struct expand_data { struct object_id oid; enum object_type type; @@ -313,28 +319,26 @@ static int subject_atom_parser(const struct ref_format *format, struct used_atom static int trailers_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { - 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++) { - const char *s = params.items[i].string; - if (!strcmp(s, "unfold")) - atom->u.contents.trailer_opts.unfold = 1; - else if (!strcmp(s, "only")) - atom->u.contents.trailer_opts.only_trailers = 1; - else { - strbuf_addf(err, _("unknown %%(trailers) argument: %s"), s); - string_list_clear(¶ms, 0); - return -1; - } + const char *argbuf = xstrfmt("%s)", arg); + char *invalid_arg = NULL; + + if (format_set_trailers_options(&atom->u.contents.trailer_opts, + &ref_trailer_buf.filter_list, + &ref_trailer_buf.sepbuf, + &ref_trailer_buf.kvsepbuf, + &argbuf, &invalid_arg)) { + if (!invalid_arg) + strbuf_addf(err, _("expected %%(trailers:key=<value>)")); + else + strbuf_addf(err, _("unknown %%(trailers) argument: %s"), invalid_arg); + free((char *)invalid_arg); + return -1; } } atom->u.contents.option = C_TRAILERS; - string_list_clear(¶ms, 0); return 0; } diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index ca62e764b5..cac7f443d0 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -814,53 +814,152 @@ test_expect_success 'set up trailers for next test' ' EOF ' -test_expect_success '%(trailers:unfold) unfolds trailers' ' - { - unfold <trailers - echo - } >expect && - git for-each-ref --format="%(trailers:unfold)" refs/heads/main >actual && - test_cmp expect actual && - git for-each-ref --format="%(contents:trailers:unfold)" refs/heads/main >actual && - test_cmp expect actual -' +test_trailer_option () { + title=$1 option=$2 + cat >expect + test_expect_success "$title" ' + git for-each-ref --format="%($option)" refs/heads/main >actual && + test_cmp expect actual && + git for-each-ref --format="%(contents:$option)" refs/heads/main >actual && + test_cmp expect actual + ' +} -test_expect_success '%(trailers:only) shows only "key: value" trailers' ' - { - grep -v patch.description <trailers && - echo - } >expect && - git for-each-ref --format="%(trailers:only)" refs/heads/main >actual && - test_cmp expect actual && - git for-each-ref --format="%(contents:trailers:only)" refs/heads/main >actual && - test_cmp expect actual -' +test_trailer_option '%(trailers:unfold) unfolds trailers' \ + 'trailers:unfold' <<-EOF + $(unfold <trailers) -test_expect_success '%(trailers:only) and %(trailers:unfold) work together' ' - { - grep -v patch.description <trailers | unfold && - echo - } >expect && - git for-each-ref --format="%(trailers:only,unfold)" refs/heads/main >actual && - test_cmp expect actual && - git for-each-ref --format="%(trailers:unfold,only)" refs/heads/main >actual && - test_cmp actual actual && - git for-each-ref --format="%(contents:trailers:only,unfold)" refs/heads/main >actual && - test_cmp expect actual && - git for-each-ref --format="%(contents:trailers:unfold,only)" refs/heads/main >actual && - test_cmp actual actual -' - -test_expect_success '%(trailers) rejects unknown trailers arguments' ' - # error message cannot be checked under i18n - cat >expect <<-EOF && + EOF + +test_trailer_option '%(trailers:only) shows only "key: value" trailers' \ + 'trailers:only' <<-EOF + $(grep -v patch.description <trailers) + + EOF + +test_trailer_option '%(trailers:only=no,only=true) shows only "key: value" trailers' \ + 'trailers:only=no,only=true' <<-EOF + $(grep -v patch.description <trailers) + + EOF + +test_trailer_option '%(trailers:only=yes) shows only "key: value" trailers' \ + 'trailers:only=yes' <<-EOF + $(grep -v patch.description <trailers) + + EOF + +test_trailer_option '%(trailers:only=no) shows all trailers' \ + 'trailers:only=no' <<-EOF + $(cat trailers) + + EOF + +test_trailer_option '%(trailers:only) and %(trailers:unfold) work together' \ + 'trailers:only,unfold' <<-EOF + $(grep -v patch.description <trailers | unfold) + + EOF + +test_trailer_option '%(trailers:unfold) and %(trailers:only) work together' \ + 'trailers:unfold,only' <<-EOF + $(grep -v patch.description <trailers | unfold) + + EOF + +test_trailer_option '%(trailers:key=foo) shows that trailer' \ + 'trailers:key=Signed-off-by' <<-EOF + Signed-off-by: A U Thor <author@example.com> + + EOF + +test_trailer_option '%(trailers:key=foo) is case insensitive' \ + 'trailers:key=SiGned-oFf-bY' <<-EOF + Signed-off-by: A U Thor <author@example.com> + + EOF + +test_trailer_option '%(trailers:key=foo:) trailing colon also works' \ + 'trailers:key=Signed-off-by:' <<-EOF + Signed-off-by: A U Thor <author@example.com> + + EOF + +test_trailer_option '%(trailers:key=foo) multiple keys' \ + 'trailers:key=Reviewed-by:,key=Signed-off-by' <<-EOF + Reviewed-by: A U Thor <author@example.com> + Signed-off-by: A U Thor <author@example.com> + + EOF + +test_trailer_option '%(trailers:key=nonexistent) becomes empty' \ + 'trailers:key=Shined-off-by:' <<-EOF + + EOF + +test_trailer_option '%(trailers:key=foo) handles multiple lines even if folded' \ + 'trailers:key=Acked-by' <<-EOF + $(grep -v patch.description <trailers | grep -v Signed-off-by | grep -v Reviewed-by) + + EOF + +test_trailer_option '%(trailers:key=foo,unfold) properly unfolds' \ + 'trailers:key=Signed-Off-by,unfold' <<-EOF + $(unfold <trailers | grep Signed-off-by) + + EOF + +test_trailer_option '%(trailers:key=foo,only=no) also includes nontrailer lines' \ + 'trailers:key=Signed-off-by,only=no' <<-EOF + Signed-off-by: A U Thor <author@example.com> + $(grep patch.description <trailers) + + EOF + +test_trailer_option '%(trailers:key=foo,valueonly) shows only value' \ + 'trailers:key=Signed-off-by,valueonly' <<-EOF + A U Thor <author@example.com> + + EOF + +test_trailer_option '%(trailers:separator) changes separator' \ + 'trailers:separator=%x2C,key=Reviewed-by,key=Signed-off-by:' <<-EOF + Reviewed-by: A U Thor <author@example.com>,Signed-off-by: A U Thor <author@example.com> + EOF + +test_trailer_option '%(trailers:key_value_separator) changes key-value separator' \ + 'trailers:key_value_separator=%x2C,key=Reviewed-by,key=Signed-off-by:' <<-EOF + Reviewed-by,A U Thor <author@example.com> + Signed-off-by,A U Thor <author@example.com> + + EOF + +test_trailer_option '%(trailers:separator,key_value_separator) changes both separators' \ + 'trailers:separator=%x2C,key_value_separator=%x2C,key=Reviewed-by,key=Signed-off-by:' <<-EOF + Reviewed-by,A U Thor <author@example.com>,Signed-off-by,A U Thor <author@example.com> + EOF + +test_failing_trailer_option () { + title=$1 option=$2 + cat >expect + test_expect_success "$title" ' + # error message cannot be checked under i18n + test_must_fail git for-each-ref --format="%($option)" refs/heads/main 2>actual && + test_i18ncmp expect actual && + test_must_fail git for-each-ref --format="%(contents:$option)" refs/heads/main 2>actual && + test_i18ncmp expect actual + ' +} + +test_failing_trailer_option '%(trailers) rejects unknown trailers arguments' \ + 'trailers:unsupported' <<-\EOF fatal: unknown %(trailers) argument: unsupported EOF - test_must_fail git for-each-ref --format="%(trailers:unsupported)" 2>actual && - test_i18ncmp expect actual && - test_must_fail git for-each-ref --format="%(contents:trailers:unsupported)" 2>actual && - test_i18ncmp expect actual -' + +test_failing_trailer_option '%(trailers:key) without value is error' \ + 'trailers:key' <<-\EOF + fatal: expected %(trailers:key=<value>) + EOF test_expect_success 'if arguments, %(contents:trailers) shows error if colon is missing' ' cat >expect <<-EOF && |