{"thread":{"id":"15107","subject":"[BUG?] git log does not decorate when custom format is used","startedAt":"2008-08-20T12:25:30Z","lastAt":"2008-09-03T19:36:15Z","messageCount":14,"participants":["MichaelTiloDressel@t-online.de","Jeff King","Junio C Hamano","Johannes Schindelin","Michael Dressel","Teemu Likonen","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"87816","messageId":"1KVmkk-2IOrGi0@fwd26.aul.t-online.de","threadId":"15107","inReplyTo":null,"subject":"[BUG?] git log does not decorate when custom format is used","fromName":"MichaelTiloDressel@t-online.de","fromEmail":"michaeltilodressel@t-online.de","sentAt":"2008-08-20T12:25:30Z","receivedAt":"2008-08-20T12:25:30Z","isPatch":false,"sender":{"key":"michaeltilodressel@t-online.de","avatar":null},"body":"Hi,\n\nis it a bug?\n\nWhen I use something like:\ngit log --pretty=format:'%H %s' --decorate\nI do not get the decoration.\nWhile \ngit log --pretty=oneline --decorate\ndoes decorate.\n\nI'm using:\ngit version 1.5.6.1\n\nCheers,\nMichael\n"},{"id":"87834","messageId":"20080820175325.GD27773@sigill.intra.peff.net","threadId":"15107","inReplyTo":"1KVmkk-2IOrGi0@fwd26.aul.t-online.de","subject":"Re: [BUG?] git log does not decorate when custom format is used","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-08-20T17:53:25Z","receivedAt":"2008-08-20T17:53:25Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 20, 2008 at 02:25:30PM +0200, MichaelTiloDressel@t-online.de wrote:\n\n> is it a bug?\n\nSort of. More like a missing feature.\n\n> When I use something like:\n> git log --pretty=format:'%H %s' --decorate\n> I do not get the decoration.\n> While \n> git log --pretty=oneline --decorate\n> does decorate.\n\nThe problem is where the decoration would go. I think it makes sense not\nto show the decoration automatically in that case, since its placement\ndepends on the user formatting.\n\nThe right solution would be a '%d' placeholder to include the\ndecoration. Patch series to follow:\n\n [1/2] decorate: allow const objects to be decorated\n [2/2] allow '%d' pretty format specifier to show decoration\n\n-Peff\n"},{"id":"87835","messageId":"20080820175532.GA32005@sigill.intra.peff.net","threadId":"15107","inReplyTo":"20080820175325.GD27773@sigill.intra.peff.net","subject":"[PATCH 1/2] decorate: allow const objects to be decorated","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-08-20T17:55:33Z","receivedAt":"2008-08-20T17:55:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We don't actually modify the struct object, so there is no\nreason not to accept const versions (and this allows other\ncallsites, like the next patch, to use the decoration\nmachinery).\n\nSigned-off-by: Jeff King \n---\nThis one is hopefully a no-brainer, and is required for the next patch.\n\n decorate.c | 11 ++++++-----\n decorate.h | 6 +++---\n 2 files changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/decorate.c b/decorate.c\nindex d9668d2..82d9e22 100644\n--- a/decorate.c\n+++ b/decorate.c\n@@ -6,13 +6,13 @@\n #include \"object.h\"\n #include \"decorate.h\"\n \n-static unsigned int hash_obj(struct object *obj, unsigned int n)\n+static unsigned int hash_obj(const struct object *obj, unsigned int n)\n {\n \tunsigned int hash = *(unsigned int *)obj->sha1;\n \treturn hash % n;\n }\n \n-static void *insert_decoration(struct decoration *n, struct object *base, void *decoration)\n+static void *insert_decoration(struct decoration *n, const struct object *base, void *decoration)\n {\n \tint size = n->size;\n \tstruct object_decoration *hash = n->hash;\n@@ -44,7 +44,7 @@ static void grow_decoration(struct decoration *n)\n \tn->nr = 0;\n \n \tfor (i = 0; i < old_size; i++) {\n-\t\tstruct object *base = old_hash[i].base;\n+\t\tconst struct object *base = old_hash[i].base;\n \t\tvoid *decoration = old_hash[i].decoration;\n \n \t\tif (!base)\n@@ -55,7 +55,8 @@ static void grow_decoration(struct decoration *n)\n }\n \n /* Add a decoration pointer, return any old one */\n-void *add_decoration(struct decoration *n, struct object *obj, void *decoration)\n+void *add_decoration(struct decoration *n, const struct object *obj,\n+\t\tvoid *decoration)\n {\n \tint nr = n->nr + 1;\n \n@@ -65,7 +66,7 @@ void *add_decoration(struct decoration *n, struct object *obj, void *decoration)\n }\n \n /* Lookup a decoration pointer */\n-void *lookup_decoration(struct decoration *n, struct object *obj)\n+void *lookup_decoration(struct decoration *n, const struct object *obj)\n {\n \tint j;\n \ndiff --git a/decorate.h b/decorate.h\nindex 1fa4ad9..e732804 100644\n--- a/decorate.h\n+++ b/decorate.h\n@@ -2,7 +2,7 @@\n #define DECORATE_H\n \n struct object_decoration {\n-\tstruct object *base;\n+\tconst struct object *base;\n \tvoid *decoration;\n };\n \n@@ -12,7 +12,7 @@ struct decoration {\n \tstruct object_decoration *hash;\n };\n \n-extern void *add_decoration(struct decoration *n, struct object *obj, void *decoration);\n-extern void *lookup_decoration(struct decoration *n, struct object *obj);\n+extern void *add_decoration(struct decoration *n, const struct object *obj, void *decoration);\n+extern void *lookup_decoration(struct decoration *n, const struct object *obj);\n \n #endif\n-- \n1.6.0.90.g00a5c.dirty\n"},{"id":"87836","messageId":"20080820180034.GB32005@sigill.intra.peff.net","threadId":"15107","inReplyTo":"20080820175325.GD27773@sigill.intra.peff.net","subject":"[PATCH 2/2] allow '%d' pretty format specifier to show decoration","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-08-20T18:00:34Z","receivedAt":"2008-08-20T18:00:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Previously, specifying\n\n git log --pretty=format:'%H %s' --decorate\n\nwould calculate decorations, but not show them. You can now\ndo:\n\n git log --pretty=format:'%H (%d) %s' --decorate\n\nto see them.\n\nSigned-off-by: Jeff King \n---\nThere is a lot of room for discussion here.\n\nFor example:\n\n - what should %d show? Right now it shows each decoration, split by\n commas. It doesn't show the enclosing parentheses automatically.\n\n Is this too strict? Should there be some way of pulling out\n individual decorations from the list, or specifying a different\n delimiter? If so, probably that should be part of a general\n improvement in the format expansion macro language.\n\n Is it too loose? Perhaps the enclosing parentheses should be\n automatic, so that %d expands to nothing if there is no decoration,\n or the whole thing otherwise. Right now you are stuck with empty ()\n if there is no decoration. Alternatively, we could support some kind\n of conditional expansion in the formatting language (but I don't\n know how crazy we want to get wit new formatting features).\n\n - should this turn on --decorate automatically? If you use '%d'\n without --decorate, you will just get no decorations. I think that\n makes sense, though, since that opens room for specifying other\n types of decorations (e.g., there could be a --decorate-tags that\n only looks at tags).\n\n Documentation/pretty-formats.txt | 1 +\n pretty.c | 15 +++++++++++++++\n 2 files changed, 16 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex c11d495..55a5954 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -116,6 +116,7 @@ The placeholders are:\n - '%cr': committer date, relative\n - '%ct': committer date, UNIX timestamp\n - '%ci': committer date, ISO 8601 format\n+- '%d': decoration (if you specified --decorate)\n - '%e': encoding\n - '%s': subject\n - '%b': body\ndiff --git a/pretty.c b/pretty.c\nindex 33ef34a..00f19e1 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -519,6 +519,21 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,\n \t\t\treturn 3;\n \t\t} else\n \t\t\treturn 0;\n+\tcase 'd':\n+\t\t{\n+\t\t\tstruct name_decoration *d;\n+\t\t\tconst char *prefix = \"\";\n+\t\t\td = lookup_decoration(&name_decoration,\n+\t\t\t\t\t&commit->object);\n+\t\t\twhile (d) {\n+\t\t\t\tstrbuf_addstr(sb, prefix);\n+\t\t\t\tprefix = \", \";\n+\t\t\t\tstrbuf_addstr(sb, d->name);\n+\t\t\t\td = d->next;\n+\t\t\t}\n+\t\t}\n+\t\treturn 1;\n+\n \t}\n \n \t/* these depend on the commit */\n-- \n1.6.0.90.g00a5c.dirty\n"},{"id":"87841","messageId":"7vwsibr0zz.fsf@gitster.siamese.dyndns.org","threadId":"15107","inReplyTo":"20080820180034.GB32005@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] allow '%d' pretty format specifier to show decoration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-20T18:32:16Z","receivedAt":"2008-08-20T18:32:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Is it too loose? Perhaps the enclosing parentheses should be\n> automatic,...\n\nI think so. The default output should primarily be for humans.\n\n> - should this turn on --decorate automatically?\n\nOh, absolutely. If you plan to have different kind of decorations in the\nfuture, then the logic can be \"if there is explicit decoration specified,\ndon't do anything, but if there isn't any, turn the default one on.\"\n\nAlso, if it doesn't already, \"--pretty=[t]format:\" should make sure parent\nrewriting is enabled when %p and %P is used. I didn't look.\n"},{"id":"87843","messageId":"alpine.DEB.1.00.0808202042260.24820@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"15107","inReplyTo":"20080820180034.GB32005@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] allow '%d' pretty format specifier to show decoration","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-08-20T18:43:29Z","receivedAt":"2008-08-20T18:43:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 20 Aug 2008, Jeff King wrote:\n\n> There is a lot of room for discussion here.\n\nIndeed. When I posted a similar patch, there was some discussion, too. \nBut no resolution, as it seemed nobody was really interested.\n\nCiao,\nDscho\n"},{"id":"87847","messageId":"7vljyrqzn0.fsf@gitster.siamese.dyndns.org","threadId":"15107","inReplyTo":"alpine.DEB.1.00.0808202042260.24820@pacific.mpi-cbg.de.mpi-cbg.de","subject":"Re: [PATCH 2/2] allow '%d' pretty format specifier to show decoration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-20T19:01:39Z","receivedAt":"2008-08-20T19:01:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Wed, 20 Aug 2008, Jeff King wrote:\n>\n>> There is a lot of room for discussion here.\n>\n> Indeed. When I posted a similar patch, there was some discussion, too. \n> But no resolution, as it seemed nobody was really interested.\n\nWhy being so negative? Got up on the wrong side of the bed?\n\nPeople drop off of a discussion for different reasons, and it is not\nnecessarily from total lack of interest (unless nobody responded to the\ninitial call-for-discussion, obviously). Perhaps there were more\ninteresting and/or pressing thing for them to think about than interaction\nbetween decorate and pretty=format at the same time.\n\nThis time around, the original requestor may be more motivated and/or have\ntime for this topic than you were motivated and/or you had time back then,\nand can carry the discussion forward. Let's see what happens.\n"},{"id":"87854","messageId":"alpine.LNX.1.10.0808202138340.4070@pollux","threadId":"15107","inReplyTo":"20080820180034.GB32005@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] allow '%d' pretty format specifier to show decoration","fromName":"Michael Dressel","fromEmail":"michaeltilodressel@t-online.de","sentAt":"2008-08-20T19:51:18Z","receivedAt":"2008-08-20T19:51:18Z","isPatch":true,"sender":{"key":"michaeltilodressel@t-online.de","avatar":null},"body":"\nOn Wed, 20 Aug 2008, Jeff King wrote:\n\n> Previously, specifying\n>\n> git log --pretty=format:'%H %s' --decorate\n>\n> would calculate decorations, but not show them. You can now\n> do:\n>\n> git log --pretty=format:'%H (%d) %s' --decorate\n>\n> to see them.\n>\n\nWow that was fast! Thanks for the help.\n\n\nFor those who care:\nI use it in a script to extract the log title of commits between certain\ntags. And to compile a simple log history of what has changed between\ntags. It was a bit more tricky than I initially thought it would be,\nbecause of merges. So what I do basically is to do a git log A..B\nwhere A and B are two of the tags I found using --decorate. My problem\nwas that my script got confused when I had braces in the log title. That was\nwhy I wanted to use format in the first place. I know there is a web\ninterface to git which probably does all that, but I wanted to compile an as \nsimple as possible text file.\n\nCheers,\nMichael\n"},{"id":"87853","messageId":"alpine.DEB.1.00.0808202153140.24820@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"15107","inReplyTo":"7vljyrqzn0.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] allow '%d' pretty format specifier to show decoration","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-08-20T19:53:54Z","receivedAt":"2008-08-20T19:53:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 20 Aug 2008, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > On Wed, 20 Aug 2008, Jeff King wrote:\n> >\n> >> There is a lot of room for discussion here.\n> >\n> > Indeed. When I posted a similar patch, there was some discussion, too. \n> > But no resolution, as it seemed nobody was really interested.\n> \n> Why being so negative? Got up on the wrong side of the bed?\n\nNo. I just forgot to give a pointer, so that the arguments brought back \nthen are not lost.\n\nCiao,\nDscho\n"},{"id":"87864","messageId":"7vzln7phe1.fsf@gitster.siamese.dyndns.org","threadId":"15107","inReplyTo":"alpine.LNX.1.10.0808202138340.4070@pollux","subject":"Re: [PATCH 2/2] allow '%d' pretty format specifier to show decoration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-20T20:21:10Z","receivedAt":"2008-08-20T20:21:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Dressel writes:\n\n> I use it in a script to extract the log title of commits between certain\n> tags.\n\n\"git shortlog\"?\n"},{"id":"87877","messageId":"alpine.LNX.1.10.0808202305230.4170@pollux","threadId":"15107","inReplyTo":"7vzln7phe1.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] allow '%d' pretty format specifier to show decoration","fromName":"Michael Dressel","fromEmail":"michaeltilodressel@t-online.de","sentAt":"2008-08-20T21:05:43Z","receivedAt":"2008-08-20T21:05:43Z","isPatch":true,"sender":{"key":"michaeltilodressel@t-online.de","avatar":null},"body":"\n\nOn Wed, 20 Aug 2008, Junio C Hamano wrote:\n\n> Michael Dressel writes:\n>\n>> I use it in a script to extract the log title of commits between certain\n>> tags.\n>\n> \"git shortlog\"?\n>\n\nI use my script to see all the changes (by log titles) between\nthe commit (prev) I ran the script the last time and the current commit (curr). Now there may be uninteresting tags and some commits may have more\nthan one tag. Merges made before tag B may introduce commits (say c1) that are \nmade before say tag A but with git log prev..curr I may get a list like this\n\ncurr (current commit)\n.\n.\nB (latest tag)\nA (previous tag)\nc1 (commit merged some time between A and B. \n.\n.\nprev (commit when script was run the last time)\n\n\nActually I use \"git rev-list prev..curr\" to get a list of commits between\nthe last time I ran the script and the current head of the branch. Then\nI need the git log --decorate only to get _all_ the tags assigned to a \ngiven commit. Git describe does not give all the tags, as discussed\nearlier. The resulting list of tags assigned to a commit is later\nsearched for the tag names I'm interested in. And then I use git log\nagain (git log A..B ) to extract the log titles between every two successive\ntags, at this point I may use git shortlog.\n\n\n\nCheers,\nMichael\n"},{"id":"88364","messageId":"20080821050223.GA3295@mithlond.arda.local","threadId":"15107","inReplyTo":"20080820180034.GB32005@sigill.intra.peff.net","subject":"\"log --pretty=format:\" language","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2008-08-21T05:02:23Z","receivedAt":"2008-08-21T05:02:23Z","isPatch":false,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"Jeff King wrote (2008-08-20 14:00 -0400):\n\n> There is a lot of room for discussion here.\n> \n> For example:\n> \n> - what should %d show? Right now it shows each decoration, split by\n> commas. It doesn't show the enclosing parentheses automatically.\n> \n> Is this too strict? Should there be some way of pulling out\n> individual decorations from the list, or specifying a different\n> delimiter? If so, probably that should be part of a general\n> improvement in the format expansion macro language.\n\nIf such \"general improvement\" takes place I'd like to point out (most \nlikely old news, but anyway) that %b can't be indented in practical \nsense. For example, the command\n\n git log --pretty=format:%x09%s%n%n%x09%b -1 3a634dc\n\nprints this:\n\n Add hints to revert documentation about other ways to undo changes\n\n Based on its name, people may read the 'git revert' documentation when\nthey want to undo local changes, especially people who have used other\nSCM's. 'git revert' may not be what they had in mind, but git\nprovides several other ways to undo changes to files. We can help\nthem by pointing them towards the git commands that do what they might\nwant to do.\n\n[...]\n"},{"id":"88376","messageId":"m33akus1tj.fsf@localhost.localdomain","threadId":"15107","inReplyTo":"20080821050223.GA3295@mithlond.arda.local","subject":"Re: \"log --pretty=format:\" language","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-08-24T18:30:38Z","receivedAt":"2008-08-24T18:30:38Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Teemu Likonen writes:\n\n> Jeff King wrote (2008-08-20 14:00 -0400):\n> \n> > There is a lot of room for discussion here.\n> > \n> > For example:\n> > \n> > - what should %d show? Right now it shows each decoration, split by\n> > commas. It doesn't show the enclosing parentheses automatically.\n> > \n> > Is this too strict? Should there be some way of pulling out\n> > individual decorations from the list, or specifying a different\n> > delimiter? If so, probably that should be part of a general\n> > improvement in the format expansion macro language.\n> \n> If such \"general improvement\" takes place I'd like to point out (most \n> likely old news, but anyway) that %b can't be indented in practical \n> sense. For example [...]\n\nWe can take a look how rpm handling of --queryformat option handles\nit. \n\nFirst, it uses %{NAME} notation instead of %X shorthand for writing\nsingle header (git-for-each-ref uses %(name) instead, so we might want\nto use %(...) instead of %{...}, or use both). It allows use of\nprintf(3) type formatters, which include field width and align, for\nexample \"%-30{NAME} %10{SIZE}\\n\".\n\nSecond, for displaying arrays (like list of files, or list of\ndependencies) or multi line output like package description it prints\neach item in the array, or each line in multi-line field within qeuare\nbrackets, e.g. \"[%-50{FILENAMES} %10{FILESIZES}\\n]\". If one want to\nrepeat single-valued field one should use %{=NAME} syntax (actually it\nsimply takes first line/first element of array), e.g. \n\"[%{=NAME}: %{FILENAMES}\\n]\"\n\nQueryformat minilanguage is more reach, see /usr/share/doc/rpm-*/queryformat\nor http://rpm5.org/docs/api/queryformat.html\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"89676","messageId":"20080903193615.GA31225@coredump.intra.peff.net","threadId":"15107","inReplyTo":"alpine.DEB.1.00.0808202042260.24820@pacific.mpi-cbg.de.mpi-cbg.de","subject":"Re: [PATCH 2/2] allow '%d' pretty format specifier to show decoration","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-09-03T19:36:15Z","receivedAt":"2008-09-03T19:36:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 20, 2008 at 08:43:29PM +0200, Johannes Schindelin wrote:\n\n> Indeed. When I posted a similar patch, there was some discussion, too. \n> But no resolution, as it seemed nobody was really interested.\n\nI know Junio called you negative, but I actually appreciate having you\npoint out related work (even if I don't get around to reading it for 2\nweeks!).\n\nIt is funny how similar our patches ended up, even though I didn't even\nrealize yours existed. It looks like the comments were not all that\ndifferent, either.\n\nTo summarize what was said then (for the benefit of Michael, who is\nmoving this forward):\n\n - Junio didn't like the expansion to include a space and enclosing\n parentheses, because then we don't know where to put the space.\n I agree with that. Though note that you will still end up with an\n extra space for a blank decoration.\n\n - René suggested a comma delimeter, which Michael's patch does.\n\n - René suggested a foobar2000-inspired expansion construct for dealing\n with the extra parentheses and space when no decoration exists. If\n we are going to expand the expansion language I think I would prefer\n something more like what show-ref uses (as Jakub suggested), but\n with \"tags\" expanded so that they can contain arbitrary data. So\n something like:\n\n %(decorate:delim=, :prefix= (:suffix=\\))\n\n or even:\n\n %(decorate:+ (%(decorate:delim=, ))\n\n to use the more shell-ish conditional.\n\n But I think it is not a problem to introduce '%d' _and_ make such\n improvements later.\n\n - René also suggested that he wanted a placeholder for git-describe\n output. Logically, this would also want %d. Maybe it is worth making\n this %decorate now to avoid confusion later.\n\n-Peff\n"}]}