{"thread":{"id":"53009","subject":"Conditional newline in pretty format","startedAt":"2020-03-17T15:28:39Z","lastAt":"2020-03-17T17:56:00Z","messageCount":5,"participants":["Robert Dailey","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"393356","messageId":"CAHd499DiCi3FJb9qWJNBNKyVQg_zYMgJRuYcH_pOP3LnGwk5Tg@mail.gmail.com","threadId":"53009","inReplyTo":null,"subject":"Conditional newline in pretty format","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2020-03-17T15:28:27Z","receivedAt":"2020-03-17T15:28:39Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"I have the following alias:\n\n```\n[alias]\n    release-notes = log --no-merges --pretty=format:'* %s%n%n%w(100,2,2)%b'\n```\n\nWith a branch `topic` checked out, I run it like so:\n\n    $ git release-notes origin..\n\nThis gives me the \"release notes\" on my branch. The goal is to have it\nformatted as a bullet-point list in markdown format so I can just copy\n& paste this output into Github, Bitbucket, or other sites and have it\nalready set up for render correctly.\n\nIt works perfectly right now except for the case where `%b` is empty.\nIn that case, I just want one newline after `%s` instead of 2. Is\nthere a way to make my second `%n` conditional on `%b` having a value?\n"},{"id":"393357","messageId":"CAHd499B+ro+d0bGA+-Y1Qnfkc1vMzXCnBfZmtZv+CscUXim=wQ@mail.gmail.com","threadId":"53009","inReplyTo":"CAHd499DiCi3FJb9qWJNBNKyVQg_zYMgJRuYcH_pOP3LnGwk5Tg@mail.gmail.com","subject":"Re: Conditional newline in pretty format","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2020-03-17T15:37:13Z","receivedAt":"2020-03-17T15:37:30Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Tue, Mar 17, 2020 at 10:28 AM Robert Dailey <rcdailey.lists@gmail.com> wrote:\n>\n> I have the following alias:\n>\n> ```\n> [alias]\n>     release-notes = log --no-merges --pretty=format:'* %s%n%n%w(100,2,2)%b'\n> ```\n>\n> With a branch `topic` checked out, I run it like so:\n>\n>     $ git release-notes origin..\n>\n> This gives me the \"release notes\" on my branch. The goal is to have it\n> formatted as a bullet-point list in markdown format so I can just copy\n> & paste this output into Github, Bitbucket, or other sites and have it\n> already set up for render correctly.\n>\n> It works perfectly right now except for the case where `%b` is empty.\n> In that case, I just want one newline after `%s` instead of 2. Is\n> there a way to make my second `%n` conditional on `%b` having a value?\n\nApologies, I forgot to add a lot more detail. First, I have two\nattachments. The first one, `alias1.png`, shows what log output looks\nlike when I execute the command mentioned above. The red annotation\nbox in the image points out the superfluous newlines in the entry that\nhas no log body (only subject). This is the newline I'm trying to get\nrid of.\n\nAnother solution I tried is `%+b`, based on this documentation:\n\n> If you add a + (plus sign) after '%' of a placeholder, a line-feed is inserted\n> immediately before the expansion if and only if the placeholder expands\n> to a non-empty string.\n\nHowever, the output I get is not as expected. Instead of a newline\ncharacter, it appears to just insert a single space character. Observe\nthe result in attachment named `alias2.png`. I'm not sure if this is a\nbug or if I'm just doing it wrong. Again, any help is greatly\nappreciated.\n"},{"id":"393361","messageId":"20200317171853.GA6598@coredump.intra.peff.net","threadId":"53009","inReplyTo":"CAHd499B+ro+d0bGA+-Y1Qnfkc1vMzXCnBfZmtZv+CscUXim=wQ@mail.gmail.com","subject":"Re: Conditional newline in pretty format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-03-17T17:18:53Z","receivedAt":"2020-03-17T17:18:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 17, 2020 at 10:37:13AM -0500, Robert Dailey wrote:\n\n> > It works perfectly right now except for the case where `%b` is empty.\n> > In that case, I just want one newline after `%s` instead of 2. Is\n> > there a way to make my second `%n` conditional on `%b` having a value?\n> [...]\n> Another solution I tried is `%+b`, based on this documentation:\n\nThat's what I would have suggested. And it does seem to work if you do:\n\n  git log --format='* %s%n%+b'\n\nbut not when you add in the indentation and wrapping:\n\n  git log --format='* %s%n%w(100,2,2)%+b'\n\nWhich is unfortunate, but I think makes sense: the wrapping sees the\nextra newline as part of the text to be wrapped, so it gets folded into\nthe first line.\n\nI think what you really want is a conditional that can cover multiple\nplaceholders, and put the wrapped body inside that. You can do that with\nthe for-each-ref placeholders, which have a real \"%(if)...%(end)\" block.\nBut I don't think the pretty-format placeholders have an equivalent. It\nwould be nice to unify them one day, but progress has been slow on that\nfront.\n\nI wonder in the meantime if it would be possible to introduce a block\nsyntax to the pretty formats, like:\n\n  git log --format='* %s%n%+{%w(100,2,2)%b}'\n\nor something. I don't know the conditional code well enough to say\nwhether that would be a trivial patch or a horribly complicated one. :)\n\n-Peff\n"},{"id":"393362","messageId":"CAHd499AfYZth1oFii66iUMDJTKVaa-S1K37jt2V027EmYsGPkA@mail.gmail.com","threadId":"53009","inReplyTo":"20200317171853.GA6598@coredump.intra.peff.net","subject":"Re: Conditional newline in pretty format","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2020-03-17T17:27:00Z","receivedAt":"2020-03-17T17:27:17Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Tue, Mar 17, 2020 at 12:18 PM Jeff King <peff@peff.net> wrote:\n>\n> On Tue, Mar 17, 2020 at 10:37:13AM -0500, Robert Dailey wrote:\n>\n> > > It works perfectly right now except for the case where `%b` is empty.\n> > > In that case, I just want one newline after `%s` instead of 2. Is\n> > > there a way to make my second `%n` conditional on `%b` having a value?\n> > [...]\n> > Another solution I tried is `%+b`, based on this documentation:\n>\n> That's what I would have suggested. And it does seem to work if you do:\n>\n>   git log --format='* %s%n%+b'\n>\n> but not when you add in the indentation and wrapping:\n>\n>   git log --format='* %s%n%w(100,2,2)%+b'\n>\n> Which is unfortunate, but I think makes sense: the wrapping sees the\n> extra newline as part of the text to be wrapped, so it gets folded into\n> the first line.\n>\n> I think what you really want is a conditional that can cover multiple\n> placeholders, and put the wrapped body inside that. You can do that with\n> the for-each-ref placeholders, which have a real \"%(if)...%(end)\" block.\n> But I don't think the pretty-format placeholders have an equivalent. It\n> would be nice to unify them one day, but progress has been slow on that\n> front.\n>\n> I wonder in the meantime if it would be possible to introduce a block\n> syntax to the pretty formats, like:\n>\n>   git log --format='* %s%n%+{%w(100,2,2)%b}'\n>\n> or something. I don't know the conditional code well enough to say\n> whether that would be a trivial patch or a horribly complicated one. :)\n\nThanks for the information. It could also be that for something this\ncomplex, expecting Git to do it internally might be unreasonable. I'll\ntry to come up with a bash script to replace the alias. It'll be a lot\nmore verbose but I can take more of a \"string builder\" approach in an\nactual script which might be more intuitive. I just wanted to check\nfor any bugs/built-in behavior before I go that route.\n"},{"id":"393363","messageId":"20200317175558.GA16061@coredump.intra.peff.net","threadId":"53009","inReplyTo":"CAHd499AfYZth1oFii66iUMDJTKVaa-S1K37jt2V027EmYsGPkA@mail.gmail.com","subject":"Re: Conditional newline in pretty format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-03-17T17:55:58Z","receivedAt":"2020-03-17T17:56:00Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 17, 2020 at 12:27:00PM -0500, Robert Dailey wrote:\n\n> > I wonder in the meantime if it would be possible to introduce a block\n> > syntax to the pretty formats, like:\n> >\n> >   git log --format='* %s%n%+{%w(100,2,2)%b}'\n> >\n> > or something. I don't know the conditional code well enough to say\n> > whether that would be a trivial patch or a horribly complicated one. :)\n> \n> Thanks for the information. It could also be that for something this\n> complex, expecting Git to do it internally might be unreasonable. I'll\n> try to come up with a bash script to replace the alias. It'll be a lot\n> more verbose but I can take more of a \"string builder\" approach in an\n> actual script which might be more intuitive. I just wanted to check\n> for any bugs/built-in behavior before I go that route.\n\nYeah, I think it would be easy to do something like this with perl\n(possibly using Git's --format directives to make it easier to parse the\nindividual pieces). Once upon a time I had a hacky patch to let you\nformat with lua inside Git, which would have made this trivial. But it\nneeded quite a bit of polishing.\n\nAt any rate, here's a patch for %{}. It seems to work but I think it may\nbe too hacky within the current system. One thing in particular is that\n%w takes effect for the rest of the string, and we apply it\nretroactively at the end of the string. I think we'd want to \"push\" a\nnew context onto a stack and pop it at the end of the block. But the\ncurrent format_commit_context is a bit muddled; some of the things we'd\nwant to do this with (wrapping, padding, etc) but not others (parsed\nelements of the commit we've parsed).\n\nI didn't pursue it further because I think the right solution is doing\nan up-front parse of the format string into a true recursive parse tree,\nand then walking that tree to format each commit.\n\nBut in the meantime, you can hack around it by \"popping\" the wrap\nparameters manually at the end of the block:\n\n  git log --format='* %s%n%+{%w(100,2,2)%b%w(0,0,0)}'\n\n---\ndiff --git a/pretty.c b/pretty.c\nindex 28afc701b6..659fc4f3e9 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1146,6 +1146,9 @@ static int format_trailer_match_cb(const struct strbuf *key, void *ud)\n \treturn 0;\n }\n \n+static size_t format_commit_item(struct strbuf *, const char *placeholder,\n+\t\t\t\t void *context);\n+\n static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\t\t\tconst char *placeholder,\n \t\t\t\tvoid *context)\n@@ -1209,6 +1212,29 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \tcase '<':\n \tcase '>':\n \t\treturn parse_padding_placeholder(placeholder, c);\n+\n+\tcase '{':\n+\t\t{\n+\t\t\t/*\n+\t\t\t * A real recursive descent parser would allow embedded\n+\t\t\t * braces, or blocks within blocks. This hacky solution\n+\t\t\t * just finds a plausible ending brace, but it's\n+\t\t\t * probably the best we can do using strbuf_expand().\n+\t\t\t *\n+\t\t\t * The copy is also ugly and inefficient, since we do\n+\t\t\t * it for every commit. This would all be much nicer if\n+\t\t\t * we pre-parsed the format into a tree.\n+\t\t\t */\n+\t\t\tconst char *beg = placeholder + 1;\n+\t\t\tconst char *end = strchrnul(beg, '}');\n+\t\t\tchar *inner = xmemdupz(beg, end - beg);\n+\n+\t\t\tstrbuf_expand(sb, inner, format_commit_item, context);\n+\t\t\tfree(inner);\n+\n+\t\t\t/* consumed whole inner string plus maybe closing brace */\n+\t\t\treturn end - placeholder + !!*end;\n+\t\t  }\n \t}\n \n \t/* these depend on the commit */\n"}]}