git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* [PATCH] log: add %S option (like --source) to log --format
@ 2019-01-11  6:30 issac.trotts
  0 siblings, 0 replies; 25+ messages in thread
From: issac.trotts @ 2019-01-11  6:30 UTC (permalink / raw)
  To: git; +Cc: noemi, Issac Trotts, Issac Trotts

From: Issac Trotts <issac.trotts@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.

Signed-off-by: Issac Trotts <issactrotts@google.com>

---

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                         | 12 ++++++++
 pretty.h                         |  1 +
 t/t4205-log-pretty-formats.sh    | 50 ++++++++++++++++++++++++++++++++
 t/t6006-rev-list-format.sh       |  4 +++
 7 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 417b638cd..de6953108 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -134,6 +134,8 @@ The placeholders are:
 - '%cI': committer date, strict ISO 8601 format
 - '%d': ref names, like the --decorate option of linkgit:git-log[1]
 - '%D': ref names without the " (", ")" wrapping.
+- '%S': ref name given on the command line by which the commit was reached
+  (like `git log --source`), only works with `git log`
 - '%e': encoding
 - '%s': subject
 - '%f': sanitized subject line, suitable for a filename
diff --git a/builtin/log.c b/builtin/log.c
index e8e51068b..9deff32b8 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 || w.source) {
 		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..1b453fbb5 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]) {
@@ -1194,6 +1195,14 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 		load_ref_decorations(NULL, DECORATE_SHORT_REFS);
 		format_decorations_extended(sb, commit, c->auto_color, "", ", ", "");
 		return 1;
+	case 'S':		/* tag/branch like --source */
+		if (!(c->pretty_ctx->rev && c->pretty_ctx->rev->sources))
+			return 0;
+		slot = revision_sources_at(c->pretty_ctx->rev->sources, commit);
+		if (!(slot && *slot))
+			return 0;
+		strbuf_addstr(sb, *slot);
+		return 1;
 	case 'g':		/* reflog info */
 		switch(placeholder[1]) {
 		case 'd':	/* reflog selector */
@@ -1498,6 +1507,9 @@ static size_t userformat_want_item(struct strbuf *sb, const char *placeholder,
 	case 'N':
 		w->notes = 1;
 		break;
+	case 'S':
+		w->source = 1;
+		break;
 	}
 	return 0;
 }
diff --git a/pretty.h b/pretty.h
index 7359d318a..87ca5dfcb 100644
--- a/pretty.h
+++ b/pretty.h
@@ -60,6 +60,7 @@ static inline int cmit_fmt_is_mail(enum cmit_fmt fmt)
 
 struct userformat_want {
 	unsigned notes:1;
+	unsigned source:1;
 };
 
 /* Set the flag "w->notes" if there is placeholder %N in "fmt". */
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
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index ec42c2f77..da113d975 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -185,6 +185,10 @@ test_expect_success 'basic colors' '
 	test_cmp expect actual
 '
 
+test_expect_success '%S is not a placeholder for rev-list yet' '
+	git rev-list --format="%S" -1 master | grep "%S"
+'
+
 test_expect_success 'advanced colors' '
 	cat >expect <<-EOF &&
 	commit $head2
-- 
2.19.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2019-01-11  6:28   ` Issac Trotts
  2019-01-11 17:37     ` Junio C Hamano
@ 2019-01-11 17:47     ` Junio C Hamano
  1 sibling, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2019-01-11 17:47 UTC (permalink / raw)
  To: Issac Trotts; +Cc: git, Noemi Mercado, Issac Trotts

Issac Trotts <issac.trotts@gmail.com> writes:

> Sounds good. Btw, did you queue it yet? I didn't see it at the mirror:
> https://github.com/git/git/commits/master.

No patch goes to 'master' directly.  

Once we see that a patch is in reviewable shape during the mailing
list discussion, we wait for a few more days and unless there are
more issues to be addressed discovered during that period, it hits
the 'next' branch, and spends a week or so before graduating.

Before all of that happens, i.e. when a patch has not proven itself
'next'-worthy, I may pick it up to make trial merges in order to see
how it interacts with other proposed updates, which happens in the
'pu' (proposed updates) branch.  As there is only limited amount of
time in a day, this obviously cannot happen to all patches sent to
the list, but I try to cover as much as possible.

You'd find it as 5ca3af27 ("log: add %S option (like --source) to
log --format", 2019-01-09) on 'pu'.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2019-01-11  6:28   ` Issac Trotts
@ 2019-01-11 17:37     ` Junio C Hamano
  2019-01-11 17:47     ` Junio C Hamano
  1 sibling, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2019-01-11 17:37 UTC (permalink / raw)
  To: Issac Trotts; +Cc: git, Noemi Mercado, Issac Trotts

Issac Trotts <issac.trotts@gmail.com> writes:

> On Tue, Jan 8, 2019 at 2:21 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> issac.trotts@gmail.com writes:
>>
>> > From: Issac Trotts <issac.trotts@gmail.com>
>>
>> Heh, I'll edit this line to match S-o-b: below.
>
> Thanks. Otherwise I have to set up git send-email again on my work laptop.

This in-body From: line is either (1) something you would write
yourself in your MUA, in which case you can just write your Google
address, or (2) the author identity a tool picks up and insert at
the body of the message for you, in which case you would make sure
that the commit is made under your Google identity (e.g. "commit
--amend --author=...").  Either case, I am not sure where the need
for send-email comes from.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2019-01-08 22:21 ` Junio C Hamano
@ 2019-01-11  6:28   ` Issac Trotts
  2019-01-11 17:37     ` Junio C Hamano
  2019-01-11 17:47     ` Junio C Hamano
  0 siblings, 2 replies; 25+ messages in thread
From: Issac Trotts @ 2019-01-11  6:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Noemi Mercado, Issac Trotts

On Tue, Jan 8, 2019 at 2:21 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> issac.trotts@gmail.com writes:
>
> > From: Issac Trotts <issac.trotts@gmail.com>
>
> Heh, I'll edit this line to match S-o-b: below.

Thanks. Otherwise I have to set up git send-email again on my work laptop.

> > 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.
> >
> > Signed-off-by: Issac Trotts <issactrotts@google.com>
> >
> > ---
>
>
>
> > 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;
>
> There are quite a few existing codepaths that change their behaviour
> based on NULL-ness of ctx.rev field.  How would we make sure that
> this change have no unintended consequence, I wonder?
>
> > +     case 'S':               /* tag/branch like --source */
> > +             if (c->pretty_ctx->rev == NULL || c->pretty_ctx->rev->sources == NULL) {
> > +                     return 0;
> > +             }
> > +             slot = revision_sources_at(c->pretty_ctx->rev->sources, commit);
> > +             if (!(slot && *slot)) {
> > +                     return 0;
> > +             }
> > +             strbuf_addstr(sb, *slot);
> > +             return 1;
>
> Let's update the style of this hunk here like so:
>
>         if (!c->pretty_ctx->rev || !c->pretty_ctx->rev->sources)
>                 return 0;
>         slot = ...;
>         if (!(slot && *slot))
>                 return 0;
>         strbuf_addstr(...);
>         return 1;

Done.

>
> I wonder if it is even better to apply de-Morgan to one of the above
> two, i.e.
>
>         if (!(c->pretty_ctx->rev && c->pretty_ctx->rev->sources))
>                 return 0;
>

Done.

>
> Anyway, thanks.  Will queue.
>

Sounds good. Btw, did you queue it yet? I didn't see it at the mirror:
https://github.com/git/git/commits/master.

I'll send out another patch with your suggestions, in case you haven't
already queued it.

Issac

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2019-01-08 13:20 issac.trotts
@ 2019-01-08 22:21 ` Junio C Hamano
  2019-01-11  6:28   ` Issac Trotts
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2019-01-08 22:21 UTC (permalink / raw)
  To: issac.trotts; +Cc: git, noemi, Issac Trotts

issac.trotts@gmail.com writes:

> From: Issac Trotts <issac.trotts@gmail.com>

Heh, I'll edit this line to match S-o-b: below.

>
> 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.
>
> Signed-off-by: Issac Trotts <issactrotts@google.com>
>
> ---



> 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;

There are quite a few existing codepaths that change their behaviour
based on NULL-ness of ctx.rev field.  How would we make sure that
this change have no unintended consequence, I wonder?

> +	case 'S':		/* tag/branch like --source */
> +		if (c->pretty_ctx->rev == NULL || c->pretty_ctx->rev->sources == NULL) {
> +			return 0;
> +		}
> +		slot = revision_sources_at(c->pretty_ctx->rev->sources, commit);
> +		if (!(slot && *slot)) {
> +			return 0;
> +		}
> +		strbuf_addstr(sb, *slot);
> +		return 1;

Let's update the style of this hunk here like so:

	if (!c->pretty_ctx->rev || !c->pretty_ctx->rev->sources)
		return 0;
	slot = ...;
	if (!(slot && *slot))
		return 0;
	strbuf_addstr(...);
	return 1;

I wonder if it is even better to apply de-Morgan to one of the above
two, i.e.

	if (!(c->pretty_ctx->rev && c->pretty_ctx->rev->sources))
		return 0;


Anyway, thanks.  Will queue.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] log: add %S option (like --source) to log --format
@ 2019-01-08 13:20 issac.trotts
  2019-01-08 22:21 ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: issac.trotts @ 2019-01-08 13:20 UTC (permalink / raw)
  To: git; +Cc: noemi, Issac Trotts, Issac Trotts

From: Issac Trotts <issac.trotts@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.

Signed-off-by: Issac Trotts <issactrotts@google.com>

---

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                         | 14 +++++++++
 pretty.h                         |  1 +
 t/t4205-log-pretty-formats.sh    | 50 ++++++++++++++++++++++++++++++++
 t/t6006-rev-list-format.sh       |  4 +++
 7 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 417b638cd..de6953108 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -134,6 +134,8 @@ The placeholders are:
 - '%cI': committer date, strict ISO 8601 format
 - '%d': ref names, like the --decorate option of linkgit:git-log[1]
 - '%D': ref names without the " (", ")" wrapping.
+- '%S': ref name given on the command line by which the commit was reached
+  (like `git log --source`), only works with `git log`
 - '%e': encoding
 - '%s': subject
 - '%f': sanitized subject line, suitable for a filename
diff --git a/builtin/log.c b/builtin/log.c
index e8e51068b..9deff32b8 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 || w.source) {
 		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..06075d625 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]) {
@@ -1194,6 +1195,16 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 		load_ref_decorations(NULL, DECORATE_SHORT_REFS);
 		format_decorations_extended(sb, commit, c->auto_color, "", ", ", "");
 		return 1;
+	case 'S':		/* tag/branch like --source */
+		if (c->pretty_ctx->rev == NULL || c->pretty_ctx->rev->sources == NULL) {
+			return 0;
+		}
+		slot = revision_sources_at(c->pretty_ctx->rev->sources, commit);
+		if (!(slot && *slot)) {
+			return 0;
+		}
+		strbuf_addstr(sb, *slot);
+		return 1;
 	case 'g':		/* reflog info */
 		switch(placeholder[1]) {
 		case 'd':	/* reflog selector */
@@ -1498,6 +1509,9 @@ static size_t userformat_want_item(struct strbuf *sb, const char *placeholder,
 	case 'N':
 		w->notes = 1;
 		break;
+	case 'S':
+		w->source = 1;
+		break;
 	}
 	return 0;
 }
diff --git a/pretty.h b/pretty.h
index 7359d318a..87ca5dfcb 100644
--- a/pretty.h
+++ b/pretty.h
@@ -60,6 +60,7 @@ static inline int cmit_fmt_is_mail(enum cmit_fmt fmt)
 
 struct userformat_want {
 	unsigned notes:1;
+	unsigned source:1;
 };
 
 /* Set the flag "w->notes" if there is placeholder %N in "fmt". */
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
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index ec42c2f77..da113d975 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -185,6 +185,10 @@ test_expect_success 'basic colors' '
 	test_cmp expect actual
 '
 
+test_expect_success '%S is not a placeholder for rev-list yet' '
+	git rev-list --format="%S" -1 master | grep "%S"
+'
+
 test_expect_success 'advanced colors' '
 	cat >expect <<-EOF &&
 	commit $head2
-- 
2.19.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2019-01-02 19:33 ` Junio C Hamano
@ 2019-01-08 13:14   ` Issac Trotts
  0 siblings, 0 replies; 25+ messages in thread
From: Issac Trotts @ 2019-01-08 13:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Noemi Mercado, Issac Trotts

On Thu, Jan 3, 2019 at 6:33 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> issac.trotts@gmail.com writes:
>
> > From: Issac Trotts <issac.trotts@gmail.com>
>
> I think you want to have
>
>         From: Issac Trotts <issactrotts@google.com>
>
> instead, so that the authorship actually matches your sign-off.

Makes sense. I'll do that.

>
> > -     if (source) {
> > +     if (source || (rev->pretty_given && (rev->commit_format == CMIT_FMT_USERFORMAT) && w.source)) {
>
> I am not sure why the above is not simply
>
>         if (source || w.source) {
>
> but needs to check pretty-given etc.  Care to explain why?

Good idea. Your simplified version passes all the tests.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2018-12-31  4:53 issac.trotts
@ 2019-01-02 19:33 ` Junio C Hamano
  2019-01-08 13:14   ` Issac Trotts
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2019-01-02 19:33 UTC (permalink / raw)
  To: issac.trotts; +Cc: git, noemi, Issac Trotts

issac.trotts@gmail.com writes:

> From: Issac Trotts <issac.trotts@gmail.com>

I think you want to have 

	From: Issac Trotts <issactrotts@google.com>

instead, so that the authorship actually matches your sign-off.

> -	if (source) {
> +	if (source || (rev->pretty_given && (rev->commit_format == CMIT_FMT_USERFORMAT) && w.source)) {

I am not sure why the above is not simply

	if (source || w.source) {

but needs to check pretty-given etc.  Care to explain why?

Thanks.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] log: add %S option (like --source) to log --format
@ 2018-12-31 21:48 issac.trotts
  0 siblings, 0 replies; 25+ messages in thread
From: issac.trotts @ 2018-12-31 21:48 UTC (permalink / raw)
  To: git; +Cc: noemi, Issac Trotts, Issac Trotts

From: Issac Trotts <issac.trotts@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.

Signed-off-by: Issac Trotts <issactrotts@google.com>

---

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                         | 14 +++++++++
 pretty.h                         |  1 +
 t/t4205-log-pretty-formats.sh    | 50 ++++++++++++++++++++++++++++++++
 t/t6006-rev-list-format.sh       |  4 +++
 7 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 417b638cd..de6953108 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -134,6 +134,8 @@ The placeholders are:
 - '%cI': committer date, strict ISO 8601 format
 - '%d': ref names, like the --decorate option of linkgit:git-log[1]
 - '%D': ref names without the " (", ")" wrapping.
+- '%S': ref name given on the command line by which the commit was reached
+  (like `git log --source`), only works with `git log`
 - '%e': encoding
 - '%s': subject
 - '%f': sanitized subject line, suitable for a filename
diff --git a/builtin/log.c b/builtin/log.c
index e8e51068b..be3025657 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) && w.source)) {
 		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..06075d625 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]) {
@@ -1194,6 +1195,16 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 		load_ref_decorations(NULL, DECORATE_SHORT_REFS);
 		format_decorations_extended(sb, commit, c->auto_color, "", ", ", "");
 		return 1;
+	case 'S':		/* tag/branch like --source */
+		if (c->pretty_ctx->rev == NULL || c->pretty_ctx->rev->sources == NULL) {
+			return 0;
+		}
+		slot = revision_sources_at(c->pretty_ctx->rev->sources, commit);
+		if (!(slot && *slot)) {
+			return 0;
+		}
+		strbuf_addstr(sb, *slot);
+		return 1;
 	case 'g':		/* reflog info */
 		switch(placeholder[1]) {
 		case 'd':	/* reflog selector */
@@ -1498,6 +1509,9 @@ static size_t userformat_want_item(struct strbuf *sb, const char *placeholder,
 	case 'N':
 		w->notes = 1;
 		break;
+	case 'S':
+		w->source = 1;
+		break;
 	}
 	return 0;
 }
diff --git a/pretty.h b/pretty.h
index 7359d318a..87ca5dfcb 100644
--- a/pretty.h
+++ b/pretty.h
@@ -60,6 +60,7 @@ static inline int cmit_fmt_is_mail(enum cmit_fmt fmt)
 
 struct userformat_want {
 	unsigned notes:1;
+	unsigned source:1;
 };
 
 /* Set the flag "w->notes" if there is placeholder %N in "fmt". */
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
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index ec42c2f77..da113d975 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -185,6 +185,10 @@ test_expect_success 'basic colors' '
 	test_cmp expect actual
 '
 
+test_expect_success '%S is not a placeholder for rev-list yet' '
+	git rev-list --format="%S" -1 master | grep "%S"
+'
+
 test_expect_success 'advanced colors' '
 	cat >expect <<-EOF &&
 	commit $head2
-- 
2.19.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] log: add %S option (like --source) to log --format
@ 2018-12-31  4:53 issac.trotts
  2019-01-02 19:33 ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: issac.trotts @ 2018-12-31  4:53 UTC (permalink / raw)
  To: git, noemi; +Cc: Issac Trotts, Issac Trotts

From: Issac Trotts <issac.trotts@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.

Signed-off-by: Issac Trotts <issactrotts@google.com>

---

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                         | 16 ++++++++++
 pretty.h                         |  1 +
 t/t4205-log-pretty-formats.sh    | 50 ++++++++++++++++++++++++++++++++
 t/t6006-rev-list-format.sh       |  4 +++
 7 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 417b638cd..de6953108 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -134,6 +134,8 @@ The placeholders are:
 - '%cI': committer date, strict ISO 8601 format
 - '%d': ref names, like the --decorate option of linkgit:git-log[1]
 - '%D': ref names without the " (", ")" wrapping.
+- '%S': ref name given on the command line by which the commit was reached
+  (like `git log --source`), only works with `git log`
 - '%e': encoding
 - '%s': subject
 - '%f': sanitized subject line, suitable for a filename
diff --git a/builtin/log.c b/builtin/log.c
index e8e51068b..be3025657 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) && w.source)) {
 		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..d1981871e 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]) {
@@ -1194,6 +1195,18 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 		load_ref_decorations(NULL, DECORATE_SHORT_REFS);
 		format_decorations_extended(sb, commit, c->auto_color, "", ", ", "");
 		return 1;
+	case 'S':		/* tag/branch like --source */
+		if (c->pretty_ctx->rev == NULL || c->pretty_ctx->rev->sources == NULL) {
+			return 0;
+		}
+		slot = revision_sources_at(c->pretty_ctx->rev->sources, commit);
+		if (!(slot && *slot)) {
+			return 0;
+		}
+		if (slot && *slot) {
+			strbuf_addstr(sb, *slot);
+			return 1;
+		}
 	case 'g':		/* reflog info */
 		switch(placeholder[1]) {
 		case 'd':	/* reflog selector */
@@ -1498,6 +1511,9 @@ static size_t userformat_want_item(struct strbuf *sb, const char *placeholder,
 	case 'N':
 		w->notes = 1;
 		break;
+	case 'S':
+		w->source = 1;
+		break;
 	}
 	return 0;
 }
diff --git a/pretty.h b/pretty.h
index 7359d318a..87ca5dfcb 100644
--- a/pretty.h
+++ b/pretty.h
@@ -60,6 +60,7 @@ static inline int cmit_fmt_is_mail(enum cmit_fmt fmt)
 
 struct userformat_want {
 	unsigned notes:1;
+	unsigned source:1;
 };
 
 /* Set the flag "w->notes" if there is placeholder %N in "fmt". */
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
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index ec42c2f77..da113d975 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -185,6 +185,10 @@ test_expect_success 'basic colors' '
 	test_cmp expect actual
 '
 
+test_expect_success '%S is not a placeholder for rev-list yet' '
+	git rev-list --format="%S" -1 master | grep "%S"
+'
+
 test_expect_success 'advanced colors' '
 	cat >expect <<-EOF &&
 	commit $head2
-- 
2.19.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2018-12-27 13:20 ` Derrick Stolee
  2018-12-28 18:14   ` Junio C Hamano
@ 2018-12-31  4:35   ` Issac Trotts
  1 sibling, 0 replies; 25+ messages in thread
From: Issac Trotts @ 2018-12-31  4:35 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: git, Noemi Mercado, Issac Trotts

Hi Derrick, thanks for your feedback.

I'll send out a new patch with the changes.

On Fri, Dec 28, 2018 at 12:20 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 12/19/2018 3:33 AM, issac.trotts@gmail.com wrote:
> > From: Issac Trotts <issac.trotts@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.
> >
> > Signed-off-by: Issac Trotts <issactrotts@google.com>
> >
> > ---
> >
> > 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                         | 15 ++++++++++
> >   pretty.h                         |  1 +
> >   t/t4205-log-pretty-formats.sh    | 50 ++++++++++++++++++++++++++++++++
> >   6 files changed, 70 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
> > index 417b638cd..de6953108 100644
> > --- a/Documentation/pretty-formats.txt
> > +++ b/Documentation/pretty-formats.txt
> > @@ -134,6 +134,8 @@ The placeholders are:
> >   - '%cI': committer date, strict ISO 8601 format
> >   - '%d': ref names, like the --decorate option of linkgit:git-log[1]
> >   - '%D': ref names without the " (", ")" wrapping.
> > +- '%S': ref name given on the command line by which the commit was reached
> > +  (like `git log --source`), only works with `git log`
>
> This "only works with `git log`" made me think about what would happen
> with `git rev-list --pretty=format:"%h %S"` and the answer (on my
> machine) was a segfault.

Good find. Checking for c->pretty_ctx->rev == NULL prevents the seg fault.

> >   - '%e': encoding
> >   - '%s': subject
> >   - '%f': sanitized subject line, suitable for a filename
> > diff --git a/builtin/log.c b/builtin/log.c
> > index e8e51068b..be3025657 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) && w.source)) {
> >               init_revision_sources(&revision_sources);
> >               rev->sources = &revision_sources;
> >       }
>
> Likely, you'll want to duplicate this initialization in the revision
> machinery. Keep this one in builtin/log.c as it was before, but add
> something like this initialization to setup_revisions(). Add a test for
> rev-list.

Okay, I added a test to check that rev-list leaves %S alone and
implicitly that it doesn't segfault.

I'd like to limit the scope of this change to what's already here,
with the check preventing the segfault, if that's okay. Could support
for %S in rev-list be in a later patch?

>
> [snip]
>
> > @@ -1194,6 +1195,17 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
> >               load_ref_decorations(NULL, DECORATE_SHORT_REFS);
> >               format_decorations_extended(sb, commit, c->auto_color, "", ", ", "");
> >               return 1;
> > +     case 'S':               /* tag/branch like --source */
> > +             if (c->pretty_ctx->rev->sources == NULL) {
> Use "if (!c->pretty_ctx->rev->sources".
> > +                     return 0;
> > +             }
> > +             slot = revision_sources_at(c->pretty_ctx->rev->sources, commit);
> > +             if (slot && *slot) {
> I'm not sure this check for 'slot' being non-null is necessary, as we
> would already get a failure in the commit-slab code (for
> revision_sources_at()) if the slab is not initialized.
> > +                     strbuf_addstr(sb, *slot);
> > +                     return 1;
> > +             } else {
> > +                     die(_("failed to get info for %%S"));
>
> Here, you die() when you fail to get a slot but above you return 0 when
> the sources are not initialized.
>
> I don't see another use of die() in this method. Is that the right way
> to handle failure here? (I'm legitimately asking because I have
> over-used 'die()' in the past and am still unclear on when it is
> appropriate.)

Fixed.

>
> >   '
> >
> > +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
>
> Please also add a simple test to t6006-rev-list-format.sh.

Done.

>
> Thanks,
>
> -Stolee
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2018-12-27 13:20 ` Derrick Stolee
@ 2018-12-28 18:14   ` Junio C Hamano
  2018-12-31  4:35   ` Issac Trotts
  1 sibling, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2018-12-28 18:14 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: issac.trotts, git, noemi, Issac Trotts

Derrick Stolee <stolee@gmail.com> writes:

>> +++ b/Documentation/pretty-formats.txt
>> @@ -134,6 +134,8 @@ The placeholders are:
>>   - '%cI': committer date, strict ISO 8601 format
>>   - '%d': ref names, like the --decorate option of linkgit:git-log[1]
>>   - '%D': ref names without the " (", ")" wrapping.
>> +- '%S': ref name given on the command line by which the commit was reached
>> +  (like `git log --source`), only works with `git log`
>
> This "only works with `git log`" made me think about what would happen
> with `git rev-list --pretty=format:"%h %S"` and the answer (on my
> machine) was a segfault.

That's a bad one X-<.

>> +		slot = revision_sources_at(c->pretty_ctx->rev->sources, commit);
>> +		if (slot && *slot) {
> I'm not sure this check for 'slot' being non-null is necessary, as we
> would already get a failure in the commit-slab code (for
> revision_sources_at()) if the slab is not initialized.
>> +			strbuf_addstr(sb, *slot);
>> +			return 1;
>> +		} else {
>> +			die(_("failed to get info for %%S"));
>
> Here, you die() when you fail to get a slot but above you return 0
> when the sources are not initialized.
>
> I don't see another use of die() in this method. Is that the right way
> to handle failure here? (I'm legitimately asking because I have
> over-used 'die()' in the past and am still unclear on when it is
> appropriate.)

This is definitely a bad one, too.  If '%d' cannot find decoration,
it would not "die".

Thanks.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2018-12-19  8:33 issac.trotts
  2018-12-21  5:24 ` Issac Trotts
@ 2018-12-27 13:20 ` Derrick Stolee
  2018-12-28 18:14   ` Junio C Hamano
  2018-12-31  4:35   ` Issac Trotts
  1 sibling, 2 replies; 25+ messages in thread
From: Derrick Stolee @ 2018-12-27 13:20 UTC (permalink / raw)
  To: issac.trotts, git; +Cc: noemi, Issac Trotts

On 12/19/2018 3:33 AM, issac.trotts@gmail.com wrote:
> From: Issac Trotts <issac.trotts@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.
>
> Signed-off-by: Issac Trotts <issactrotts@google.com>
>
> ---
>
> 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                         | 15 ++++++++++
>   pretty.h                         |  1 +
>   t/t4205-log-pretty-formats.sh    | 50 ++++++++++++++++++++++++++++++++
>   6 files changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
> index 417b638cd..de6953108 100644
> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -134,6 +134,8 @@ The placeholders are:
>   - '%cI': committer date, strict ISO 8601 format
>   - '%d': ref names, like the --decorate option of linkgit:git-log[1]
>   - '%D': ref names without the " (", ")" wrapping.
> +- '%S': ref name given on the command line by which the commit was reached
> +  (like `git log --source`), only works with `git log`

This "only works with `git log`" made me think about what would happen 
with `git rev-list --pretty=format:"%h %S"` and the answer (on my 
machine) was a segfault.

>   - '%e': encoding
>   - '%s': subject
>   - '%f': sanitized subject line, suitable for a filename
> diff --git a/builtin/log.c b/builtin/log.c
> index e8e51068b..be3025657 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) && w.source)) {
>   		init_revision_sources(&revision_sources);
>   		rev->sources = &revision_sources;
>   	}

Likely, you'll want to duplicate this initialization in the revision 
machinery. Keep this one in builtin/log.c as it was before, but add 
something like this initialization to setup_revisions(). Add a test for 
rev-list.

[snip]

> @@ -1194,6 +1195,17 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
>   		load_ref_decorations(NULL, DECORATE_SHORT_REFS);
>   		format_decorations_extended(sb, commit, c->auto_color, "", ", ", "");
>   		return 1;
> +	case 'S':		/* tag/branch like --source */
> +		if (c->pretty_ctx->rev->sources == NULL) {
Use "if (!c->pretty_ctx->rev->sources".
> +			return 0;
> +		}
> +		slot = revision_sources_at(c->pretty_ctx->rev->sources, commit);
> +		if (slot && *slot) {
I'm not sure this check for 'slot' being non-null is necessary, as we 
would already get a failure in the commit-slab code (for 
revision_sources_at()) if the slab is not initialized.
> +			strbuf_addstr(sb, *slot);
> +			return 1;
> +		} else {
> +			die(_("failed to get info for %%S"));

Here, you die() when you fail to get a slot but above you return 0 when 
the sources are not initialized.

I don't see another use of die() in this method. Is that the right way 
to handle failure here? (I'm legitimately asking because I have 
over-used 'die()' in the past and am still unclear on when it is 
appropriate.)

>   '
>   
> +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

Please also add a simple test to t6006-rev-list-format.sh.

Thanks,

-Stolee


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2018-12-22 22:22   ` Jeff King
@ 2018-12-25  2:12     ` Issac Trotts
  0 siblings, 0 replies; 25+ messages in thread
From: Issac Trotts @ 2018-12-25  2:12 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Noemi Mercado, Issac Trotts

Got it, sounds good.

On Sat, Dec 22, 2018 at 2:22 PM Jeff King <peff@peff.net> wrote:
>
> On Thu, Dec 20, 2018 at 09:24:10PM -0800, Issac Trotts wrote:
>
> > Hi all, friendly ping. Is there still interest in merging this patch?
>
> Yes, but I think people are largely absent for holidays at this point.
> If there hasn't been any movement on it, I'll try to review after Jan
> 5th (it also wouldn't hurt to re-post around then).
>
> -Peff

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2018-12-21  5:24 ` Issac Trotts
@ 2018-12-22 22:22   ` Jeff King
  2018-12-25  2:12     ` Issac Trotts
  0 siblings, 1 reply; 25+ messages in thread
From: Jeff King @ 2018-12-22 22:22 UTC (permalink / raw)
  To: Issac Trotts; +Cc: git, Noemi Mercado, Issac Trotts

On Thu, Dec 20, 2018 at 09:24:10PM -0800, Issac Trotts wrote:

> Hi all, friendly ping. Is there still interest in merging this patch?

Yes, but I think people are largely absent for holidays at this point.
If there hasn't been any movement on it, I'll try to review after Jan
5th (it also wouldn't hurt to re-post around then).

-Peff

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2018-12-19  8:33 issac.trotts
@ 2018-12-21  5:24 ` Issac Trotts
  2018-12-22 22:22   ` Jeff King
  2018-12-27 13:20 ` Derrick Stolee
  1 sibling, 1 reply; 25+ messages in thread
From: Issac Trotts @ 2018-12-21  5:24 UTC (permalink / raw)
  To: git; +Cc: Noemi Mercado, Issac Trotts

Hi all, friendly ping. Is there still interest in merging this patch?

Thanks,
Issac

On Wed, Dec 19, 2018 at 12:33 AM <issac.trotts@gmail.com> wrote:
>
> From: Issac Trotts <issac.trotts@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.
>
> Signed-off-by: Issac Trotts <issactrotts@google.com>
>
> ---
>
> 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                         | 15 ++++++++++
>  pretty.h                         |  1 +
>  t/t4205-log-pretty-formats.sh    | 50 ++++++++++++++++++++++++++++++++
>  6 files changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
> index 417b638cd..de6953108 100644
> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -134,6 +134,8 @@ The placeholders are:
>  - '%cI': committer date, strict ISO 8601 format
>  - '%d': ref names, like the --decorate option of linkgit:git-log[1]
>  - '%D': ref names without the " (", ")" wrapping.
> +- '%S': ref name given on the command line by which the commit was reached
> +  (like `git log --source`), only works with `git log`
>  - '%e': encoding
>  - '%s': subject
>  - '%f': sanitized subject line, suitable for a filename
> diff --git a/builtin/log.c b/builtin/log.c
> index e8e51068b..be3025657 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) && w.source)) {
>                 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..258e86e9c 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]) {
> @@ -1194,6 +1195,17 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
>                 load_ref_decorations(NULL, DECORATE_SHORT_REFS);
>                 format_decorations_extended(sb, commit, c->auto_color, "", ", ", "");
>                 return 1;
> +       case 'S':               /* tag/branch like --source */
> +               if (c->pretty_ctx->rev->sources == NULL) {
> +                       return 0;
> +               }
> +               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"));
> +               }
>         case 'g':               /* reflog info */
>                 switch(placeholder[1]) {
>                 case 'd':       /* reflog selector */
> @@ -1498,6 +1510,9 @@ static size_t userformat_want_item(struct strbuf *sb, const char *placeholder,
>         case 'N':
>                 w->notes = 1;
>                 break;
> +       case 'S':
> +               w->source = 1;
> +               break;
>         }
>         return 0;
>  }
> diff --git a/pretty.h b/pretty.h
> index 7359d318a..87ca5dfcb 100644
> --- a/pretty.h
> +++ b/pretty.h
> @@ -60,6 +60,7 @@ static inline int cmit_fmt_is_mail(enum cmit_fmt fmt)
>
>  struct userformat_want {
>         unsigned notes:1;
> +       unsigned source:1;
>  };
>
>  /* Set the flag "w->notes" if there is placeholder %N in "fmt". */
> 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
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2018-12-19  8:07       ` Issac Trotts
@ 2018-12-19 17:09         ` Issac Trotts
  0 siblings, 0 replies; 25+ messages in thread
From: Issac Trotts @ 2018-12-19 17:09 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, Noemi Mercado

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:
>
> 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] log: add %S option (like --source) to log --format
@ 2018-12-19  8:33 issac.trotts
  2018-12-21  5:24 ` Issac Trotts
  2018-12-27 13:20 ` Derrick Stolee
  0 siblings, 2 replies; 25+ messages in thread
From: issac.trotts @ 2018-12-19  8:33 UTC (permalink / raw)
  To: git; +Cc: noemi, Issac Trotts, Issac Trotts

From: Issac Trotts <issac.trotts@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.

Signed-off-by: Issac Trotts <issactrotts@google.com>

---

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                         | 15 ++++++++++
 pretty.h                         |  1 +
 t/t4205-log-pretty-formats.sh    | 50 ++++++++++++++++++++++++++++++++
 6 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 417b638cd..de6953108 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -134,6 +134,8 @@ The placeholders are:
 - '%cI': committer date, strict ISO 8601 format
 - '%d': ref names, like the --decorate option of linkgit:git-log[1]
 - '%D': ref names without the " (", ")" wrapping.
+- '%S': ref name given on the command line by which the commit was reached
+  (like `git log --source`), only works with `git log`
 - '%e': encoding
 - '%s': subject
 - '%f': sanitized subject line, suitable for a filename
diff --git a/builtin/log.c b/builtin/log.c
index e8e51068b..be3025657 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) && w.source)) {
 		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..258e86e9c 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]) {
@@ -1194,6 +1195,17 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 		load_ref_decorations(NULL, DECORATE_SHORT_REFS);
 		format_decorations_extended(sb, commit, c->auto_color, "", ", ", "");
 		return 1;
+	case 'S':		/* tag/branch like --source */
+		if (c->pretty_ctx->rev->sources == NULL) {
+			return 0;
+		}
+		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"));
+		}
 	case 'g':		/* reflog info */
 		switch(placeholder[1]) {
 		case 'd':	/* reflog selector */
@@ -1498,6 +1510,9 @@ static size_t userformat_want_item(struct strbuf *sb, const char *placeholder,
 	case 'N':
 		w->notes = 1;
 		break;
+	case 'S':
+		w->source = 1;
+		break;
 	}
 	return 0;
 }
diff --git a/pretty.h b/pretty.h
index 7359d318a..87ca5dfcb 100644
--- a/pretty.h
+++ b/pretty.h
@@ -60,6 +60,7 @@ static inline int cmit_fmt_is_mail(enum cmit_fmt fmt)
 
 struct userformat_want {
 	unsigned notes:1;
+	unsigned source:1;
 };
 
 /* Set the flag "w->notes" if there is placeholder %N in "fmt". */
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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2018-12-19  3:47     ` Issac Trotts
@ 2018-12-19  8:07       ` Issac Trotts
  2018-12-19 17:09         ` Issac Trotts
  0 siblings, 1 reply; 25+ messages in thread
From: Issac Trotts @ 2018-12-19  8:07 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, Noemi Mercado

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2018-12-18 17:14   ` Issac Trotts
@ 2018-12-19  3:47     ` Issac Trotts
  2018-12-19  8:07       ` Issac Trotts
  0 siblings, 1 reply; 25+ messages in thread
From: Issac Trotts @ 2018-12-19  3:47 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, Noemi Mercado

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2018-12-17 15:59 ` Jeff King
@ 2018-12-18 17:14   ` Issac Trotts
  2018-12-19  3:47     ` Issac Trotts
  0 siblings, 1 reply; 25+ messages in thread
From: Issac Trotts @ 2018-12-18 17:14 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, Noemi Mercado

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:
>
> 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2018-12-17  9:31 ` Junio C Hamano
@ 2018-12-18  3:35   ` Issac Trotts
  0 siblings, 0 replies; 25+ messages in thread
From: Issac Trotts @ 2018-12-18  3:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Noemi Mercado

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:
>
> 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.

> > 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.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2018-12-17  6:25 Issac Trotts
  2018-12-17  9:31 ` Junio C Hamano
@ 2018-12-17 15:59 ` Jeff King
  2018-12-18 17:14   ` Issac Trotts
  1 sibling, 1 reply; 25+ messages in thread
From: Jeff King @ 2018-12-17 15:59 UTC (permalink / raw)
  To: Issac Trotts; +Cc: Junio C Hamano, git, Noemi Mercado

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] log: add %S option (like --source) to log --format
  2018-12-17  6:25 Issac Trotts
@ 2018-12-17  9:31 ` Junio C Hamano
  2018-12-18  3:35   ` Issac Trotts
  2018-12-17 15:59 ` Jeff King
  1 sibling, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2018-12-17  9:31 UTC (permalink / raw)
  To: Issac Trotts; +Cc: git, Noemi Mercado

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),

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")..

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] log: add %S option (like --source) to log --format
@ 2018-12-17  6:25 Issac Trotts
  2018-12-17  9:31 ` Junio C Hamano
  2018-12-17 15:59 ` Jeff King
  0 siblings, 2 replies; 25+ messages in thread
From: Issac Trotts @ 2018-12-17  6:25 UTC (permalink / raw)
  To: git; +Cc: Noemi Mercado

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, back to index

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-11  6:30 [PATCH] log: add %S option (like --source) to log --format issac.trotts
  -- strict thread matches above, loose matches on Subject: below --
2019-01-08 13:20 issac.trotts
2019-01-08 22:21 ` Junio C Hamano
2019-01-11  6:28   ` Issac Trotts
2019-01-11 17:37     ` Junio C Hamano
2019-01-11 17:47     ` Junio C Hamano
2018-12-31 21:48 issac.trotts
2018-12-31  4:53 issac.trotts
2019-01-02 19:33 ` Junio C Hamano
2019-01-08 13:14   ` Issac Trotts
2018-12-19  8:33 issac.trotts
2018-12-21  5:24 ` Issac Trotts
2018-12-22 22:22   ` Jeff King
2018-12-25  2:12     ` Issac Trotts
2018-12-27 13:20 ` Derrick Stolee
2018-12-28 18:14   ` Junio C Hamano
2018-12-31  4:35   ` Issac Trotts
2018-12-17  6:25 Issac Trotts
2018-12-17  9:31 ` Junio C Hamano
2018-12-18  3:35   ` Issac Trotts
2018-12-17 15:59 ` Jeff King
2018-12-18 17:14   ` Issac Trotts
2018-12-19  3:47     ` Issac Trotts
2018-12-19  8:07       ` Issac Trotts
2018-12-19 17:09         ` Issac Trotts

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox