Re: [PATCH v4 2/7] pretty: allow %(trailers) options with explicit value

[Junio C Hamano <gitster@pobox.com>]

Anders Waldenborg <anders@0x63.nu> writes:

> In addition to old %(trailers:only) it is now allowed to write
> %(trailers:only=yes)

s/$/. Similarly the unfold option can take a boolean./

> By itself this only gives (the not quite so useful) possibility to have
> users change their mind in the middle of a formatting
> string (%(trailers:only=true,only=false)). However, it gives users the
> opportunity to override defaults from future options.

Makes sense.

> +** 'unfold[=val]': make it behave as if interpret-trailer's `--unfold`
> +   option was given. In same way as to for `only` it can be followed
> +   by an equal sign and explicit value. E.g.,
> +   `%(trailers:only,unfold=true)` unfolds and shows all trailer lines.

> +static int match_placeholder_bool_arg(const char *to_parse, const char *candidate,
> +				      const char **end, int *val)
> +{
> +	const char *p;
> +	if (!skip_prefix(to_parse, candidate, &p))
> +		return 0;
> +
> +	if (match_placeholder_arg(p, "=no", end) ||
> +	    match_placeholder_arg(p, "=off", end) ||
> +	    match_placeholder_arg(p, "=false", end)) {
> +		*val = 0;
> +		return 1;
> +	}
> +
> +	if (match_placeholder_arg(p, "", end) ||
> +	    match_placeholder_arg(p, "=yes", end) ||
> +	    match_placeholder_arg(p, "=on", end) ||
> +	    match_placeholder_arg(p, "=true", end)) {
> +		*val = 1;
> +		return 1;
> +	}

Hmph.  Is there a possibility to arrenge the code so that we do not
have to maintain these variants of true/false representations here,
when we should already have one in config.c?

The match_placeholder_arg() function is a bit too limiting as it can
only recognize the value that we know about for a thing like this.
Instead, perhaps we can cut what follows "=" syntactically, looking
for either NUL, ',', or ')', and then call git_parse_maybe_bool() on
it.  That way, we can handle %(trailers:only=bogo) more sensibly,
no?  Syntactically we can recognize that the user wanted to give
'bogo' as the value to 'only', and say "'bogo' is not a boolean" if
we did so.

> +	return 0;
> +}
> +