{"thread":{"id":"50065","subject":"[PATCH] log: add %S option (like --source) to log --format","startedAt":"2018-12-19T08:33:28Z","lastAt":"2018-12-31T04:35:17Z","messageCount":7,"participants":["issac.trotts@gmail.com","Issac Trotts","Jeff King","Derrick Stolee","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"365567","messageId":"20181219083305.2500-1-issac.trotts@gmail.com","threadId":"50065","inReplyTo":null,"subject":"[PATCH] log: add %S option (like --source) to log --format","fromName":"","fromEmail":"issac.trotts@gmail.com","sentAt":"2018-12-19T08:33:05Z","receivedAt":"2018-12-19T08:33:28Z","isPatch":true,"sender":{"key":"issac.trotts@gmail.com","avatar":"https://gravatar.com/avatar/44dd86aa2ea47c79d9b2a0a99b90e0ff18601855d2f05c6b84d6c432c5906721?d=mp&s=160"},"body":"From: Issac Trotts <issac.trotts@gmail.com>\n\nMake it possible to write for example\n\n        git log --format=\"%H,%S\"\n\nwhere the %S at the end is a new placeholder that prints out the ref\n(tag/branch) for each commit.\n\nUsing %d might seem like an alternative but it only shows the ref for the last\ncommit in the branch.\n\nSigned-off-by: Issac Trotts <issactrotts@google.com>\n\n---\n\nThis change is based on a question from Stack Overflow:\nhttps://stackoverflow.com/questions/12712775/git-get-source-information-in-format\n---\n Documentation/pretty-formats.txt |  2 ++\n builtin/log.c                    |  2 +-\n log-tree.c                       |  1 +\n pretty.c                         | 15 ++++++++++\n pretty.h                         |  1 +\n t/t4205-log-pretty-formats.sh    | 50 ++++++++++++++++++++++++++++++++\n 6 files changed, 70 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex 417b638cd..de6953108 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -134,6 +134,8 @@ The placeholders are:\n - '%cI': committer date, strict ISO 8601 format\n - '%d': ref names, like the --decorate option of linkgit:git-log[1]\n - '%D': ref names without the \" (\", \")\" wrapping.\n+- '%S': ref name given on the command line by which the commit was reached\n+  (like `git log --source`), only works with `git log`\n - '%e': encoding\n - '%s': subject\n - '%f': sanitized subject line, suitable for a filename\ndiff --git a/builtin/log.c b/builtin/log.c\nindex e8e51068b..be3025657 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -203,7 +203,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t    rev->diffopt.filter || rev->diffopt.flags.follow_renames)\n \t\trev->always_show_header = 0;\n \n-\tif (source) {\n+\tif (source || (rev->pretty_given && (rev->commit_format == CMIT_FMT_USERFORMAT) && w.source)) {\n \t\tinit_revision_sources(&revision_sources);\n \t\trev->sources = &revision_sources;\n \t}\ndiff --git a/log-tree.c b/log-tree.c\nindex 10680c139..3cb14256e 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -700,6 +700,7 @@ void show_log(struct rev_info *opt)\n \tctx.color = opt->diffopt.use_color;\n \tctx.expand_tabs_in_log = opt->expand_tabs_in_log;\n \tctx.output_encoding = get_log_output_encoding();\n+\tctx.rev = opt;\n \tif (opt->from_ident.mail_begin && opt->from_ident.name_begin)\n \t\tctx.from_ident = &opt->from_ident;\n \tif (opt->graph)\ndiff --git a/pretty.c b/pretty.c\nindex b83a3ecd2..258e86e9c 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1084,6 +1084,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \tstruct commit_list *p;\n \tconst char *arg;\n \tint ch;\n+\tchar **slot;\n \n \t/* these are independent of the commit */\n \tswitch (placeholder[0]) {\n@@ -1194,6 +1195,17 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\tload_ref_decorations(NULL, DECORATE_SHORT_REFS);\n \t\tformat_decorations_extended(sb, commit, c->auto_color, \"\", \", \", \"\");\n \t\treturn 1;\n+\tcase 'S':\t\t/* tag/branch like --source */\n+\t\tif (c->pretty_ctx->rev->sources == NULL) {\n+\t\t\treturn 0;\n+\t\t}\n+\t\tslot = revision_sources_at(c->pretty_ctx->rev->sources, commit);\n+\t\tif (slot && *slot) {\n+\t\t\tstrbuf_addstr(sb, *slot);\n+\t\t\treturn 1;\n+\t\t} else {\n+\t\t\tdie(_(\"failed to get info for %%S\"));\n+\t\t}\n \tcase 'g':\t\t/* reflog info */\n \t\tswitch(placeholder[1]) {\n \t\tcase 'd':\t/* reflog selector */\n@@ -1498,6 +1510,9 @@ static size_t userformat_want_item(struct strbuf *sb, const char *placeholder,\n \tcase 'N':\n \t\tw->notes = 1;\n \t\tbreak;\n+\tcase 'S':\n+\t\tw->source = 1;\n+\t\tbreak;\n \t}\n \treturn 0;\n }\ndiff --git a/pretty.h b/pretty.h\nindex 7359d318a..87ca5dfcb 100644\n--- a/pretty.h\n+++ b/pretty.h\n@@ -60,6 +60,7 @@ static inline int cmit_fmt_is_mail(enum cmit_fmt fmt)\n \n struct userformat_want {\n \tunsigned notes:1;\n+\tunsigned source:1;\n };\n \n /* Set the flag \"w->notes\" if there is placeholder %N in \"fmt\". */\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 978a8a66f..7df8c3d4e 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -621,4 +621,54 @@ test_expect_success 'trailer parsing not fooled by --- line' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'set up %S tests' '\n+\tgit checkout --orphan source-a &&\n+\ttest_commit one &&\n+\ttest_commit two &&\n+\tgit checkout -b source-b HEAD^ &&\n+\ttest_commit three\n+'\n+\n+test_expect_success 'log --format=%S paints branch names' '\n+\tcat >expect <<-\\EOF &&\n+\tsource-b\n+\tsource-a\n+\tsource-b\n+\tEOF\n+\tgit log --format=%S source-a source-b >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --format=%S paints tag names' '\n+\tgit tag -m tagged source-tag &&\n+\tcat >expect <<-\\EOF &&\n+\tsource-tag\n+\tsource-a\n+\tsource-tag\n+\tEOF\n+\tgit log --format=%S source-tag source-a >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --format=%S paints symmetric ranges' '\n+\tcat >expect <<-\\EOF &&\n+\tsource-b\n+\tsource-a\n+\tEOF\n+\tgit log --format=%S source-a...source-b >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%S in git log --format works with other placeholders (part 1)' '\n+\tgit log --format=\"source-b %h\" source-b >expect &&\n+\tgit log --format=\"%S %h\" source-b >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%S in git log --format works with other placeholders (part 2)' '\n+\tgit log --format=\"%h source-b\" source-b >expect &&\n+\tgit log --format=\"%h %S\" source-b >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.19.1\n\n"},{"id":"365718","messageId":"CANdyxMyz3u+ajH0X7BPJBPBT0iepWhunA_VA+HEGFrurYghSWQ@mail.gmail.com","threadId":"50065","inReplyTo":"20181219083305.2500-1-issac.trotts@gmail.com","subject":"Re: [PATCH] log: add %S option (like --source) to log --format","fromName":"Issac Trotts","fromEmail":"issac.trotts@gmail.com","sentAt":"2018-12-21T05:24:10Z","receivedAt":"2018-12-21T05:24:25Z","isPatch":true,"sender":{"key":"issac.trotts@gmail.com","avatar":"https://gravatar.com/avatar/44dd86aa2ea47c79d9b2a0a99b90e0ff18601855d2f05c6b84d6c432c5906721?d=mp&s=160"},"body":"Hi all, friendly ping. Is there still interest in merging this patch?\n\nThanks,\nIssac\n\nOn Wed, Dec 19, 2018 at 12:33 AM <issac.trotts@gmail.com> wrote:\n>\n> From: Issac Trotts <issac.trotts@gmail.com>\n>\n> Make it possible to write for example\n>\n>         git log --format=\"%H,%S\"\n>\n> where the %S at the end is a new placeholder that prints out the ref\n> (tag/branch) for each commit.\n>\n> Using %d might seem like an alternative but it only shows the ref for the last\n> commit in the branch.\n>\n> Signed-off-by: Issac Trotts <issactrotts@google.com>\n>\n> ---\n>\n> This change is based on a question from Stack Overflow:\n> https://stackoverflow.com/questions/12712775/git-get-source-information-in-format\n> ---\n>  Documentation/pretty-formats.txt |  2 ++\n>  builtin/log.c                    |  2 +-\n>  log-tree.c                       |  1 +\n>  pretty.c                         | 15 ++++++++++\n>  pretty.h                         |  1 +\n>  t/t4205-log-pretty-formats.sh    | 50 ++++++++++++++++++++++++++++++++\n>  6 files changed, 70 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\n> index 417b638cd..de6953108 100644\n> --- a/Documentation/pretty-formats.txt\n> +++ b/Documentation/pretty-formats.txt\n> @@ -134,6 +134,8 @@ The placeholders are:\n>  - '%cI': committer date, strict ISO 8601 format\n>  - '%d': ref names, like the --decorate option of linkgit:git-log[1]\n>  - '%D': ref names without the \" (\", \")\" wrapping.\n> +- '%S': ref name given on the command line by which the commit was reached\n> +  (like `git log --source`), only works with `git log`\n>  - '%e': encoding\n>  - '%s': subject\n>  - '%f': sanitized subject line, suitable for a filename\n> diff --git a/builtin/log.c b/builtin/log.c\n> index e8e51068b..be3025657 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -203,7 +203,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n>             rev->diffopt.filter || rev->diffopt.flags.follow_renames)\n>                 rev->always_show_header = 0;\n>\n> -       if (source) {\n> +       if (source || (rev->pretty_given && (rev->commit_format == CMIT_FMT_USERFORMAT) && w.source)) {\n>                 init_revision_sources(&revision_sources);\n>                 rev->sources = &revision_sources;\n>         }\n> diff --git a/log-tree.c b/log-tree.c\n> index 10680c139..3cb14256e 100644\n> --- a/log-tree.c\n> +++ b/log-tree.c\n> @@ -700,6 +700,7 @@ void show_log(struct rev_info *opt)\n>         ctx.color = opt->diffopt.use_color;\n>         ctx.expand_tabs_in_log = opt->expand_tabs_in_log;\n>         ctx.output_encoding = get_log_output_encoding();\n> +       ctx.rev = opt;\n>         if (opt->from_ident.mail_begin && opt->from_ident.name_begin)\n>                 ctx.from_ident = &opt->from_ident;\n>         if (opt->graph)\n> diff --git a/pretty.c b/pretty.c\n> index b83a3ecd2..258e86e9c 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -1084,6 +1084,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n>         struct commit_list *p;\n>         const char *arg;\n>         int ch;\n> +       char **slot;\n>\n>         /* these are independent of the commit */\n>         switch (placeholder[0]) {\n> @@ -1194,6 +1195,17 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n>                 load_ref_decorations(NULL, DECORATE_SHORT_REFS);\n>                 format_decorations_extended(sb, commit, c->auto_color, \"\", \", \", \"\");\n>                 return 1;\n> +       case 'S':               /* tag/branch like --source */\n> +               if (c->pretty_ctx->rev->sources == NULL) {\n> +                       return 0;\n> +               }\n> +               slot = revision_sources_at(c->pretty_ctx->rev->sources, commit);\n> +               if (slot && *slot) {\n> +                       strbuf_addstr(sb, *slot);\n> +                       return 1;\n> +               } else {\n> +                       die(_(\"failed to get info for %%S\"));\n> +               }\n>         case 'g':               /* reflog info */\n>                 switch(placeholder[1]) {\n>                 case 'd':       /* reflog selector */\n> @@ -1498,6 +1510,9 @@ static size_t userformat_want_item(struct strbuf *sb, const char *placeholder,\n>         case 'N':\n>                 w->notes = 1;\n>                 break;\n> +       case 'S':\n> +               w->source = 1;\n> +               break;\n>         }\n>         return 0;\n>  }\n> diff --git a/pretty.h b/pretty.h\n> index 7359d318a..87ca5dfcb 100644\n> --- a/pretty.h\n> +++ b/pretty.h\n> @@ -60,6 +60,7 @@ static inline int cmit_fmt_is_mail(enum cmit_fmt fmt)\n>\n>  struct userformat_want {\n>         unsigned notes:1;\n> +       unsigned source:1;\n>  };\n>\n>  /* Set the flag \"w->notes\" if there is placeholder %N in \"fmt\". */\n> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\n> index 978a8a66f..7df8c3d4e 100755\n> --- a/t/t4205-log-pretty-formats.sh\n> +++ b/t/t4205-log-pretty-formats.sh\n> @@ -621,4 +621,54 @@ test_expect_success 'trailer parsing not fooled by --- line' '\n>         test_cmp expect actual\n>  '\n>\n> +test_expect_success 'set up %S tests' '\n> +       git checkout --orphan source-a &&\n> +       test_commit one &&\n> +       test_commit two &&\n> +       git checkout -b source-b HEAD^ &&\n> +       test_commit three\n> +'\n> +\n> +test_expect_success 'log --format=%S paints branch names' '\n> +       cat >expect <<-\\EOF &&\n> +       source-b\n> +       source-a\n> +       source-b\n> +       EOF\n> +       git log --format=%S source-a source-b >actual &&\n> +       test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --format=%S paints tag names' '\n> +       git tag -m tagged source-tag &&\n> +       cat >expect <<-\\EOF &&\n> +       source-tag\n> +       source-a\n> +       source-tag\n> +       EOF\n> +       git log --format=%S source-tag source-a >actual &&\n> +       test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --format=%S paints symmetric ranges' '\n> +       cat >expect <<-\\EOF &&\n> +       source-b\n> +       source-a\n> +       EOF\n> +       git log --format=%S source-a...source-b >actual &&\n> +       test_cmp expect actual\n> +'\n> +\n> +test_expect_success '%S in git log --format works with other placeholders (part 1)' '\n> +       git log --format=\"source-b %h\" source-b >expect &&\n> +       git log --format=\"%S %h\" source-b >actual &&\n> +       test_cmp expect actual\n> +'\n> +\n> +test_expect_success '%S in git log --format works with other placeholders (part 2)' '\n> +       git log --format=\"%h source-b\" source-b >expect &&\n> +       git log --format=\"%h %S\" source-b >actual &&\n> +       test_cmp expect actual\n> +'\n> +\n>  test_done\n> --\n> 2.19.1\n>\n"},{"id":"365756","messageId":"20181222222224.GA15914@sigill.intra.peff.net","threadId":"50065","inReplyTo":"CANdyxMyz3u+ajH0X7BPJBPBT0iepWhunA_VA+HEGFrurYghSWQ@mail.gmail.com","subject":"Re: [PATCH] log: add %S option (like --source) to log --format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-12-22T22:22:24Z","receivedAt":"2018-12-22T22:22:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 20, 2018 at 09:24:10PM -0800, Issac Trotts wrote:\n\n> Hi all, friendly ping. Is there still interest in merging this patch?\n\nYes, but I think people are largely absent for holidays at this point.\nIf there hasn't been any movement on it, I'll try to review after Jan\n5th (it also wouldn't hurt to re-post around then).\n\n-Peff\n"},{"id":"365796","messageId":"CANdyxMzg-RU82Enb5MpBrWSoteHQvw9SmsNiMbTWqBSfqM+ixQ@mail.gmail.com","threadId":"50065","inReplyTo":"20181222222224.GA15914@sigill.intra.peff.net","subject":"Re: [PATCH] log: add %S option (like --source) to log --format","fromName":"Issac Trotts","fromEmail":"issac.trotts@gmail.com","sentAt":"2018-12-25T02:12:20Z","receivedAt":"2018-12-25T02:12:36Z","isPatch":true,"sender":{"key":"issac.trotts@gmail.com","avatar":"https://gravatar.com/avatar/44dd86aa2ea47c79d9b2a0a99b90e0ff18601855d2f05c6b84d6c432c5906721?d=mp&s=160"},"body":"Got it, sounds good.\n\nOn Sat, Dec 22, 2018 at 2:22 PM Jeff King <peff@peff.net> wrote:\n>\n> On Thu, Dec 20, 2018 at 09:24:10PM -0800, Issac Trotts wrote:\n>\n> > Hi all, friendly ping. Is there still interest in merging this patch?\n>\n> Yes, but I think people are largely absent for holidays at this point.\n> If there hasn't been any movement on it, I'll try to review after Jan\n> 5th (it also wouldn't hurt to re-post around then).\n>\n> -Peff\n"},{"id":"365838","messageId":"5dfd92d1-2e87-3006-1630-a33794b6066b@gmail.com","threadId":"50065","inReplyTo":"20181219083305.2500-1-issac.trotts@gmail.com","subject":"Re: [PATCH] log: add %S option (like --source) to log --format","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-12-27T13:20:32Z","receivedAt":"2018-12-27T13:20:38Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/19/2018 3:33 AM, issac.trotts@gmail.com wrote:\n> From: Issac Trotts <issac.trotts@gmail.com>\n>\n> Make it possible to write for example\n>\n>          git log --format=\"%H,%S\"\n>\n> where the %S at the end is a new placeholder that prints out the ref\n> (tag/branch) for each commit.\n>\n> Using %d might seem like an alternative but it only shows the ref for the last\n> commit in the branch.\n>\n> Signed-off-by: Issac Trotts <issactrotts@google.com>\n>\n> ---\n>\n> This change is based on a question from Stack Overflow:\n> https://stackoverflow.com/questions/12712775/git-get-source-information-in-format\n> ---\n>   Documentation/pretty-formats.txt |  2 ++\n>   builtin/log.c                    |  2 +-\n>   log-tree.c                       |  1 +\n>   pretty.c                         | 15 ++++++++++\n>   pretty.h                         |  1 +\n>   t/t4205-log-pretty-formats.sh    | 50 ++++++++++++++++++++++++++++++++\n>   6 files changed, 70 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\n> index 417b638cd..de6953108 100644\n> --- a/Documentation/pretty-formats.txt\n> +++ b/Documentation/pretty-formats.txt\n> @@ -134,6 +134,8 @@ The placeholders are:\n>   - '%cI': committer date, strict ISO 8601 format\n>   - '%d': ref names, like the --decorate option of linkgit:git-log[1]\n>   - '%D': ref names without the \" (\", \")\" wrapping.\n> +- '%S': ref name given on the command line by which the commit was reached\n> +  (like `git log --source`), only works with `git log`\n\nThis \"only works with `git log`\" made me think about what would happen \nwith `git rev-list --pretty=format:\"%h %S\"` and the answer (on my \nmachine) was a segfault.\n\n>   - '%e': encoding\n>   - '%s': subject\n>   - '%f': sanitized subject line, suitable for a filename\n> diff --git a/builtin/log.c b/builtin/log.c\n> index e8e51068b..be3025657 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -203,7 +203,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n>   \t    rev->diffopt.filter || rev->diffopt.flags.follow_renames)\n>   \t\trev->always_show_header = 0;\n>   \n> -\tif (source) {\n> +\tif (source || (rev->pretty_given && (rev->commit_format == CMIT_FMT_USERFORMAT) && w.source)) {\n>   \t\tinit_revision_sources(&revision_sources);\n>   \t\trev->sources = &revision_sources;\n>   \t}\n\nLikely, you'll want to duplicate this initialization in the revision \nmachinery. Keep this one in builtin/log.c as it was before, but add \nsomething like this initialization to setup_revisions(). Add a test for \nrev-list.\n\n[snip]\n\n> @@ -1194,6 +1195,17 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n>   \t\tload_ref_decorations(NULL, DECORATE_SHORT_REFS);\n>   \t\tformat_decorations_extended(sb, commit, c->auto_color, \"\", \", \", \"\");\n>   \t\treturn 1;\n> +\tcase 'S':\t\t/* tag/branch like --source */\n> +\t\tif (c->pretty_ctx->rev->sources == NULL) {\nUse \"if (!c->pretty_ctx->rev->sources\".\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t\tslot = revision_sources_at(c->pretty_ctx->rev->sources, commit);\n> +\t\tif (slot && *slot) {\nI'm not sure this check for 'slot' being non-null is necessary, as we \nwould already get a failure in the commit-slab code (for \nrevision_sources_at()) if the slab is not initialized.\n> +\t\t\tstrbuf_addstr(sb, *slot);\n> +\t\t\treturn 1;\n> +\t\t} else {\n> +\t\t\tdie(_(\"failed to get info for %%S\"));\n\nHere, you die() when you fail to get a slot but above you return 0 when \nthe sources are not initialized.\n\nI don't see another use of die() in this method. Is that the right way \nto handle failure here? (I'm legitimately asking because I have \nover-used 'die()' in the past and am still unclear on when it is \nappropriate.)\n\n>   '\n>   \n> +test_expect_success 'set up %S tests' '\n> +\tgit checkout --orphan source-a &&\n> +\ttest_commit one &&\n> +\ttest_commit two &&\n> +\tgit checkout -b source-b HEAD^ &&\n> +\ttest_commit three\n> +'\n> +\n> +test_expect_success 'log --format=%S paints branch names' '\n> +\tcat >expect <<-\\EOF &&\n> +\tsource-b\n> +\tsource-a\n> +\tsource-b\n> +\tEOF\n> +\tgit log --format=%S source-a source-b >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --format=%S paints tag names' '\n> +\tgit tag -m tagged source-tag &&\n> +\tcat >expect <<-\\EOF &&\n> +\tsource-tag\n> +\tsource-a\n> +\tsource-tag\n> +\tEOF\n> +\tgit log --format=%S source-tag source-a >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --format=%S paints symmetric ranges' '\n> +\tcat >expect <<-\\EOF &&\n> +\tsource-b\n> +\tsource-a\n> +\tEOF\n> +\tgit log --format=%S source-a...source-b >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success '%S in git log --format works with other placeholders (part 1)' '\n> +\tgit log --format=\"source-b %h\" source-b >expect &&\n> +\tgit log --format=\"%S %h\" source-b >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success '%S in git log --format works with other placeholders (part 2)' '\n> +\tgit log --format=\"%h source-b\" source-b >expect &&\n> +\tgit log --format=\"%h %S\" source-b >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>   test_done\n\nPlease also add a simple test to t6006-rev-list-format.sh.\n\nThanks,\n\n-Stolee\n\n"},{"id":"365906","messageId":"xmqqd0plplhe.fsf@gitster-ct.c.googlers.com","threadId":"50065","inReplyTo":"5dfd92d1-2e87-3006-1630-a33794b6066b@gmail.com","subject":"Re: [PATCH] log: add %S option (like --source) to log --format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-12-28T18:14:21Z","receivedAt":"2018-12-28T18:14:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n>> +++ b/Documentation/pretty-formats.txt\n>> @@ -134,6 +134,8 @@ The placeholders are:\n>>   - '%cI': committer date, strict ISO 8601 format\n>>   - '%d': ref names, like the --decorate option of linkgit:git-log[1]\n>>   - '%D': ref names without the \" (\", \")\" wrapping.\n>> +- '%S': ref name given on the command line by which the commit was reached\n>> +  (like `git log --source`), only works with `git log`\n>\n> This \"only works with `git log`\" made me think about what would happen\n> with `git rev-list --pretty=format:\"%h %S\"` and the answer (on my\n> machine) was a segfault.\n\nThat's a bad one X-<.\n\n>> +\t\tslot = revision_sources_at(c->pretty_ctx->rev->sources, commit);\n>> +\t\tif (slot && *slot) {\n> I'm not sure this check for 'slot' being non-null is necessary, as we\n> would already get a failure in the commit-slab code (for\n> revision_sources_at()) if the slab is not initialized.\n>> +\t\t\tstrbuf_addstr(sb, *slot);\n>> +\t\t\treturn 1;\n>> +\t\t} else {\n>> +\t\t\tdie(_(\"failed to get info for %%S\"));\n>\n> Here, you die() when you fail to get a slot but above you return 0\n> when the sources are not initialized.\n>\n> I don't see another use of die() in this method. Is that the right way\n> to handle failure here? (I'm legitimately asking because I have\n> over-used 'die()' in the past and am still unclear on when it is\n> appropriate.)\n\nThis is definitely a bad one, too.  If '%d' cannot find decoration,\nit would not \"die\".\n\nThanks.\n"},{"id":"366003","messageId":"CANdyxMz8RsCUmumyNP=0Nxh8TpoC2z5TGRW8Hpf_Z_MH7wAj4Q@mail.gmail.com","threadId":"50065","inReplyTo":"5dfd92d1-2e87-3006-1630-a33794b6066b@gmail.com","subject":"Re: [PATCH] log: add %S option (like --source) to log --format","fromName":"Issac Trotts","fromEmail":"issac.trotts@gmail.com","sentAt":"2018-12-31T04:35:02Z","receivedAt":"2018-12-31T04:35:17Z","isPatch":true,"sender":{"key":"issac.trotts@gmail.com","avatar":"https://gravatar.com/avatar/44dd86aa2ea47c79d9b2a0a99b90e0ff18601855d2f05c6b84d6c432c5906721?d=mp&s=160"},"body":"Hi Derrick, thanks for your feedback.\n\nI'll send out a new patch with the changes.\n\nOn Fri, Dec 28, 2018 at 12:20 AM Derrick Stolee <stolee@gmail.com> wrote:\n>\n> On 12/19/2018 3:33 AM, issac.trotts@gmail.com wrote:\n> > From: Issac Trotts <issac.trotts@gmail.com>\n> >\n> > Make it possible to write for example\n> >\n> >          git log --format=\"%H,%S\"\n> >\n> > where the %S at the end is a new placeholder that prints out the ref\n> > (tag/branch) for each commit.\n> >\n> > Using %d might seem like an alternative but it only shows the ref for the last\n> > commit in the branch.\n> >\n> > Signed-off-by: Issac Trotts <issactrotts@google.com>\n> >\n> > ---\n> >\n> > This change is based on a question from Stack Overflow:\n> > https://stackoverflow.com/questions/12712775/git-get-source-information-in-format\n> > ---\n> >   Documentation/pretty-formats.txt |  2 ++\n> >   builtin/log.c                    |  2 +-\n> >   log-tree.c                       |  1 +\n> >   pretty.c                         | 15 ++++++++++\n> >   pretty.h                         |  1 +\n> >   t/t4205-log-pretty-formats.sh    | 50 ++++++++++++++++++++++++++++++++\n> >   6 files changed, 70 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\n> > index 417b638cd..de6953108 100644\n> > --- a/Documentation/pretty-formats.txt\n> > +++ b/Documentation/pretty-formats.txt\n> > @@ -134,6 +134,8 @@ The placeholders are:\n> >   - '%cI': committer date, strict ISO 8601 format\n> >   - '%d': ref names, like the --decorate option of linkgit:git-log[1]\n> >   - '%D': ref names without the \" (\", \")\" wrapping.\n> > +- '%S': ref name given on the command line by which the commit was reached\n> > +  (like `git log --source`), only works with `git log`\n>\n> This \"only works with `git log`\" made me think about what would happen\n> with `git rev-list --pretty=format:\"%h %S\"` and the answer (on my\n> machine) was a segfault.\n\nGood find. Checking for c->pretty_ctx->rev == NULL prevents the seg fault.\n\n> >   - '%e': encoding\n> >   - '%s': subject\n> >   - '%f': sanitized subject line, suitable for a filename\n> > diff --git a/builtin/log.c b/builtin/log.c\n> > index e8e51068b..be3025657 100644\n> > --- a/builtin/log.c\n> > +++ b/builtin/log.c\n> > @@ -203,7 +203,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n> >           rev->diffopt.filter || rev->diffopt.flags.follow_renames)\n> >               rev->always_show_header = 0;\n> >\n> > -     if (source) {\n> > +     if (source || (rev->pretty_given && (rev->commit_format == CMIT_FMT_USERFORMAT) && w.source)) {\n> >               init_revision_sources(&revision_sources);\n> >               rev->sources = &revision_sources;\n> >       }\n>\n> Likely, you'll want to duplicate this initialization in the revision\n> machinery. Keep this one in builtin/log.c as it was before, but add\n> something like this initialization to setup_revisions(). Add a test for\n> rev-list.\n\nOkay, I added a test to check that rev-list leaves %S alone and\nimplicitly that it doesn't segfault.\n\nI'd like to limit the scope of this change to what's already here,\nwith the check preventing the segfault, if that's okay. Could support\nfor %S in rev-list be in a later patch?\n\n>\n> [snip]\n>\n> > @@ -1194,6 +1195,17 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n> >               load_ref_decorations(NULL, DECORATE_SHORT_REFS);\n> >               format_decorations_extended(sb, commit, c->auto_color, \"\", \", \", \"\");\n> >               return 1;\n> > +     case 'S':               /* tag/branch like --source */\n> > +             if (c->pretty_ctx->rev->sources == NULL) {\n> Use \"if (!c->pretty_ctx->rev->sources\".\n> > +                     return 0;\n> > +             }\n> > +             slot = revision_sources_at(c->pretty_ctx->rev->sources, commit);\n> > +             if (slot && *slot) {\n> I'm not sure this check for 'slot' being non-null is necessary, as we\n> would already get a failure in the commit-slab code (for\n> revision_sources_at()) if the slab is not initialized.\n> > +                     strbuf_addstr(sb, *slot);\n> > +                     return 1;\n> > +             } else {\n> > +                     die(_(\"failed to get info for %%S\"));\n>\n> Here, you die() when you fail to get a slot but above you return 0 when\n> the sources are not initialized.\n>\n> I don't see another use of die() in this method. Is that the right way\n> to handle failure here? (I'm legitimately asking because I have\n> over-used 'die()' in the past and am still unclear on when it is\n> appropriate.)\n\nFixed.\n\n>\n> >   '\n> >\n> > +test_expect_success 'set up %S tests' '\n> > +     git checkout --orphan source-a &&\n> > +     test_commit one &&\n> > +     test_commit two &&\n> > +     git checkout -b source-b HEAD^ &&\n> > +     test_commit three\n> > +'\n> > +\n> > +test_expect_success 'log --format=%S paints branch names' '\n> > +     cat >expect <<-\\EOF &&\n> > +     source-b\n> > +     source-a\n> > +     source-b\n> > +     EOF\n> > +     git log --format=%S source-a source-b >actual &&\n> > +     test_cmp expect actual\n> > +'\n> > +\n> > +test_expect_success 'log --format=%S paints tag names' '\n> > +     git tag -m tagged source-tag &&\n> > +     cat >expect <<-\\EOF &&\n> > +     source-tag\n> > +     source-a\n> > +     source-tag\n> > +     EOF\n> > +     git log --format=%S source-tag source-a >actual &&\n> > +     test_cmp expect actual\n> > +'\n> > +\n> > +test_expect_success 'log --format=%S paints symmetric ranges' '\n> > +     cat >expect <<-\\EOF &&\n> > +     source-b\n> > +     source-a\n> > +     EOF\n> > +     git log --format=%S source-a...source-b >actual &&\n> > +     test_cmp expect actual\n> > +'\n> > +\n> > +test_expect_success '%S in git log --format works with other placeholders (part 1)' '\n> > +     git log --format=\"source-b %h\" source-b >expect &&\n> > +     git log --format=\"%S %h\" source-b >actual &&\n> > +     test_cmp expect actual\n> > +'\n> > +\n> > +test_expect_success '%S in git log --format works with other placeholders (part 2)' '\n> > +     git log --format=\"%h source-b\" source-b >expect &&\n> > +     git log --format=\"%h %S\" source-b >actual &&\n> > +     test_cmp expect actual\n> > +'\n> > +\n> >   test_done\n>\n> Please also add a simple test to t6006-rev-list-format.sh.\n\nDone.\n\n>\n> Thanks,\n>\n> -Stolee\n>\n"}]}