Re: [PATCH v2 2/3] pretty: add tag option to %(describe)

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Oct 26, 2021, 05:25 UTC
Message-ID: <CAPig+cTe9iMCteUYZZP_8cYoOzbg-95ptuVdvvk0SKUGMgrDjg@mail.gmail.com>
In-Reply-To: <20211026013452.1372122-3-eschwartz@archlinux.org>

On Mon, Oct 25, 2021 at 9:36 PM Eli Schwartz <eschwartz@archlinux.org> wrote:

Show 15 quoted lines

> The %(describe) placeholder by default, like `git describe`, only
> supports annotated tags. However, some people do use lightweight tags
> for releases, and would like to describe those anyway. The command line
> tool has an option to support this.
>
> Teach the placeholder to support this as well.
>
> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
> ---
> diff --git a/pretty.c b/pretty.c
> @@ -1229,10 +1230,21 @@ static size_t parse_describe_args(const char *start, struct strvec *args)
>                 for (i = 0; !found && i < ARRAY_SIZE(option); i++) {
>                         switch(option[i].type) {
> +                       case OPT_BOOL:
> +                               if(match_placeholder_bool_arg(arg, option[i].name, &arg, &optval)) {

Style nit: add space after `if`

Show 5 quoted lines

> +                                       if (optval) {
> +                                               strvec_pushf(args, "--%s", option[i].name);
> +                                       } else {
> +                                               strvec_pushf(args, "--no-%s", option[i].name);
> +                                       }

We would normally omit the braces for this simple `if`:

    if (optval)
        strvec_pushf(...);
    else
        strvec_pushf(...);

... or maybe even use the ternary operator:

    strvec_pushf(args, "--%s%s", optval ? "" : "no-", option[i].name);

but it's highly subjective whether or not that's more readable.