# [PATCH] log: add %S option (like --source) to log --format 8 messages from 2018-12-17 to 2018-12-19. Participants: Issac Trotts, Junio C Hamano, Jeff King. Thread: https://gitlist.dev/t/50049 ## Issac Trotts, 2018-12-17 06:25 Subject: [PATCH] log: add %S option (like --source) to log --format Message-ID: URL: https://gitlist.dev/e/CANdyxMwxPqTMfLsoK-2JT3Wf3hXZnQNCPRS04aSHzsMbYJZo-Q%40mail.gmail.com ``` Make it possible to write for example git log --format="%H,%S" where the %S at the end is a new placeholder that prints out the ref (tag/branch) for each commit. Using %d might seem like an alternative but it only shows the ref for the last commit in the branch. This change is based on a question from Stack Overflow: https://stackoverflow.com/questions/12712775/git-get-source-information-in-format --- Documentation/pretty-formats.txt | 2 ++ builtin/log.c | 2 +- log-tree.c | 1 + pretty.c | 10 +++++++ t/t4205-log-pretty-formats.sh | 50 ++++++++++++++++++++++++++++++++ 5 files changed, 64 insertions(+), 1 deletion(-) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index 417b638cd..acfe7e0a4 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -104,6 +104,8 @@ The placeholders are: - '%H': commit hash - '%h': abbreviated commit hash +- '%S': ref name given on the command line by which the commit was reached + (like `git log --source`) - '%T': tree hash - '%t': abbreviated tree hash - '%P': parent hashes diff --git a/builtin/log.c b/builtin/log.c index e8e51068b..a314ea2b6 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -203,7 +203,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, rev->diffopt.filter || rev->diffopt.flags.follow_renames) rev->always_show_header = 0; - if (source) { + if (source || (rev->pretty_given && rev->commit_format == CMIT_FMT_USERFORMAT)) { init_revision_sources(&revision_sources); rev->sources = &revision_sources; } diff --git a/log-tree.c b/log-tree.c index 10680c139..3cb14256e 100644 --- a/log-tree.c +++ b/log-tree.c @@ -700,6 +700,7 @@ void show_log(struct rev_info *opt) ctx.color = opt->diffopt.use_color; ctx.expand_tabs_in_log = opt->expand_tabs_in_log; ctx.output_encoding = get_log_output_encoding(); + ctx.rev = opt; if (opt->from_ident.mail_begin && opt->from_ident.name_begin) ctx.from_ident = &opt->from_ident; if (opt->graph) diff --git a/pretty.c b/pretty.c index b83a3ecd2..b1774acad 100644 --- a/pretty.c +++ b/pretty.c @@ -1084,6 +1084,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ struct commit_list *p; const char *arg; int ch; + char **slot; /* these are independent of the commit */ switch (placeholder[0]) { @@ -1149,6 +1150,15 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ parse_object(the_repository, &commit->object.oid); switch (placeholder[0]) { + case 'S': /* tag/branch like --source */ + slot = revision_sources_at(c->pretty_ctx->rev->sources, commit); + if (slot && *slot) { + strbuf_addstr(sb, *slot); + return 1; + } else { + die(_("failed to get info for %%S")); + return 0; + } case 'H': /* commit hash */ strbuf_addstr(sb, diff_get_color(c->auto_color, DIFF_COMMIT)); strbuf_addstr(sb, oid_to_hex(&commit->object.oid)); diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index 978a8a66f..7df8c3d4e 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -621,4 +621,54 @@ test_expect_success 'trailer parsing not fooled by --- line' ' test_cmp expect actual ' +test_expect_success 'set up %S tests' ' + git checkout --orphan source-a && + test_commit one && + test_commit two && + git checkout -b source-b HEAD^ && + test_commit three +' + +test_expect_success 'log --format=%S paints branch names' ' + cat >expect <<-\EOF && + source-b + source-a + source-b + EOF + git log --format=%S source-a source-b >actual && + test_cmp expect actual +' + +test_expect_success 'log --format=%S paints tag names' ' + git tag -m tagged source-tag && + cat >expect <<-\EOF && + source-tag + source-a + source-tag + EOF + git log --format=%S source-tag source-a >actual && + test_cmp expect actual +' + +test_expect_success 'log --format=%S paints symmetric ranges' ' + cat >expect <<-\EOF && + source-b + source-a + EOF + git log --format=%S source-a...source-b >actual && + test_cmp expect actual +' + +test_expect_success '%S in git log --format works with other placeholders (part 1)' ' + git log --format="source-b %h" source-b >expect && + git log --format="%S %h" source-b >actual && + test_cmp expect actual +' + +test_expect_success '%S in git log --format works with other placeholders (part 2)' ' + git log --format="%h source-b" source-b >expect && + git log --format="%h %S" source-b >actual && + test_cmp expect actual +' + test_done -- 2.19.1 ``` ## Junio C Hamano, 2018-12-17 09:31 Subject: Re: [PATCH] log: add %S option (like --source) to log --format Message-ID: URL: https://gitlist.dev/e/xmqqk1k8bitm.fsf%40gitster-ct.c.googlers.com In-Reply-To: ``` Issac Trotts writes: > Make it possible to write for example > > git log --format="%H,%S" > > where the %S at the end is a new placeholder that prints out the ref > (tag/branch) for each commit. > > Using %d might seem like an alternative but it only shows the ref for > the last commit in the branch. Have your sign-off here (see Documentation/SubmittingPatches), and then have a line that only has three-dashes on it, and then any other additional info like this. > > This change is based on a question from Stack Overflow: > https://stackoverflow.com/questions/12712775/git-get-source-information-in-format > --- > Documentation/pretty-formats.txt | 2 ++ > builtin/log.c | 2 +- > log-tree.c | 1 + > pretty.c | 10 +++++++ > t/t4205-log-pretty-formats.sh | 50 ++++++++++++++++++++++++++++++++ > 5 files changed, 64 insertions(+), 1 deletion(-) > > diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt > index 417b638cd..acfe7e0a4 100644 > --- a/Documentation/pretty-formats.txt > +++ b/Documentation/pretty-formats.txt > @@ -104,6 +104,8 @@ The placeholders are: > > - '%H': commit hash > - '%h': abbreviated commit hash > +- '%S': ref name given on the command line by which the commit was reached > + (like `git log --source`) This looks entirely out of place, among the description of the basic basic data like the object name of the commit itself, its tree, etc. Describe this immediately after %d and %D are explained, perhaps. > diff --git a/builtin/log.c b/builtin/log.c > index e8e51068b..a314ea2b6 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -203,7 +203,7 @@ static void cmd_log_init_finish(int argc, const > char **argv, const char *prefix, > rev->diffopt.filter || rev->diffopt.flags.follow_renames) > rev->always_show_header = 0; > > - if (source) { > + if (source || (rev->pretty_given && rev->commit_format == > CMIT_FMT_USERFORMAT)) { Broken line (you let your MUA line-wrap and corrupt your patch???) > init_revision_sources(&revision_sources); > rev->sources = &revision_sources; > } This means anybody who asks for say --format='%aN %s' pays the price of keeping track of each commit's source, which is unreasonable. Perhaps mimick the way how presence of %N is detected in userformat_want_item() so that we do not pay the price for init_display_notes() when a format that does not care about %N is given? > @@ -1149,6 +1150,15 @@ static size_t format_commit_one(struct strbuf > *sb, /* in UTF-8 */ > parse_object(the_repository, &commit->object.oid); > > switch (placeholder[0]) { > + case 'S': /* tag/branch like --source */ > + slot = revision_sources_at(c->pretty_ctx->rev->sources, commit); > + if (slot && *slot) { > + strbuf_addstr(sb, *slot); > + return 1; > + } else { > + die(_("failed to get info for %%S")); > + return 0; > + } Have this next to case arms that deal with 'd' and 'D". > case 'H': /* commit hash */ > strbuf_addstr(sb, diff_get_color(c->auto_color, DIFF_COMMIT)); > strbuf_addstr(sb, oid_to_hex(&commit->object.oid)); This is a tangent, but I think this existing case arm should move to the previous block before parsing the commit, as %H only needs the object name of the commit itself (reword the comment before that switch to read from "independent of the commit" to "computable without parsing the commit").. ``` ## Jeff King, 2018-12-17 15:59 Subject: Re: [PATCH] log: add %S option (like --source) to log --format Message-ID: <20181217155915.GA914@sigill.intra.peff.net> URL: https://gitlist.dev/e/20181217155915.GA914%40sigill.intra.peff.net In-Reply-To: ``` On Sun, Dec 16, 2018 at 10:25:14PM -0800, Issac Trotts wrote: > Make it possible to write for example > > git log --format="%H,%S" > > where the %S at the end is a new placeholder that prints out the ref > (tag/branch) for each commit. Seems like a reasonable thing to want. One curious thing about "--source" is that it requires cooperation from the actual traversal. So if you did: git rev-list | git diff-tree --format="%H %S" we don't have the %S information in the latter command. I think that's probably acceptable as long as it does something sane when we don't have that information (e.g., replace it with an empty string). It might also be worth calling out in the documentation. -Peff ``` ## Issac Trotts, 2018-12-18 03:35 Subject: Re: [PATCH] log: add %S option (like --source) to log --format Message-ID: URL: https://gitlist.dev/e/CANdyxMxCe9xjkRYGyovRo4UU5A5PGRwpFbE4ej74hXJUF3_9zQ%40mail.gmail.com In-Reply-To: ``` Hi Junio, thanks for your feedback. I'll submit a new patch as soon as I've addressed all the feedback. On Mon, Dec 17, 2018 at 1:31 AM Junio C Hamano wrote: > > Issac Trotts writes: > > > Make it possible to write for example > > > > git log --format="%H,%S" > > > > where the %S at the end is a new placeholder that prints out the ref > > (tag/branch) for each commit. > > > > Using %d might seem like an alternative but it only shows the ref for > > the last commit in the branch. > > Have your sign-off here (see Documentation/SubmittingPatches), Done. > and then have a line that only has three-dashes on it, > > and then any other additional info like this. Done. > > This change is based on a question from Stack Overflow: > > https://stackoverflow.com/questions/12712775/git-get-source-information-in-format > > --- > > > > > > Documentation/pretty-formats.txt | 2 ++ > > builtin/log.c | 2 +- > > log-tree.c | 1 + > > pretty.c | 10 +++++++ > > t/t4205-log-pretty-formats.sh | 50 ++++++++++++++++++++++++++++++++ > > 5 files changed, 64 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt > > index 417b638cd..acfe7e0a4 100644 > > --- a/Documentation/pretty-formats.txt > > +++ b/Documentation/pretty-formats.txt > > @@ -104,6 +104,8 @@ The placeholders are: > > > > - '%H': commit hash > > - '%h': abbreviated commit hash > > +- '%S': ref name given on the command line by which the commit was reached > > + (like `git log --source`) > > This looks entirely out of place, among the description of the basic > basic data like the object name of the commit itself, its tree, etc. > > Describe this immediately after %d and %D are explained, perhaps. Done. > > > diff --git a/builtin/log.c b/builtin/log.c > > index e8e51068b..a314ea2b6 100644 > > --- a/builtin/log.c > > +++ b/builtin/log.c > > @@ -203,7 +203,7 @@ static void cmd_log_init_finish(int argc, const > > char **argv, const char *prefix, > > rev->diffopt.filter || rev->diffopt.flags.follow_renames) > > rev->always_show_header = 0; > > > > - if (source) { > > + if (source || (rev->pretty_given && rev->commit_format == > > CMIT_FMT_USERFORMAT)) { > > Broken line (you let your MUA line-wrap and corrupt your patch???) I see. I need to use `git send-email`. > > > init_revision_sources(&revision_sources); > > rev->sources = &revision_sources; > > } > > This means anybody who asks for say --format='%aN %s' pays the price > of keeping track of each commit's source, which is unreasonable. > > Perhaps mimick the way how presence of %N is detected in > userformat_want_item() so that we do not pay the price for > init_display_notes() when a format that does not care about %N is > given? Done. > > > @@ -1149,6 +1150,15 @@ static size_t format_commit_one(struct strbuf > > *sb, /* in UTF-8 */ > > parse_object(the_repository, &commit->object.oid); > > > > switch (placeholder[0]) { > > + case 'S': /* tag/branch like --source */ > > + slot = revision_sources_at(c->pretty_ctx->rev->sources, commit); > > + if (slot && *slot) { > > + strbuf_addstr(sb, *slot); > > + return 1; > > + } else { > > + die(_("failed to get info for %%S")); > > + return 0; > > + } > > Have this next to case arms that deal with 'd' and 'D". Done. > > > case 'H': /* commit hash */ > > strbuf_addstr(sb, diff_get_color(c->auto_color, DIFF_COMMIT)); > > strbuf_addstr(sb, oid_to_hex(&commit->object.oid)); > > This is a tangent, but I think this existing case arm should move to > the previous block before parsing the commit, as %H only needs the > object name of the commit itself (reword the comment before that > switch to read from "independent of the commit" to "computable > without parsing the commit").. I hope it's all right if I keep my patch focused on just one thing. ``` ## Issac Trotts, 2018-12-18 17:14 Subject: Re: [PATCH] log: add %S option (like --source) to log --format Message-ID: URL: https://gitlist.dev/e/CANdyxMzsf00k44TVkw%2B9uL4E3G_0rjYNgiMFK2o7MX83fCMPMQ%40mail.gmail.com In-Reply-To: <20181217155915.GA914@sigill.intra.peff.net> ``` Hi Peff, thanks for the feedback. I tried a variant of the command you showed and it yielded a seg fault: ˋˋˋ [ issactrotts ~/git ] ./git diff-tree -s --pretty=tformat:'%S %H %s' HEAD Segmentation fault: 11 ˋˋˋ I'll see if I can track it down this evening. Issac On Mon, Dec 17, 2018 at 7:59 AM Jeff King wrote: > > On Sun, Dec 16, 2018 at 10:25:14PM -0800, Issac Trotts wrote: > > > Make it possible to write for example > > > > git log --format="%H,%S" > > > > where the %S at the end is a new placeholder that prints out the ref > > (tag/branch) for each commit. > > Seems like a reasonable thing to want. > > One curious thing about "--source" is that it requires cooperation from > the actual traversal. So if you did: > > git rev-list | git diff-tree --format="%H %S" > > we don't have the %S information in the latter command. I think that's > probably acceptable as long as it does something sane when we don't have > that information (e.g., replace it with an empty string). It might also > be worth calling out in the documentation. > > -Peff ``` ## Issac Trotts, 2018-12-19 03:47 Subject: Re: [PATCH] log: add %S option (like --source) to log --format Message-ID: URL: https://gitlist.dev/e/CANdyxMzOpRRu%2BB0cq5cbrmYsy47cod8N4mWrGtfOgRRgKVh%2BxA%40mail.gmail.com In-Reply-To: ``` On Tue, Dec 18, 2018 at 9:14 AM Issac Trotts wrote: > > Hi Peff, thanks for the feedback. I tried a variant of the command you > showed and it yielded a seg fault: > ˋˋˋ > [ issactrotts ~/git ] ./git diff-tree -s --pretty=tformat:'%S %H %s' HEAD > Segmentation fault: 11 > ˋˋˋ > I'll see if I can track it down this evening. Okay, found it. My solution is to make %S not do anything for commands other than `log` since `log` is the only one that has --source defined as far as I can tell. I'm waiting for Travis to run and will post an updated patch if everything looks good there. > Issac > > On Mon, Dec 17, 2018 at 7:59 AM Jeff King wrote: > > > > On Sun, Dec 16, 2018 at 10:25:14PM -0800, Issac Trotts wrote: > > > > > Make it possible to write for example > > > > > > git log --format="%H,%S" > > > > > > where the %S at the end is a new placeholder that prints out the ref > > > (tag/branch) for each commit. > > > > Seems like a reasonable thing to want. > > > > One curious thing about "--source" is that it requires cooperation from > > the actual traversal. So if you did: > > > > git rev-list | git diff-tree --format="%H %S" > > > > we don't have the %S information in the latter command. I think that's > > probably acceptable as long as it does something sane when we don't have > > that information (e.g., replace it with an empty string). It might also > > be worth calling out in the documentation. > > > > -Peff ``` ## Issac Trotts, 2018-12-19 08:07 Subject: Re: [PATCH] log: add %S option (like --source) to log --format Message-ID: URL: https://gitlist.dev/e/CANdyxMzYMc2DGC8JUE7N6Ry7iOLH09SwR5OFQ64c1vtbO388FA%40mail.gmail.com In-Reply-To: ``` Travis showed one failure. I'm not sure if it's a flake. https://travis-ci.org/ijt/git/builds/469843833. Rerunning it. On Tue, Dec 18, 2018 at 7:47 PM Issac Trotts wrote: > > On Tue, Dec 18, 2018 at 9:14 AM Issac Trotts wrote: > > > > Hi Peff, thanks for the feedback. I tried a variant of the command you > > showed and it yielded a seg fault: > > ˋˋˋ > > [ issactrotts ~/git ] ./git diff-tree -s --pretty=tformat:'%S %H %s' HEAD > > Segmentation fault: 11 > > ˋˋˋ > > I'll see if I can track it down this evening. > > Okay, found it. My solution is to make %S not do anything for commands > other than `log` since `log` is the only one that has --source defined > as far as I can tell. > > I'm waiting for Travis to run and will post an updated patch if > everything looks good there. > > > Issac > > > > On Mon, Dec 17, 2018 at 7:59 AM Jeff King wrote: > > > > > > On Sun, Dec 16, 2018 at 10:25:14PM -0800, Issac Trotts wrote: > > > > > > > Make it possible to write for example > > > > > > > > git log --format="%H,%S" > > > > > > > > where the %S at the end is a new placeholder that prints out the ref > > > > (tag/branch) for each commit. > > > > > > Seems like a reasonable thing to want. > > > > > > One curious thing about "--source" is that it requires cooperation from > > > the actual traversal. So if you did: > > > > > > git rev-list | git diff-tree --format="%H %S" > > > > > > we don't have the %S information in the latter command. I think that's > > > probably acceptable as long as it does something sane when we don't have > > > that information (e.g., replace it with an empty string). It might also > > > be worth calling out in the documentation. > > > > > > -Peff ``` ## Issac Trotts, 2018-12-19 17:09 Subject: Re: [PATCH] log: add %S option (like --source) to log --format Message-ID: URL: https://gitlist.dev/e/CANdyxMzLRur8Y66-fOhusW%2BrW%3DgYaM5pnotRrEwb-iy7Sa29ew%40mail.gmail.com In-Reply-To: ``` Looks like that was a flake. Everything passes now. https://travis-ci.org/ijt/git/builds/469843833 On Wed, Dec 19, 2018 at 12:07 AM Issac Trotts wrote: > > Travis showed one failure. I'm not sure if it's a flake. > https://travis-ci.org/ijt/git/builds/469843833. Rerunning it. > > On Tue, Dec 18, 2018 at 7:47 PM Issac Trotts wrote: > > > > On Tue, Dec 18, 2018 at 9:14 AM Issac Trotts wrote: > > > > > > Hi Peff, thanks for the feedback. I tried a variant of the command you > > > showed and it yielded a seg fault: > > > ˋˋˋ > > > [ issactrotts ~/git ] ./git diff-tree -s --pretty=tformat:'%S %H %s' HEAD > > > Segmentation fault: 11 > > > ˋˋˋ > > > I'll see if I can track it down this evening. > > > > Okay, found it. My solution is to make %S not do anything for commands > > other than `log` since `log` is the only one that has --source defined > > as far as I can tell. > > > > I'm waiting for Travis to run and will post an updated patch if > > everything looks good there. > > > > > Issac > > > > > > On Mon, Dec 17, 2018 at 7:59 AM Jeff King wrote: > > > > > > > > On Sun, Dec 16, 2018 at 10:25:14PM -0800, Issac Trotts wrote: > > > > > > > > > Make it possible to write for example > > > > > > > > > > git log --format="%H,%S" > > > > > > > > > > where the %S at the end is a new placeholder that prints out the ref > > > > > (tag/branch) for each commit. > > > > > > > > Seems like a reasonable thing to want. > > > > > > > > One curious thing about "--source" is that it requires cooperation from > > > > the actual traversal. So if you did: > > > > > > > > git rev-list | git diff-tree --format="%H %S" > > > > > > > > we don't have the %S information in the latter command. I think that's > > > > probably acceptable as long as it does something sane when we don't have > > > > that information (e.g., replace it with an empty string). It might also > > > > be worth calling out in the documentation. > > > > > > > > -Peff ```