Taylor Blau <m...@ttaylorr.com> writes:
> In preparation for adding consistent "%(trailers)" atom options to
> `git-for-each-ref(1)`'s "--format" argument, change "%(trailers)" in
> pretty.c to separate sub-arguments with a ",", instead of a ":".
>
> Multiple sub-arguments are given either as "%(trailers:unfold,only)" or
> "%(trailers:only,unfold)".
>
> This change disambiguates between "top-level" arguments, and arguments
> given to the trailers atom itself. It is consistent with the behavior of
> "%(upstream)" and "%(push)" atoms.
Looks good.
Ignore the remainder unless you are interested in the recent "make
style" discussion.
>
> Signed-off-by: Taylor Blau <m...@ttaylorr.com>
> ---
> pretty.c | 34 +++++++++++++++++++++++++++++-----
> t/t4205-log-pretty-formats.sh | 4 ++--
> 2 files changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/pretty.c b/pretty.c
> index 94eab5c89..eec128bc1 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1056,6 +1056,25 @@ static size_t parse_padding_placeholder(struct strbuf
> *sb,
> return 0;
> }
>
> +static int match_placeholder_arg(const char *to_parse,
> + const char *candidate,
> + const char **end)
"make style" wants to format this like so:
static int match_placeholder_arg(const char *to_parse, const char *candidate,
const char **end)
I think I can live with either versions ;-)
> +{
> + const char *p;
> + if (!(skip_prefix(to_parse, candidate, &p)))
> + return 0;
> + if (*p == ',') {
> + *end = p + 1;
> + return 1;
> + }
> + if (*p == ')') {
> + *end = p;
> + return 1;
> + }
> + return 0;
> +}
> +
> +
"make style" seems to be unhappy to see double blank here, and I
tend to agree.
> static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
> const char *placeholder,
> void *context)
> @@ -1285,11 +1304,16 @@ 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;
> - while (*arg == ':') {
> - if (skip_prefix(arg, ":only", &arg))
> - opts.only_trailers = 1;
> - else if (skip_prefix(arg, ":unfold", &arg))
> - opts.unfold = 1;
> + if (*arg == ':') {
> + arg++;
> + for (;;) {
> + if (match_placeholder_arg(arg, "only", &arg))
> + opts.only_trailers = 1;
> + else if (match_placeholder_arg(arg, "unfold",
> &arg))
> + opts.unfold = 1;
> + else
> + break;
> + }
> }
> if (*arg == ')') {
> format_trailers_from_commit(sb, msg + c->subject_off,
> &opts);
> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
> index ec5f53010..977472f53 100755
> --- a/t/t4205-log-pretty-formats.sh
> +++ b/t/t4205-log-pretty-formats.sh
> @@ -588,8 +588,8 @@ test_expect_success '%(trailers:unfold) unfolds trailers'
> '
> '
>
> test_expect_success ':only and :unfold work together' '
> - git log --no-walk --pretty="%(trailers:only:unfold)" >actual &&
> - git log --no-walk --pretty="%(trailers:unfold:only)" >reverse &&
> + git log --no-walk --pretty="%(trailers:only,unfold)" >actual &&
> + git log --no-walk --pretty="%(trailers:unfold,only)" >reverse &&
> test_cmp actual reverse &&
> {
> grep -v patch.description <trailers | unfold &&
