Re: [PATCH v4 1/6] pretty.c: delimit "%(trailers)" arguments with ","
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 2, 2017, 00:11 UTC
- Message-ID
- <xmqqbmlqtnll.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20171001161852.84571-1-me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
Show 10 quoted lines
> 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.
Show 18 quoted lines
> > Signed-off-by: Taylor Blau <me@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 ;-)
Show 16 quoted lines
> +{
> + 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.
Show 40 quoted lines
> 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 &&