| Message ID | 20211029184512.1568017-3-eschwartz@archlinux.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add some more options to the pretty-formats | expand |
Eli Schwartz <eschwartz@archlinux.org> writes: > + > +** 'tags[=<BOOL>]': Instead of only considering annotated tags, > + consider lightweight tags as well. This part contradicts what Jean-Noël's df34a41f is trying to achieve, which can be seen in these hunks from it: @@ -273,12 +273,12 @@ endif::git-rev-list[] If any option is provided multiple times the last occurrence wins. + -The boolean options accept an optional value `[=<BOOL>]`. The values +The boolean options accept an optional value `[=<value>]`. The values `true`, `false`, `on`, `off` etc. are all accepted. See the "boolean" sub-section in "EXAMPLES" in linkgit:git-config[1]. If a boolean option is given with no value, it's enabled. + -** 'key=<K>': only show trailers with specified key. Matching is done +** 'key=<key>': only show trailers with specified <key>. Matching is done case-insensitively and trailing colon is optional. If option is given multiple times trailer lines matching any of the keys are shown. This option automatically enables the `only` option so that @@ -286,25 +286,25 @@ option is given with no value, it's enabled. desired it can be disabled with `only=false`. E.g., `%(trailers:key=Reviewed-by)` shows trailer lines with key `Reviewed-by`. -** 'only[=<BOOL>]': select whether non-trailer lines from the trailer +** 'only[=<bool-value>]': select whether non-trailer lines from the trailer block should be included. -** 'separator=<SEP>': specify a separator inserted between trailer +** 'separator=<sep>': specify a separator inserted between trailer ... So, let's instead use tags[=<bool-value>]: Instead of only considering ... i.e. lowercase, with -value suffix. Thanks.
On 10/29/21 4:18 PM, Junio C Hamano wrote: > Eli Schwartz <eschwartz@archlinux.org> writes: > >> + >> +** 'tags[=<BOOL>]': Instead of only considering annotated tags, >> + consider lightweight tags as well. > > This part contradicts what Jean-Noël's df34a41f is trying to > achieve, which can be seen in these hunks from it: > > [...] > > So, let's instead use > > tags[=<bool-value>]: Instead of only considering ... > > i.e. lowercase, with -value suffix. An interesting change. I can use that description style, sure. Though I will note the commit message for it talks a lot about replacing spaces with hyphens, and very little about consolidating on case *or* using different language such as: -* 'format:<string>' +* 'format:<format-string>' I also assume that it's fine for my patches to be inconsistent with the base commit, as it's expected df34a41f or some revision of it will be merged around the same time?
Junio C Hamano <gitster@pobox.com> writes: > Eli Schwartz <eschwartz@archlinux.org> writes: > >> + >> +** 'tags[=<BOOL>]': Instead of only considering annotated tags, >> + consider lightweight tags as well. > > This part contradicts what Jean-Noël's df34a41f is trying to > achieve, which can be seen in these hunks from it: > ... > So, let's instead use > > tags[=<bool-value>]: Instead of only considering ... > > i.e. lowercase, with -value suffix. The other topic merges earlier to 'seen' before your topic, and FYI, the diff between the tip of 'seen' before and after your topic gets merged looks like this, with my semantic conflict resolution. Notice the way placeholders are spelled in lowercase and generally have more descriptive names. Thanks. diff --git c/Documentation/pretty-formats.txt w/Documentation/pretty-formats.txt index d465cd59dd..25cfffab38 100644 --- c/Documentation/pretty-formats.txt +++ w/Documentation/pretty-formats.txt @@ -220,6 +220,12 @@ The placeholders are: inconsistent when tags are added or removed at the same time. + +** 'tags[=<bool-value>]': Instead of only considering annotated tags, + consider lightweight tags as well. +** 'abbrev=<number>': Instead of using the default number of hexadecimal digits + (which will vary according to the number of objects in the repository with a + default of 7) of the abbreviated object name, use <number> digits, or as many digits + as needed to form a unique object name. ** 'match=<pattern>': Only consider tags matching the given `glob(7)` pattern, excluding the "refs/tags/" prefix. ** 'exclude=<pattern>': Do not consider tags matching the given @@ -273,11 +279,6 @@ endif::git-rev-list[] If any option is provided multiple times the last occurrence wins. + -The boolean options accept an optional value `[=<value>]`. The values -`true`, `false`, `on`, `off` etc. are all accepted. See the "boolean" -sub-section in "EXAMPLES" in linkgit:git-config[1]. If a boolean -option is given with no value, it's enabled. -+ ** 'key=<key>': only show trailers with specified <key>. Matching is done case-insensitively and trailing colon is optional. If option is given multiple times trailer lines matching any of the keys are @@ -313,6 +314,11 @@ insert an empty string unless we are traversing reflog entries (e.g., by decoration format if `--decorate` was not already provided on the command line. +The boolean options accept an optional value `[=<bool-value>]`. The values +`true`, `false`, `on`, `off` etc. are all accepted. See the "boolean" +sub-section in "EXAMPLES" in linkgit:git-config[1]. If a boolean +option is given with no value, it's enabled. + If you add a `+` (plus sign) after '%' of a placeholder, a line-feed is inserted immediately before the expansion if and only if the placeholder expands to a non-empty string.
On 10/29/21 5:28 PM, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > >> Eli Schwartz <eschwartz@archlinux.org> writes: >> >>> + >>> +** 'tags[=<BOOL>]': Instead of only considering annotated tags, >>> + consider lightweight tags as well. >> >> This part contradicts what Jean-Noël's df34a41f is trying to >> achieve, which can be seen in these hunks from it: >> ... >> So, let's instead use >> >> tags[=<bool-value>]: Instead of only considering ... >> >> i.e. lowercase, with -value suffix. > > The other topic merges earlier to 'seen' before your topic, and FYI, > the diff between the tip of 'seen' before and after your topic gets > merged looks like this, with my semantic conflict resolution. > > Notice the way placeholders are spelled in lowercase and generally > have more descriptive names. > > Thanks. > > diff --git c/Documentation/pretty-formats.txt w/Documentation/pretty-formats.txt > index d465cd59dd..25cfffab38 100644 > --- c/Documentation/pretty-formats.txt > +++ w/Documentation/pretty-formats.txt > @@ -220,6 +220,12 @@ The placeholders are: > inconsistent when tags are added or removed at > the same time. > + > +** 'tags[=<bool-value>]': Instead of only considering annotated tags, > + consider lightweight tags as well. > +** 'abbrev=<number>': Instead of using the default number of hexadecimal digits As a matter of curiosity, why "bool-value" but not "number-value"? Isn't the "value" part implicit?
Eli Schwartz <eschwartz@archlinux.org> writes: > I also assume that it's fine for my patches to be inconsistent with the > base commit, as it's expected df34a41f or some revision of it will be > merged around the same time? Yes, such inconsistencies will be gone when the both topics get merged. You can just assume that the details of what you write may not matter in the end result ;-) Or perhaps the other topic would graduate first, in which case you have a chance to rebase these patches on top of the 'master' that already have the other topic. Your patches would then be consistent with the base commit, as the base would already be cleaned up. Thanks.
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index ef6bd420ae..86ed801aad 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -220,6 +220,8 @@ The placeholders are: inconsistent when tags are added or removed at the same time. + +** 'tags[=<BOOL>]': Instead of only considering annotated tags, + consider lightweight tags as well. ** 'match=<pattern>': Only consider tags matching the given `glob(7)` pattern, excluding the "refs/tags/" prefix. ** 'exclude=<pattern>': Do not consider tags matching the given @@ -273,11 +275,6 @@ endif::git-rev-list[] If any option is provided multiple times the last occurrence wins. + -The boolean options accept an optional value `[=<BOOL>]`. The values -`true`, `false`, `on`, `off` etc. are all accepted. See the "boolean" -sub-section in "EXAMPLES" in linkgit:git-config[1]. If a boolean -option is given with no value, it's enabled. -+ ** 'key=<K>': only show trailers with specified key. Matching is done case-insensitively and trailing colon is optional. If option is given multiple times trailer lines matching any of the keys are @@ -313,6 +310,11 @@ insert an empty string unless we are traversing reflog entries (e.g., by decoration format if `--decorate` was not already provided on the command line. +The boolean options accept an optional value `[=<BOOL>]`. The values +`true`, `false`, `on`, `off` etc. are all accepted. See the "boolean" +sub-section in "EXAMPLES" in linkgit:git-config[1]. If a boolean +option is given with no value, it's enabled. + If you add a `+` (plus sign) after '%' of a placeholder, a line-feed is inserted immediately before the expansion if and only if the placeholder expands to a non-empty string. diff --git a/pretty.c b/pretty.c index 2ec023a0d0..a105ef2a15 100644 --- a/pretty.c +++ b/pretty.c @@ -1218,8 +1218,9 @@ static size_t parse_describe_args(const char *start, struct strvec *args) { struct { char *name; - enum { OPT_STRING } type; + enum { OPT_BOOL, OPT_STRING, } type; } option[] = { + { "tags", OPT_BOOL}, { "exclude", OPT_STRING }, { "match", OPT_STRING }, }; @@ -1229,10 +1230,20 @@ static size_t parse_describe_args(const char *start, struct strvec *args) int found = 0; const char *argval; size_t arglen = 0; + int optval = 0; int i; 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)) { + if (optval) + strvec_pushf(args, "--%s", option[i].name); + else + strvec_pushf(args, "--no-%s", option[i].name); + found = 1; + } + break; case OPT_STRING: if (match_placeholder_arg_value(arg, option[i].name, &arg, &argval, &arglen)) { diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index 5865daa8f8..d4acf8882f 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -1002,4 +1002,12 @@ test_expect_success '%(describe:exclude=...) vs git describe --exclude ...' ' test_cmp expect actual ' +test_expect_success '%(describe:tags) vs git describe --tags' ' + test_when_finished "git tag -d tagname" && + git tag tagname && + git describe --tags >expect && + git log -1 --format="%(describe:tags)" >actual && + test_cmp expect actual +' + test_done
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> --- Documentation/pretty-formats.txt | 12 +++++++----- pretty.c | 13 ++++++++++++- t/t4205-log-pretty-formats.sh | 8 ++++++++ 3 files changed, 27 insertions(+), 6 deletions(-)