threads / patch / 18142

patch'%S' option for pretty printing to support --source

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

## tl;dr

5 messages between Mar 5, 2009 and Mar 6, 2009. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Petri Hodju· Mar 5, 2009, 07:18 UTC · lore

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

Show changes to pretty.c +9 −0

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

Jeff King· Mar 5, 2009, 09:17 UTC · re: Petri Hodju · lore

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

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

Show 6 quoted lines

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

Johannes Schindelin· Mar 5, 2009, 10:59 UTC · re: Jeff King · lore

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

Hi,

On Thu, 5 Mar 2009, Jeff King wrote:

Show 20 quoted lines

> 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

Deskin Miller· Mar 5, 2009, 19:41 UTC · re: Jeff King · lore

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

On Thu, Mar 5, 2009 at 04:17, Jeff King <peff@peff.net> wrote:

Show 12 quoted lines

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

Show 7 quoted lines

> 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

Jeff King· Mar 6, 2009, 05:25 UTC · re: Deskin Miller · lore

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

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

Show 6 quoted lines

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