[PATCH] '%S' option for pretty printing to support --source

[PATCH] '%S' option for pretty printing to support --source

[Petri Hodju]

From 79f817e25aada377ccb40ebf76c29af7f21e1ec4 Mon Sep 17 00:00:00 2001
From: Petri Hodju <petrihodju@yahoo.com>
Date: Thu, 5 Mar 2009 09:00:39 +0200
Subject: [PATCH] '%S' option for pretty printing to support --source
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

Print out the ref name by which each commit was reached. Works only when --source option is used

Examples:

git-log --graph --pretty=format:"%h(%S) — %s (%cr)" --abbrev-commit --date=relative --source

Show ref by which each commit is reachable in current branch

git-log --graph --pretty=format:"%h(%S) — %s (%cr)" --abbrev-commit --date=relative --source --all

Show ref by which each commit is reachable globally

Signed-off-by: Petri Hodju <petrihodju@yahoo.com>
---
 pretty.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/pretty.c b/pretty.c
index 6cd9149..cf05b37 100644
--- a/pretty.c
+++ b/pretty.c
@@ -544,6 +544,12 @@ static void format_decoration(struct strbuf *sb, const struct commit *commit)
 		strbuf_addch(sb, ')');
 }
 
+static void format_source(struct strbuf *sb, const struct commit *commit)
+{
+    if (commit->util)
+	strbuf_addstr(sb, (char *) commit->util);
+}
+
 static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
                                void *context)
 {
@@ -650,6 +656,9 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 	case 'd':
 		format_decoration(sb, commit);
 		return 1;
+	case 'S':
+		format_source(sb, commit);
+		return 1;
 	}
 
 	/* For the rest we have to parse the commit header. */
-- 
1.6.2.rc2.22.g1d035.dirty

Re: [PATCH] '%S' option for pretty printing to support --source

[Jeff King]

On Thu, Mar 05, 2009 at 09:18:28AM +0200, Petri Hodju wrote:

> +static void format_source(struct strbuf *sb, const struct commit *commit)
> +{
> +    if (commit->util)
> +	strbuf_addstr(sb, (char *) commit->util);
> +}
> +

Hmm. This is the second patch in the last few weeks to use commit->util
to carry information for --pretty=format: (I am cc'ing Deskin Miller,
who wrote the first).

They cannot both work, obviously. So we need to do one of:

  - refactor the information out of commit->util to somewhere else

  - allow multiple commit->util users somehow (which I think is a
    potential performance problem -- the simplistic design is meant to
    avoid things like allocation overhead)

  - gracefully block concurrent use of conflicting features

-Peff

Re: [PATCH] '%S' option for pretty printing to support --source

[Johannes Schindelin]

Hi,

On Thu, 5 Mar 2009, Jeff King wrote:

> On Thu, Mar 05, 2009 at 09:18:28AM +0200, Petri Hodju wrote:
> 
> > +static void format_source(struct strbuf *sb, const struct commit *commit)
> > +{
> > +    if (commit->util)
> > +	strbuf_addstr(sb, (char *) commit->util);
> > +}
> > +
> 
> Hmm. This is the second patch in the last few weeks to use commit->util
> to carry information for --pretty=format: (I am cc'ing Deskin Miller,
> who wrote the first).
> 
> They cannot both work, obviously. So we need to do one of:
> 
>   - refactor the information out of commit->util to somewhere else
> 
>   - allow multiple commit->util users somehow (which I think is a
>     potential performance problem -- the simplistic design is meant to
>     avoid things like allocation overhead)

The common way to do this is to use struct decoration.  I was under the 
impression that --source already used that method (IIRC both --source and 
struct decoration come from Linus, the latter of which having been 
rejected when I submitted it as a struct object_hash patch, which would 
have been a better name IMHO).

Ciao,
Dscho

Re: [PATCH] '%S' option for pretty printing to support --source

[Deskin Miller]

On Thu, Mar 5, 2009 at 04:17, Jeff King <peff@peff.net> wrote:
> On Thu, Mar 05, 2009 at 09:18:28AM +0200, Petri Hodju wrote:
>
>> +static void format_source(struct strbuf *sb, const struct commit *commit)
>> +{
>> +    if (commit->util)
>> +     strbuf_addstr(sb, (char *) commit->util);
>> +}
>> +
>
> Hmm. This is the second patch in the last few weeks to use commit->util
> to carry information for --pretty=format: (I am cc'ing Deskin Miller,
> who wrote the first).

Thanks Jeff.  Fortunately I managed to catch this one anyway.

Petri, the patch series from me which Jeff is referring to is viewable at

http://thread.gmane.org/gmane.comp.version-control.git/111524/

for reference.

I am in the middle of a move and ought to be packing right now, so
needless to say my git budget at the moment is pretty much nil, and
will be so for at least another week I'd guess.  This is to say, I've
not done any additional work in light of Jeff's or Dscho's comments on
my series, though I intend to once I'm relocated.

> They cannot both work, obviously. So we need to do one of:
>
>  - refactor the information out of commit->util to somewhere else
>
>  - allow multiple commit->util users somehow (which I think is a
>    potential performance problem -- the simplistic design is meant to
>    avoid things like allocation overhead)

I'm inclined to do as Dscho suggests here: glancing at the current
struct decoration usage briefly I think my reflog printing could work
that way with no problem.  However, this would largely ignore your
other comments about prettifying the pretty-printing code.  If a new
series using struct decoration isn't useful, let me know, otherwise
I'll plan on doing this once I have a chance.

>  - gracefully block concurrent use of conflicting features

I agree that any blocking should be graceful, but ultimately I find
the idea of disallowing features because they happen to use the same
underlying implementation distasteful.  With a little work we should
be able to allow both with no problem.

Deskin Miller

Re: [PATCH] '%S' option for pretty printing to support --source

[Jeff King]

On Thu, Mar 05, 2009 at 02:41:44PM -0500, Deskin Miller wrote:

> >  - gracefully block concurrent use of conflicting features
> 
> I agree that any blocking should be graceful, but ultimately I find
> the idea of disallowing features because they happen to use the same
> underlying implementation distasteful.  With a little work we should
> be able to allow both with no problem.

Yes, I would prefer to avoid blocking if at all possible. But I included
it as a last resort until things can be fixed correctly. IOW, by
"graceful" I just meant "die with an error instead of segfaulting".
Which would still arguably be a bug, but at least would clue the user in
to what is happening.

Anyway, happy moving and I'll look forward to seeing your patch when it
is ready. :)

-Peff