* [PATCH 0/2] add format specifiers to display trailers @ 2016-11-18 23:08 Jacob Keller 2016-11-18 23:08 ` [PATCH 1/2] pretty: add %bT format for displaying trailers of a commit message Jacob Keller ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Jacob Keller @ 2016-11-18 23:08 UTC (permalink / raw) To: git; +Cc: Jacob Keller From: Jacob Keller <jacob.keller@gmail.com> This is based off of jt/use-trailer-api-in-commands so that we can make use of the public trailer API that will parse a string for trailers. I use trailers as a way to store extra commit metadata, and would like a convenient way to obtain the trailers of a commit message easily. This adds format specifiers to both the ref-filter API and the pretty formats. I am not a fan of %bT but %t and %T were already taken. I don't really know if it's ok to use %bT, since I think we used to allow "%bT" format, though i don't think this is likely used much in practice. I am open to suggestions for the pretty format specifier. Additionally, I am somewhat not a fan of the way that if you have a series of trailers which are trailer format, but not recognized, such as the following: <text> My-tag: my value My-other-tag: my other value [non-trailer line] My-tag: my third value --- Git interpret-trailers will not recognize this as a trailer block because it doesn't have any standard git tags within it. Would it be ok to augment the trailer interpretation to say that if we have over 75% trailers in the block that we accept it even if it doesn't have any real recognized tags? I say this because I regularly use extra tags in my git projects to represent change metadata, and it would be nice if the tag block could be recognized even if it has 1-2 lines of non-trailer formatting in it... Thoughts? Jacob Keller (2): pretty: add %bT format for displaying trailers of a commit message ref-filter: add support to display trailers as part of contents Documentation/git-for-each-ref.txt | 2 ++ Documentation/pretty-formats.txt | 1 + pretty.c | 18 ++++++++++++++++++ ref-filter.c | 22 +++++++++++++++++++++- t/t4205-log-pretty-formats.sh | 26 ++++++++++++++++++++++++++ t/t6300-for-each-ref.sh | 26 ++++++++++++++++++++++++++ 6 files changed, 94 insertions(+), 1 deletion(-) -- 2.11.0.rc2.152.g4d04e67 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] pretty: add %bT format for displaying trailers of a commit message 2016-11-18 23:08 [PATCH 0/2] add format specifiers to display trailers Jacob Keller @ 2016-11-18 23:08 ` Jacob Keller 2016-11-18 23:08 ` [PATCH 2/2] ref-filter: add support to display trailers as part of contents Jacob Keller 2016-11-18 23:38 ` [PATCH 0/2] add format specifiers to display trailers Junio C Hamano 2 siblings, 0 replies; 9+ messages in thread From: Jacob Keller @ 2016-11-18 23:08 UTC (permalink / raw) To: git; +Cc: Jacob Keller From: Jacob Keller <jacob.keller@gmail.com> Recent patches have expanded on the trailers.c code and we have the builtin commant git-interpret-trailers which can be used to add or modify trailer lines. However, there is no easy way to simply display the trailers of a commit message. Add support for %bT format modifier which will use the trailer_info_get() calls to read trailers in an identical way as git interpret-trailers does. Add documentation and tests for the same. Signed-off-by: Jacob Keller <jacob.keller@gmail.com> --- Documentation/pretty-formats.txt | 1 + pretty.c | 18 ++++++++++++++++++ t/t4205-log-pretty-formats.sh | 26 ++++++++++++++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index 3bcee2ddb124..9ee68a4cb64a 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -138,6 +138,7 @@ The placeholders are: - '%s': subject - '%f': sanitized subject line, suitable for a filename - '%b': body +- '%bT': trailers of body as interpreted by linkgit:git-interpret-trailers[1] - '%B': raw body (unwrapped subject and body) ifndef::git-rev-list[] - '%N': commit notes diff --git a/pretty.c b/pretty.c index 37b2c3b1f995..ea8764334865 100644 --- a/pretty.c +++ b/pretty.c @@ -10,6 +10,7 @@ #include "color.h" #include "reflog-walk.h" #include "gpg-interface.h" +#include "trailer.h" static char *user_format; static struct cmt_fmt_map { @@ -889,6 +890,16 @@ const char *format_subject(struct strbuf *sb, const char *msg, return msg; } +static void format_trailers(struct strbuf *sb, const char *msg) +{ + struct trailer_info info; + + trailer_info_get(&info, msg); + strbuf_add(sb, info.trailer_start, + info.trailer_end - info.trailer_start); + trailer_info_release(&info); +} + static void parse_commit_message(struct format_commit_context *c) { const char *msg = c->message + c->message_off; @@ -1289,6 +1300,13 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ format_sanitized_subject(sb, msg + c->subject_off); return 1; case 'b': /* body */ + switch (placeholder[1]) { + case 'T': + format_trailers(sb, msg + c->subject_off); + return 2; + default: + break; + } strbuf_addstr(sb, msg + c->body_off); return 1; } diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index f5435fd250ba..7a35941ddcbd 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -535,4 +535,30 @@ test_expect_success 'clean log decoration' ' test_cmp expected actual1 ' +cat >trailers <<EOF +Signed-off-by: A U Thor <author@example.com> +Acked-by: A U Thor <author@example.com> +[ v2 updated patch description ] +Signed-off-by: A U Thor <author@example.com> +EOF + +test_expect_success 'pretty format %bT shows trailers' ' + echo "Some contents" >trailerfile && + git add trailerfile && + git commit -F - <<-EOF && + trailers: this commit message has trailers + + This commit is a test commit with trailers at the end. We parse this + message and display the trailers using %bT + + $(cat trailers) + EOF + git log --no-walk --pretty="%bT" >actual && + cat >expect <<-EOF && + $(cat trailers) + + EOF + test_cmp expect actual +' + test_done -- 2.11.0.rc2.152.g4d04e67 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] ref-filter: add support to display trailers as part of contents 2016-11-18 23:08 [PATCH 0/2] add format specifiers to display trailers Jacob Keller 2016-11-18 23:08 ` [PATCH 1/2] pretty: add %bT format for displaying trailers of a commit message Jacob Keller @ 2016-11-18 23:08 ` Jacob Keller 2016-11-18 23:38 ` [PATCH 0/2] add format specifiers to display trailers Junio C Hamano 2 siblings, 0 replies; 9+ messages in thread From: Jacob Keller @ 2016-11-18 23:08 UTC (permalink / raw) To: git; +Cc: Jacob Keller From: Jacob Keller <jacob.keller@gmail.com> Add %(trailers) and %(contents:trailers) to display the trailers as interpreted by trailer_info_get. Update documentation and add a test for the new feature. Signed-off-by: Jacob Keller <jacob.keller@gmail.com> --- Documentation/git-for-each-ref.txt | 2 ++ ref-filter.c | 22 +++++++++++++++++++++- t/t6300-for-each-ref.sh | 26 ++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index f57e69bc83e3..e5807eede787 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -165,6 +165,8 @@ of all lines of the commit message up to the first blank line. The next line is 'contents:body', where body is all of the lines after the first blank line. The optional GPG signature is `contents:signature`. The first `N` lines of the message is obtained using `contents:lines=N`. +Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1] +are obtained as 'contents:trailers'. For sorting purposes, fields with numeric values sort in numeric order (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`). diff --git a/ref-filter.c b/ref-filter.c index d4c2931f3aab..b6f1bb73ed37 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -13,6 +13,7 @@ #include "utf8.h" #include "git-compat-util.h" #include "version.h" +#include "trailer.h" typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type; @@ -40,7 +41,7 @@ static struct used_atom { enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT } remote_ref; struct { - enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB } option; + enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB, C_TRAILERS } option; unsigned int nlines; } contents; enum { O_FULL, O_SHORT } objectname; @@ -85,6 +86,13 @@ static void subject_atom_parser(struct used_atom *atom, const char *arg) atom->u.contents.option = C_SUB; } +static void trailers_atom_parser(struct used_atom *atom, const char *arg) +{ + if (arg) + die(_("%%(trailers) does not take arguments")); + atom->u.contents.option = C_TRAILERS; +} + static void contents_atom_parser(struct used_atom *atom, const char *arg) { if (!arg) @@ -95,6 +103,8 @@ static void contents_atom_parser(struct used_atom *atom, const char *arg) atom->u.contents.option = C_SIG; else if (!strcmp(arg, "subject")) atom->u.contents.option = C_SUB; + else if (!strcmp(arg, "trailers")) + atom->u.contents.option = C_TRAILERS; else if (skip_prefix(arg, "lines=", &arg)) { atom->u.contents.option = C_LINES; if (strtoul_ui(arg, 10, &atom->u.contents.nlines)) @@ -194,6 +204,7 @@ static struct { { "creatordate", FIELD_TIME }, { "subject", FIELD_STR, subject_atom_parser }, { "body", FIELD_STR, body_atom_parser }, + { "trailers", FIELD_STR, trailers_atom_parser }, { "contents", FIELD_STR, contents_atom_parser }, { "upstream", FIELD_STR, remote_ref_atom_parser }, { "push", FIELD_STR, remote_ref_atom_parser }, @@ -785,6 +796,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj name++; if (strcmp(name, "subject") && strcmp(name, "body") && + strcmp(name, "trailers") && !starts_with(name, "contents")) continue; if (!subpos) @@ -808,6 +820,14 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj /* Size is the length of the message after removing the signature */ append_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines); v->s = strbuf_detach(&s, NULL); + } else if (atom->u.contents.option == C_TRAILERS) { + struct trailer_info info; + + /* Search for trailer info */ + trailer_info_get(&info, subpos); + v->s = xmemdupz(info.trailer_start, + info.trailer_end - info.trailer_start); + trailer_info_release(&info); } else if (atom->u.contents.option == C_BARE) v->s = xstrdup(subpos); } diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index 19a2823025e7..eb4bac0fe477 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -553,4 +553,30 @@ test_expect_success 'Verify sort with multiple keys' ' refs/tags/bogo refs/tags/master > actual && test_cmp expected actual ' + +cat >trailers <<EOF +Reviewed-by: A U Thor <author@example.com> +Signed-off-by: A U Thor <author@example.com> +EOF + +test_expect_success 'basic atom: head contents:trailers' ' + echo "Some contents" > two && + git add two && + git commit -F - <<-EOF && + trailers: this commit message has trailers + + Some message contents + + $(cat trailers) + EOF + git for-each-ref --format="%(contents:trailers)" refs/heads/master >actual && + sanitize_pgp <actual >actual.clean && + # git for-each-ref ends with a blank line + cat >expect <<-EOF && + $(cat trailers) + + EOF + test_cmp expect actual.clean +' + test_done -- 2.11.0.rc2.152.g4d04e67 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] add format specifiers to display trailers 2016-11-18 23:08 [PATCH 0/2] add format specifiers to display trailers Jacob Keller 2016-11-18 23:08 ` [PATCH 1/2] pretty: add %bT format for displaying trailers of a commit message Jacob Keller 2016-11-18 23:08 ` [PATCH 2/2] ref-filter: add support to display trailers as part of contents Jacob Keller @ 2016-11-18 23:38 ` Junio C Hamano 2016-11-18 23:42 ` Jacob Keller 2 siblings, 1 reply; 9+ messages in thread From: Junio C Hamano @ 2016-11-18 23:38 UTC (permalink / raw) To: Jacob Keller, Jonathan Tan; +Cc: git, Jacob Keller Jacob Keller <jacob.e.keller@intel.com> writes: > Git interpret-trailers will not recognize this as a trailer block > because it doesn't have any standard git tags within it. Would it be ok > to augment the trailer interpretation to say that if we have over 75% > trailers in the block that we accept it even if it doesn't have any real > recognized tags? I thought the documented way to do this is to configure one of your custom trailer as such. Jonathan? > pretty: add %bT format for displaying trailers of a commit message Are %(...) taken already? In longer term, it would be nice if we can unify the --pretty formats and for-each-ref formats, so it is probably better if we avoid adding any new short ones to the former. We have %s and %b so that we can reconstruct the whole thing by using both. It is unclear how %bT fits in this picture. I wonder if we also need another placeholder that expands to the body of the message without the trailer---otherwise the whole set would become incoherent, no? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] add format specifiers to display trailers 2016-11-18 23:38 ` [PATCH 0/2] add format specifiers to display trailers Junio C Hamano @ 2016-11-18 23:42 ` Jacob Keller 2016-11-21 17:23 ` Junio C Hamano 2016-11-21 20:47 ` [PATCH] doc: mention user-configured trailers Jonathan Tan 0 siblings, 2 replies; 9+ messages in thread From: Jacob Keller @ 2016-11-18 23:42 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jacob Keller, Jonathan Tan, Git mailing list On Fri, Nov 18, 2016 at 3:38 PM, Junio C Hamano <gitster@pobox.com> wrote: > Jacob Keller <jacob.e.keller@intel.com> writes: > >> Git interpret-trailers will not recognize this as a trailer block >> because it doesn't have any standard git tags within it. Would it be ok >> to augment the trailer interpretation to say that if we have over 75% >> trailers in the block that we accept it even if it doesn't have any real >> recognized tags? > > I thought the documented way to do this is to configure one of your > custom trailer as such. Jonathan? > That would be fine then, if that works. >> pretty: add %bT format for displaying trailers of a commit message > > Are %(...) taken already? In longer term, it would be nice if we > can unify the --pretty formats and for-each-ref formats, so it is > probably better if we avoid adding any new short ones to the former. > Oh, I hadn't considered adding a longer one. I'll rework this to use longer ones. > We have %s and %b so that we can reconstruct the whole thing by > using both. It is unclear how %bT fits in this picture. I wonder > if we also need another placeholder that expands to the body of the > message without the trailer---otherwise the whole set would become > incoherent, no? > I'm not entirely sure what to do here. I just wanted a way to easily format "just the trailers" of a message. We could add something that formats just the non-trailers, that's not too difficult. Not really sure what I'd call it though. Thanks, Jake ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] add format specifiers to display trailers 2016-11-18 23:42 ` Jacob Keller @ 2016-11-21 17:23 ` Junio C Hamano 2016-11-29 18:43 ` Keller, Jacob E 2016-11-21 20:47 ` [PATCH] doc: mention user-configured trailers Jonathan Tan 1 sibling, 1 reply; 9+ messages in thread From: Junio C Hamano @ 2016-11-21 17:23 UTC (permalink / raw) To: Jacob Keller; +Cc: Jacob Keller, Jonathan Tan, Git mailing list Jacob Keller <jacob.keller@gmail.com> writes: >> We have %s and %b so that we can reconstruct the whole thing by >> using both. It is unclear how %bT fits in this picture. I wonder >> if we also need another placeholder that expands to the body of the >> message without the trailer---otherwise the whole set would become >> incoherent, no? > > I'm not entirely sure what to do here. I just wanted a way to easily > format "just the trailers" of a message. We could add something that > formats just the non-trailers, that's not too difficult. Not really > sure what I'd call it though. I was wondering if %(log:<name of a part>) was a better way to go. %(log:title) and %(log:body) would be equivalents of traditional %s and %b, and %(log:body) in turn would be a shorter way to write %(log:description)%+(log:trailer), i.e. show the message body, and if there is a trailer block, add it after adding a blank line. Or something like that? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] add format specifiers to display trailers 2016-11-21 17:23 ` Junio C Hamano @ 2016-11-29 18:43 ` Keller, Jacob E 0 siblings, 0 replies; 9+ messages in thread From: Keller, Jacob E @ 2016-11-29 18:43 UTC (permalink / raw) To: gitster@pobox.com, jacob.keller@gmail.com Cc: git@vger.kernel.org, jonathantanmy@google.com On Mon, 2016-11-21 at 09:23 -0800, Junio C Hamano wrote: > Jacob Keller <jacob.keller@gmail.com> writes: > > > > We have %s and %b so that we can reconstruct the whole thing by > > > using both. It is unclear how %bT fits in this picture. I > > > wonder > > > if we also need another placeholder that expands to the body of > > > the > > > message without the trailer---otherwise the whole set would > > > become > > > incoherent, no? > > > > I'm not entirely sure what to do here. I just wanted a way to > > easily > > format "just the trailers" of a message. We could add something > > that > > formats just the non-trailers, that's not too difficult. Not really > > sure what I'd call it though. > > I was wondering if %(log:<name of a part>) was a better way to go. > > %(log:title) and %(log:body) would be equivalents of traditional %s > and %b, and %(log:body) in turn would be a shorter way to write > %(log:description)%+(log:trailer), i.e. show the message body, and > if there is a trailer block, add it after adding a blank line. > > Or something like that? That would work for me. Thanks, Jake ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] doc: mention user-configured trailers 2016-11-18 23:42 ` Jacob Keller 2016-11-21 17:23 ` Junio C Hamano @ 2016-11-21 20:47 ` Jonathan Tan 2016-11-21 21:22 ` Junio C Hamano 1 sibling, 1 reply; 9+ messages in thread From: Jonathan Tan @ 2016-11-21 20:47 UTC (permalink / raw) To: git; +Cc: Jonathan Tan, gitster, jacob.e.keller In commit 1462450 ("trailer: allow non-trailers in trailer block", 2016-10-21), functionality was added (and tested [1]) to allow non-trailer lines in trailer blocks, as long as those blocks contain at least one Git-generated or user-configured trailer, and consists of at least 25% trailers. The documentation was updated to mention this new functionality, but did not mention "user-configured trailer". Further update the documentation to also mention "user-configured trailer". [1] "with non-trailer lines mixed with a configured trailer" in t/t7513-interpret-trailers.sh Signed-off-by: Jonathan Tan <jonathantanmy@google.com> --- Yes, mentioning a trailer in a Git config will cause interpret-trailers to treat it similarly to a Git-generated trailer (in that its presence causes a block partially consisting of trailers to be considered a trailer block). See the commit message above for a test case that verifies that. I took a look at the documentation, and it wasn't completely documented, so here is a patch to correct that. Documentation/git-interpret-trailers.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt index e99bda6..09074c7 100644 --- a/Documentation/git-interpret-trailers.txt +++ b/Documentation/git-interpret-trailers.txt @@ -49,7 +49,8 @@ will be added before the new trailer. Existing trailers are extracted from the input message by looking for a group of one or more lines that (i) are all trailers, or (ii) contains at -least one Git-generated trailer and consists of at least 25% trailers. +least one Git-generated or user-configured trailer and consists of at +least 25% trailers. The group must be preceded by one or more empty (or whitespace-only) lines. The group must either be at the end of the message or be the last non-whitespace lines before a line that starts with '---'. Such three -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] doc: mention user-configured trailers 2016-11-21 20:47 ` [PATCH] doc: mention user-configured trailers Jonathan Tan @ 2016-11-21 21:22 ` Junio C Hamano 0 siblings, 0 replies; 9+ messages in thread From: Junio C Hamano @ 2016-11-21 21:22 UTC (permalink / raw) To: Jonathan Tan; +Cc: git, jacob.e.keller Jonathan Tan <jonathantanmy@google.com> writes: > In commit 1462450 ("trailer: allow non-trailers in trailer block", > 2016-10-21), functionality was added (and tested [1]) to allow > non-trailer lines in trailer blocks, as long as those blocks contain at > least one Git-generated or user-configured trailer, and consists of at > least 25% trailers. The documentation was updated to mention this new > functionality, but did not mention "user-configured trailer". > > Further update the documentation to also mention "user-configured > trailer". > > [1] "with non-trailer lines mixed with a configured trailer" in > t/t7513-interpret-trailers.sh > > Signed-off-by: Jonathan Tan <jonathantanmy@google.com> > --- > > Yes, mentioning a trailer in a Git config will cause interpret-trailers > to treat it similarly to a Git-generated trailer (in that its presence > causes a block partially consisting of trailers to be considered a > trailer block). See the commit message above for a test case that > verifies that. > > I took a look at the documentation, and it wasn't completely documented, > so here is a patch to correct that. Looks sensible. Thanks. > Documentation/git-interpret-trailers.txt | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt > index e99bda6..09074c7 100644 > --- a/Documentation/git-interpret-trailers.txt > +++ b/Documentation/git-interpret-trailers.txt > @@ -49,7 +49,8 @@ will be added before the new trailer. > > Existing trailers are extracted from the input message by looking for > a group of one or more lines that (i) are all trailers, or (ii) contains at > -least one Git-generated trailer and consists of at least 25% trailers. > +least one Git-generated or user-configured trailer and consists of at > +least 25% trailers. > The group must be preceded by one or more empty (or whitespace-only) lines. > The group must either be at the end of the message or be the last > non-whitespace lines before a line that starts with '---'. Such three ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-11-29 18:43 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-11-18 23:08 [PATCH 0/2] add format specifiers to display trailers Jacob Keller 2016-11-18 23:08 ` [PATCH 1/2] pretty: add %bT format for displaying trailers of a commit message Jacob Keller 2016-11-18 23:08 ` [PATCH 2/2] ref-filter: add support to display trailers as part of contents Jacob Keller 2016-11-18 23:38 ` [PATCH 0/2] add format specifiers to display trailers Junio C Hamano 2016-11-18 23:42 ` Jacob Keller 2016-11-21 17:23 ` Junio C Hamano 2016-11-29 18:43 ` Keller, Jacob E 2016-11-21 20:47 ` [PATCH] doc: mention user-configured trailers Jonathan Tan 2016-11-21 21:22 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).