{"thread":{"id":"37526","subject":"[PATCH] pretty-format: add append line-feed format specifier","startedAt":"2014-09-09T18:09:50Z","lastAt":"2014-09-12T16:36:56Z","messageCount":10,"participants":["Harry Jeffery","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"249083","messageId":"540F426E.6080908@exec64.co.uk","threadId":"37526","inReplyTo":null,"subject":"[PATCH] pretty-format: add append line-feed format specifier","fromName":"Harry Jeffery","fromEmail":"harry@exec64.co.uk","sentAt":"2014-09-09T18:09:50Z","receivedAt":"2014-09-09T18:09:50Z","isPatch":true,"sender":{"key":"harry@exec64.co.uk","avatar":"https://avatars.githubusercontent.com/u/808760?v=4"},"body":"Add a new format prefix `_` that causes a line-feed to be inserted\nimmediately after an expansion if the expansion expands to a non-empty\nstring. This is useful for when you would like a line for an expansion\nto be prepended, but only when the expansion expands to a non empty\nstring, such as inserting a '%_d' expansion before a commit to show any\nrefs pointing towards it.\n\nSigned-off-by: Harry Jeffery \n---\n Documentation/pretty-formats.txt | 4 ++++\n pretty.c | 10 ++++++++--\n 2 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/pretty-formats.txt \nb/Documentation/pretty-formats.txt\nindex 85d6353..842cd17 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -197,6 +197,10 @@ If you add a ` ` (space) after '%' of a \nplaceholder, a space\n is inserted immediately before the expansion if and only if the\n placeholder expands to a non-empty string.\n\n+If you add a `_` (underscore) after '%' of a placeholder, a line-feed\n+is inserted immediately after the expansion if and only if the\n+placeholder expands to a non-empty string.\n+\n * 'tformat:'\n +\n The 'tformat:' format works exactly like 'format:', except that it\ndiff --git a/pretty.c b/pretty.c\nindex 44b9f64..ddb930d 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1416,7 +1416,8 @@ static size_t format_commit_item(struct strbuf \n*sb, /* in UTF-8 */\n \t\tNO_MAGIC,\n \t\tADD_LF_BEFORE_NON_EMPTY,\n \t\tDEL_LF_BEFORE_EMPTY,\n-\t\tADD_SP_BEFORE_NON_EMPTY\n+\t\tADD_SP_BEFORE_NON_EMPTY,\n+\t\tADD_LF_AFTER_NON_EMPTY\n \t} magic = NO_MAGIC;\n\n \tswitch (placeholder[0]) {\n@@ -1429,6 +1430,9 @@ static size_t format_commit_item(struct strbuf \n*sb, /* in UTF-8 */\n \tcase ' ':\n \t\tmagic = ADD_SP_BEFORE_NON_EMPTY;\n \t\tbreak;\n+\tcase '_':\n+\t\tmagic = ADD_LF_AFTER_NON_EMPTY;\n+\t\tbreak;\n \tdefault:\n \t\tbreak;\n \t}\n@@ -1449,6 +1453,8 @@ static size_t format_commit_item(struct strbuf \n*sb, /* in UTF-8 */\n \t} else if (orig_len != sb->len) {\n \t\tif (magic == ADD_LF_BEFORE_NON_EMPTY)\n \t\t\tstrbuf_insert(sb, orig_len, \"\\n\", 1);\n+\t\telse if (magic == ADD_LF_AFTER_NON_EMPTY)\n+\t\t\tstrbuf_addch(sb, '\\n');\n \t\telse if (magic == ADD_SP_BEFORE_NON_EMPTY)\n \t\t\tstrbuf_insert(sb, orig_len, \" \", 1);\n \t}\n@@ -1460,7 +1466,7 @@ static size_t userformat_want_item(struct strbuf \n*sb, const char *placeholder,\n {\n \tstruct userformat_want *w = context;\n\n-\tif (*placeholder == '+' || *placeholder == '-' || *placeholder == ' ')\n+\tif (*placeholder == '+' || *placeholder == '-' || *placeholder == ' ' \n|| *placeholder == '_')\n \t\tplaceholder++;\n\n \tswitch (*placeholder) {\n-- \n2.1.0\n"},{"id":"249088","messageId":"xmqqmwa8k3lg.fsf@gitster.dls.corp.google.com","threadId":"37526","inReplyTo":"540F426E.6080908@exec64.co.uk","subject":"Re: [PATCH] pretty-format: add append line-feed format specifier","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-09T19:15:23Z","receivedAt":"2014-09-09T19:15:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Harry Jeffery writes:\n\n> Add a new format prefix `_` that causes a line-feed to be inserted\n> immediately after an expansion if the expansion expands to a non-empty\n> string. This is useful for when you would like a line for an expansion\n> to be prepended, but only when the expansion expands to a non empty\n> string, such as inserting a '%_d' expansion before a commit to show any\n> refs pointing towards it.\n\nIs this different from \"%n%-d\"?\n"},{"id":"249089","messageId":"540F554C.5010301@exec64.co.uk","threadId":"37526","inReplyTo":"xmqqmwa8k3lg.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] pretty-format: add append line-feed format specifier","fromName":"Harry Jeffery","fromEmail":"harry@exec64.co.uk","sentAt":"2014-09-09T19:30:20Z","receivedAt":"2014-09-09T19:30:20Z","isPatch":true,"sender":{"key":"harry@exec64.co.uk","avatar":"https://avatars.githubusercontent.com/u/808760?v=4"},"body":"On 09/09/14 20:15, Junio C Hamano wrote:\n> Is this different from \"%n%-d\"?\n>\n\nYes. \"%n%-d\" will place the newline before the expansion, not after.\n\nlog --decorate --pretty=format:\"%n%-d%h\\\\ %t\\\\ [%cn]\\\\ %s\"\n---\n\n (HEAD, upstream/master, master)85f0837 c29da1d [Junio C Hamano] Start \nthe post-2.1 cycle\n f655651 4027a43 [Junio C Hamano] Merge branch 'rs/strbuf-getcwd'\n 51eeaea 1f4970c [Junio C Hamano] Merge branch 'ta/pretty-parse-config'\n 4740891 8961621 [Junio C Hamano] Merge branch 'bc/archive-pax-header-mode'\n---\n\nlog --decorate --pretty=format:\"%_d%%h\\\\ %t\\\\ [%cn]\\\\ %s\"\n---\n (HEAD, upstream/master, master)\n85f0837 c29da1d [Junio C Hamano] Start the post-2.1 cycle\nf655651 4027a43 [Junio C Hamano] Merge branch 'rs/strbuf-getcwd'\n51eeaea 1f4970c [Junio C Hamano] Merge branch 'ta/pretty-parse-config'\n4740891 8961621 [Junio C Hamano] Merge branch 'bc/archive-pax-header-mode'\n---\n\nThe latter is the output I've been trying to accomplish, and as far as I \ncan tell, this patch is the only way to achieve it.\n\nWell, you can do \"%d%n\" but that will put a blank line before every \ncommit that doesn't have a ref.\n"},{"id":"249091","messageId":"xmqqegvkk2k3.fsf@gitster.dls.corp.google.com","threadId":"37526","inReplyTo":"540F554C.5010301@exec64.co.uk","subject":"Re: [PATCH] pretty-format: add append line-feed format specifier","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-09T19:37:48Z","receivedAt":"2014-09-09T19:37:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Harry Jeffery writes:\n\n> On 09/09/14 20:15, Junio C Hamano wrote:\n>> Is this different from \"%n%-d\"?\n>>\n>\n> Yes. \"%n%-d\" will place the newline before the expansion, not after.\n\nMaybe \"%[-+ ]\" needs to be rethought, instead of making things worse\nby turning it into \"%[-_+ ]\", as the next person who comes would\nwant to add space after the expansion and would need to find yet\nanother letter like you did with '_'.\n"},{"id":"249095","messageId":"20140909214520.GA13603@peff.net","threadId":"37526","inReplyTo":"xmqqegvkk2k3.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] pretty-format: add append line-feed format specifier","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-09-09T21:45:21Z","receivedAt":"2014-09-09T21:45:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 09, 2014 at 12:37:48PM -0700, Junio C Hamano wrote:\n\n> Harry Jeffery writes:\n> \n> > On 09/09/14 20:15, Junio C Hamano wrote:\n> >> Is this different from \"%n%-d\"?\n> >>\n> >\n> > Yes. \"%n%-d\" will place the newline before the expansion, not after.\n> \n> Maybe \"%[-+ ]\" needs to be rethought, instead of making things worse\n> by turning it into \"%[-_+ ]\", as the next person who comes would\n> want to add space after the expansion and would need to find yet\n> another letter like you did with '_'.\n\nYeah, that was my thought on reading the initial patch, too. Why limit\nourselves to newlines and spaces. I'd much rather have full conditional\nexpansion, like \"${foo:+prefix $foo suffix}\" in the shell.\n\nSomething like the patch below might work, but I didn't test it very\nthoroughly (and note the comments, which might need dealing with). Maybe\nit would make a sensible base for Harry to build on if he wants to\npursue this.\n\nWith it, you can do:\n\n git log --format='%h %s%if(%d,%n Decoration:%d)' origin\n\nto get:\n\n 85f0837 Start the post-2.1 cycle\n Decoration: (origin/master, origin/HEAD, github/foo)\n f655651 Merge branch 'rs/strbuf-getcwd'\n 51eeaea Merge branch 'ta/pretty-parse-config'\n 4740891 Merge branch 'bc/archive-pax-header-mode'\n 0e28161 Merge branch 'pr/remotes-in-hashmap'\n 44ceb79 Merge branch 'jk/pretty-empty-format'\n 56f214e Merge branch 'ta/config-set'\n e8e4ce7 Merge branch 'rs/init-no-duplicate-real-path'\n 1d8a6f6 Merge branch 'mm/config-edit-global'\n c518279 Merge branch 'jc/reopen-lock-file'\n 96db324 Merge git://github.com/git-l10n/git-po\n Decoration: (origin/maint)\n\nYou could also make \"%d\" more flexible with it. We unconditionally\ninclude the \" (...)\" wrapper when expanding it. But assuming we\nintroduced a \"%D\" that is _just_ the decoration names, you could do:\n\n %if(%D, (%D))\n\nto get the same effect with much more flexibility.\n\n---\ndiff --git a/pretty.c b/pretty.c\nindex fe34ddc..96cd512 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1398,6 +1398,42 @@ static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */\n \t\tADD_SP_BEFORE_NON_EMPTY\n \t} magic = NO_MAGIC;\n \n+\tif (starts_with(placeholder, \"if(\")) {\n+\t\tconst char *cond_beg, *cond_end;\n+\t\tconst char *data_beg, *data_end;\n+\t\tchar *buf;\n+\t\tstruct strbuf scratch = STRBUF_INIT;\n+\n+\t\t/* can't handle commas in conditions; allow backslash-escaping? */\n+\t\tcond_beg = cond_end = placeholder + 3;\n+\t\tfor (cond_end = cond_beg; *cond_end != ','; cond_end++)\n+\t\t\tif (!*cond_end)\n+\t\t\t\treturn 0;\n+\n+\t\t/* ditto for nested parentheses; backslash escaping? */\n+\t\tdata_beg = cond_end + 1;\n+\t\tfor (data_end = data_beg; *data_end != ')'; data_end++)\n+\t\t\tif (!*data_end)\n+\t\t\t\treturn 0;\n+\n+\t\t/*\n+\t\t * Should teach formatters to return size only without\n+\t\t * actually writing to scratch buffer?\n+\t\t */\n+\t\tbuf = xmemdupz(cond_beg, cond_end - cond_beg);\n+\t\tstrbuf_expand(&scratch, buf, format_commit_item, context);\n+\t\tfree(buf);\n+\n+\t\tif (scratch.len) {\n+\t\t\tbuf = xmemdupz(data_beg, data_end - data_beg);\n+\t\t\tstrbuf_expand(sb, buf, format_commit_item, context);\n+\t\t\tfree(buf);\n+\t\t}\n+\t\tstrbuf_release(&scratch);\n+\n+\t\treturn data_end - placeholder + 1;\n+\t}\n+\n \tswitch (placeholder[0]) {\n \tcase '-':\n \t\tmagic = DEL_LF_BEFORE_EMPTY;\n"},{"id":"249099","messageId":"540F7C70.4010909@exec64.co.uk","threadId":"37526","inReplyTo":"20140909214520.GA13603@peff.net","subject":"Re: [PATCH] pretty-format: add append line-feed format specifier","fromName":"Harry Jeffery","fromEmail":"harry@exec64.co.uk","sentAt":"2014-09-09T22:17:20Z","receivedAt":"2014-09-09T22:17:20Z","isPatch":true,"sender":{"key":"harry@exec64.co.uk","avatar":"https://avatars.githubusercontent.com/u/808760?v=4"},"body":"On 09/09/14 22:45, Jeff King wrote:\n> Yeah, that was my thought on reading the initial patch, too. Why limit\n> ourselves to newlines and spaces. I'd much rather have full conditional\n> expansion, like \"${foo:+prefix $foo suffix}\" in the shell.\n>\n> Something like the patch below might work, but I didn't test it very\n> thoroughly (and note the comments, which might need dealing with). Maybe\n> it would make a sensible base for Harry to build on if he wants to\n> pursue this.\n\nI definitely prefer your more general solution to my \nbare-minimum-to-scratch-itch patch. I'd certainly be willing to take \nyour patch and expand upon it (pun unintended) once Junio has weighed in \non your suggestions.\n\n> You could also make \"%d\" more flexible with it. We unconditionally\n> include the \" (...)\" wrapper when expanding it. But assuming we\n> introduced a \"%D\" that is _just_ the decoration names, you could do:\n>\n> %if(%D, (%D))\n>\n> to get the same effect with much more flexibility.\n\nRegardless of what happens with the conditional expansion I think it \nwould definitely be a useful addition to be able to print the decorators \nwithout the \" (...)\" wrapper. I think it's general enough that it'd \nwarrant its own separate patch rather than being part of a patch series \nfor the conditional expansion.\n"},{"id":"249103","messageId":"20140909223158.GD14029@peff.net","threadId":"37526","inReplyTo":"540F7C70.4010909@exec64.co.uk","subject":"Re: [PATCH] pretty-format: add append line-feed format specifier","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-09-09T22:31:59Z","receivedAt":"2014-09-09T22:31:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 09, 2014 at 11:17:20PM +0100, Harry Jeffery wrote:\n\n> I definitely prefer your more general solution to my\n> bare-minimum-to-scratch-itch patch. I'd certainly be willing to take your\n> patch and expand upon it (pun unintended) once Junio has weighed in on your\n> suggestions.\n\nThanks. I am always happy to see contributors willing to pick up and run\nwith ideas.\n\nIt is probably out-of-scope for what you want, but while we are talking\nabout %d, it may be worth considering whether there is something simple\nwe can do to make formatting list-like items more flexible. E.g., even\nwith \"%D\", you are stuck with the format \"foo, bar, baz\" for multiple\ndecorations. Some kind of \"%join(%d,; )\" might work to produce \"foo;\nbar; baz\" (or whatever you want). But that may also be crossing the line\ninto insanity, and we would be better to allow some Turing-complete\nembedded language like lua. For that matter, conditionals might be\ncrossing that insanity line, too.\n\n> Regardless of what happens with the conditional expansion I think it would\n> definitely be a useful addition to be able to print the decorators without\n> the \" (...)\" wrapper. I think it's general enough that it'd warrant its own\n> separate patch rather than being part of a patch series for the conditional\n> expansion.\n\nYeah, I agree it can be separate.\n\n-Peff\n"},{"id":"249139","messageId":"xmqqtx4fgzqe.fsf@gitster.dls.corp.google.com","threadId":"37526","inReplyTo":"20140909214520.GA13603@peff.net","subject":"Re: [PATCH] pretty-format: add append line-feed format specifier","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-10T17:19:21Z","receivedAt":"2014-09-10T17:19:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Something like the patch below might work, but I didn't test it very\n> thoroughly (and note the comments, which might need dealing with). Maybe\n> it would make a sensible base for Harry to build on if he wants to\n> pursue this.\n>\n> With it, you can do:\n>\n> git log --format='%h %s%if(%d,%n Decoration:%d)' origin\n> ...\n> You could also make \"%d\" more flexible with it. We unconditionally\n> include the \" (...)\" wrapper when expanding it. But assuming we\n> introduced a \"%D\" that is _just_ the decoration names, you could do:\n>\n> %if(%D, (%D))\n>\n> to get the same effect with much more flexibility.\n\nYup.\n\nI do not think we need to go overboard to support nesting and stuff,\nlet alone turing completeness ;-), especially when we are going to\ntest the condition part only for emptyness. Even with this simple\npatch, I sense that we are near a slipperly slope of wanting to add\n%unless(%d, ) or %ifelse(%d,%d, \\(undefined\\)), so I am not 100%\nconvinced yet.\n"},{"id":"249280","messageId":"20140912044901.GC15519@peff.net","threadId":"37526","inReplyTo":"xmqqtx4fgzqe.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] pretty-format: add append line-feed format specifier","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-09-12T04:49:01Z","receivedAt":"2014-09-12T04:49:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 10, 2014 at 10:19:21AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Something like the patch below might work, but I didn't test it very\n> > thoroughly (and note the comments, which might need dealing with). Maybe\n> > it would make a sensible base for Harry to build on if he wants to\n> > pursue this.\n> >\n> > With it, you can do:\n> >\n> > git log --format='%h %s%if(%d,%n Decoration:%d)' origin\n> > ...\n> > You could also make \"%d\" more flexible with it. We unconditionally\n> > include the \" (...)\" wrapper when expanding it. But assuming we\n> > introduced a \"%D\" that is _just_ the decoration names, you could do:\n> >\n> > %if(%D, (%D))\n> >\n> > to get the same effect with much more flexibility.\n> \n> Yup.\n> \n> I do not think we need to go overboard to support nesting and stuff,\n> let alone turing completeness ;-), especially when we are going to\n> test the condition part only for emptyness. Even with this simple\n> patch, I sense that we are near a slipperly slope of wanting to add\n> %unless(%d, ) or %ifelse(%d,%d, \\(undefined\\)), so I am not 100%\n> convinced yet.\n\nWhat compelled me is the fact that we already started down the slippery\nslope with %+ and friends. Providing conditionals but then only allowing\ncertain characters seems weirdly limiting. But I guess it is all part of\nthe same slope.\n\nI dunno. I wrote that original set of lua pretty-format patches to try\nto stop the insanity once and for all. But I realized that I do not\nactually want to do anything complicated with the output formats, and\n\"--oneline\" and a few simple \"--format\" calls usually are enough. And if\nI do want more, I pipe it into a perl script (typically using --format\nto make it simple to parse).\n\nWe could also provide the data in some standard structured format like\nJSON to make the \"pipe to your language of choice\" option easier on\npeople. But using custom --formats to do so is not that hard, and is way\nmore efficient than dumping all of the data.\n\n-Peff\n"},{"id":"249308","messageId":"xmqqtx4cbxsn.fsf@gitster.dls.corp.google.com","threadId":"37526","inReplyTo":"20140912044901.GC15519@peff.net","subject":"Re: [PATCH] pretty-format: add append line-feed format specifier","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-12T16:36:56Z","receivedAt":"2014-09-12T16:36:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I dunno. I wrote that original set of lua pretty-format patches to try\n> to stop the insanity once and for all. But I realized that I do not\n> actually want to do anything complicated with the output formats, and\n> \"--oneline\" and a few simple \"--format\" calls usually are enough.\n\nYeah, I share that exactly, and %+ and friends are meant to be on\nthis side of the line between \"a few simple\" and \"complicated\". I\nwas not sure which side %if() falls myself, and while I still am not\nsure, if I were asked to decide which today, I would probably say it\nis on the \"simple\" side.\n"}]}