threads / patch / 50049

patchlog: add %S option (like --source) to log --format

Subject: [PATCH] log: add %S option (like --source) to log --format

## tl;dr

8 messages between Dec 17, 2018 and Dec 19, 2018. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Issac Trotts· Dec 17, 2018, 06:25 UTC · lore
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(-)
Show changes to 5 files +64 −1

Documentation/pretty-formats.txt, builtin/log.c, log-tree.c, pretty.c, t/t4205-log-pretty-formats.sh

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· Dec 17, 2018, 09:31 UTC · re: Issac Trotts · lore

Re: [PATCH] log: add %S option (like --source) to log --format

Issac Trotts <issac.trotts@gmail.com> writes:
Show 9 quoted lines
> 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
> ---
Show 17 quoted lines
>  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.
Show 12 quoted lines
> 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?

Show 14 quoted lines
> @@ -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")..

Issac Trotts· Dec 18, 2018, 03:35 UTC · re: Junio C Hamano · lore

Re: [PATCH] log: add %S option (like --source) to log --format

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 <gitster@pobox.com> wrote:
Show 14 quoted lines
>
> Issac Trotts <issac.trotts@gmail.com> 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.
Show 29 quoted lines
> > 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.
Show 15 quoted lines
>
> > 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`.
Show 12 quoted lines
>
> >          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.
Show 17 quoted lines
>
> > @@ -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.
Show 10 quoted lines
>
> >      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.
Jeff King· Dec 17, 2018, 15:59 UTC · re: Issac Trotts · lore

Re: [PATCH] log: add %S option (like --source) to log --format

On Sun, Dec 16, 2018 at 10:25:14PM -0800, Issac Trotts wrote:
Show 6 quoted lines
> 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· Dec 18, 2018, 17:14 UTC · re: Jeff King · lore

Re: [PATCH] log: add %S option (like --source) to log --format

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 <peff@peff.net> wrote:
Show 23 quoted lines
>
> 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· Dec 19, 2018, 03:47 UTC · re: Issac Trotts · lore

Re: [PATCH] log: add %S option (like --source) to log --format

On Tue, Dec 18, 2018 at 9:14 AM Issac Trotts <issac.trotts@gmail.com> wrote:
Show 8 quoted lines
>
> 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.

Show 26 quoted lines
> Issac
>
> On Mon, Dec 17, 2018 at 7:59 AM Jeff King <peff@peff.net> 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· Dec 19, 2018, 08:07 UTC · re: Issac Trotts · lore

Re: [PATCH] log: add %S option (like --source) to log --format

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 <issac.trotts@gmail.com> wrote:
Show 44 quoted lines
>
> On Tue, Dec 18, 2018 at 9:14 AM Issac Trotts <issac.trotts@gmail.com> 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 <peff@peff.net> 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· Dec 19, 2018, 17:09 UTC · re: Issac Trotts · lore

Re: [PATCH] log: add %S option (like --source) to log --format

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 <issac.trotts@gmail.com> wrote:
Show 49 quoted lines
>
> 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 <issac.trotts@gmail.com> wrote:
> >
> > On Tue, Dec 18, 2018 at 9:14 AM Issac Trotts <issac.trotts@gmail.com> 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 <peff@peff.net> 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

← back to recent threads