On 10/26/21 1:25 AM, Eric Sunshine wrote:
On Mon, Oct 25, 2021 at 9:36 PM Eli Schwartz [off-list ref] wrote:
quoted
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 <redacted>
---
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`
Oops, I am not sure how this happened. It's wrong in the switch too.
quoted
+ 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.
Although the braces feel more natural to me for clarity purposes, it's a
good point that the git coding style says to omit them for single
statements, and I should have followed that here.
The ternary doesn't feel readable to me, however.
...
Thanks for the style review!
--
Eli Schwartz
Arch Linux Bug Wrangler and Trusted User