undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge branch 'hv/trailer-formatting'

Browse Source 
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
This commit is contained in:
Junio C Hamano 2021-03-01 14:02:57 -08:00
commit 28714238c8
5 changed files with 235 additions and 102 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
Documentation/git-for-each-ref.txt
View File

@ -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`).

98
pretty.c
View File

@ -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);

12
pretty.h
View File

@ -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 */

36
ref-filter.c
View File

@ -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(&params, 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(&params, 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(&params, 0);
return 0;
}

183
t/t6300-for-each-ref.sh
View File

@ -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
'
EOF
test_expect_success '%(trailers) rejects unknown trailers arguments' '
# error message cannot be checked under i18n
cat >expect <<-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 &&