Conditional newline in pretty format

Conditional newline in pretty format

[Robert Dailey]

I have the following alias:

```
[alias]
    release-notes = log --no-merges --pretty=format:'* %s%n%n%w(100,2,2)%b'
```

With a branch `topic` checked out, I run it like so:

    $ git release-notes origin..

This gives me the "release notes" on my branch. The goal is to have it
formatted as a bullet-point list in markdown format so I can just copy
& paste this output into Github, Bitbucket, or other sites and have it
already set up for render correctly.

It works perfectly right now except for the case where `%b` is empty.
In that case, I just want one newline after `%s` instead of 2. Is
there a way to make my second `%n` conditional on `%b` having a value?

Re: Conditional newline in pretty format

[Robert Dailey]

[-- Attachment #1: Type: text/plain, Size: 1667 bytes --]

On Tue, Mar 17, 2020 at 10:28 AM Robert Dailey <rcdailey.lists@gmail.com> wrote:
>
> I have the following alias:
>
> ```
> [alias]
>     release-notes = log --no-merges --pretty=format:'* %s%n%n%w(100,2,2)%b'
> ```
>
> With a branch `topic` checked out, I run it like so:
>
>     $ git release-notes origin..
>
> This gives me the "release notes" on my branch. The goal is to have it
> formatted as a bullet-point list in markdown format so I can just copy
> & paste this output into Github, Bitbucket, or other sites and have it
> already set up for render correctly.
>
> It works perfectly right now except for the case where `%b` is empty.
> In that case, I just want one newline after `%s` instead of 2. Is
> there a way to make my second `%n` conditional on `%b` having a value?

Apologies, I forgot to add a lot more detail. First, I have two
attachments. The first one, `alias1.png`, shows what log output looks
like when I execute the command mentioned above. The red annotation
box in the image points out the superfluous newlines in the entry that
has no log body (only subject). This is the newline I'm trying to get
rid of.

Another solution I tried is `%+b`, based on this documentation:

> If you add a + (plus sign) after '%' of a placeholder, a line-feed is inserted
> immediately before the expansion if and only if the placeholder expands
> to a non-empty string.

However, the output I get is not as expected. Instead of a newline
character, it appears to just insert a single space character. Observe
the result in attachment named `alias2.png`. I'm not sure if this is a
bug or if I'm just doing it wrong. Again, any help is greatly
appreciated.

[-- Attachment #2: alias2.png --]
[-- Type: image/png, Size: 14317 bytes --]

[-- Attachment #3: alias1.png --]
[-- Type: image/png, Size: 18423 bytes --]

Re: Conditional newline in pretty format

[Jeff King]

On Tue, Mar 17, 2020 at 10:37:13AM -0500, Robert Dailey wrote:

> > It works perfectly right now except for the case where `%b` is empty.
> > In that case, I just want one newline after `%s` instead of 2. Is
> > there a way to make my second `%n` conditional on `%b` having a value?
> [...]
> Another solution I tried is `%+b`, based on this documentation:

That's what I would have suggested. And it does seem to work if you do:

  git log --format='* %s%n%+b'

but not when you add in the indentation and wrapping:

  git log --format='* %s%n%w(100,2,2)%+b'

Which is unfortunate, but I think makes sense: the wrapping sees the
extra newline as part of the text to be wrapped, so it gets folded into
the first line.

I think what you really want is a conditional that can cover multiple
placeholders, and put the wrapped body inside that. You can do that with
the for-each-ref placeholders, which have a real "%(if)...%(end)" block.
But I don't think the pretty-format placeholders have an equivalent. It
would be nice to unify them one day, but progress has been slow on that
front.

I wonder in the meantime if it would be possible to introduce a block
syntax to the pretty formats, like:

  git log --format='* %s%n%+{%w(100,2,2)%b}'

or something. I don't know the conditional code well enough to say
whether that would be a trivial patch or a horribly complicated one. :)

-Peff

Re: Conditional newline in pretty format

[Robert Dailey]

On Tue, Mar 17, 2020 at 12:18 PM Jeff King <peff@peff.net> wrote:
>
> On Tue, Mar 17, 2020 at 10:37:13AM -0500, Robert Dailey wrote:
>
> > > It works perfectly right now except for the case where `%b` is empty.
> > > In that case, I just want one newline after `%s` instead of 2. Is
> > > there a way to make my second `%n` conditional on `%b` having a value?
> > [...]
> > Another solution I tried is `%+b`, based on this documentation:
>
> That's what I would have suggested. And it does seem to work if you do:
>
>   git log --format='* %s%n%+b'
>
> but not when you add in the indentation and wrapping:
>
>   git log --format='* %s%n%w(100,2,2)%+b'
>
> Which is unfortunate, but I think makes sense: the wrapping sees the
> extra newline as part of the text to be wrapped, so it gets folded into
> the first line.
>
> I think what you really want is a conditional that can cover multiple
> placeholders, and put the wrapped body inside that. You can do that with
> the for-each-ref placeholders, which have a real "%(if)...%(end)" block.
> But I don't think the pretty-format placeholders have an equivalent. It
> would be nice to unify them one day, but progress has been slow on that
> front.
>
> I wonder in the meantime if it would be possible to introduce a block
> syntax to the pretty formats, like:
>
>   git log --format='* %s%n%+{%w(100,2,2)%b}'
>
> or something. I don't know the conditional code well enough to say
> whether that would be a trivial patch or a horribly complicated one. :)

Thanks for the information. It could also be that for something this
complex, expecting Git to do it internally might be unreasonable. I'll
try to come up with a bash script to replace the alias. It'll be a lot
more verbose but I can take more of a "string builder" approach in an
actual script which might be more intuitive. I just wanted to check
for any bugs/built-in behavior before I go that route.

Re: Conditional newline in pretty format

[Jeff King]

On Tue, Mar 17, 2020 at 12:27:00PM -0500, Robert Dailey wrote:

> > I wonder in the meantime if it would be possible to introduce a block
> > syntax to the pretty formats, like:
> >
> >   git log --format='* %s%n%+{%w(100,2,2)%b}'
> >
> > or something. I don't know the conditional code well enough to say
> > whether that would be a trivial patch or a horribly complicated one. :)
> 
> Thanks for the information. It could also be that for something this
> complex, expecting Git to do it internally might be unreasonable. I'll
> try to come up with a bash script to replace the alias. It'll be a lot
> more verbose but I can take more of a "string builder" approach in an
> actual script which might be more intuitive. I just wanted to check
> for any bugs/built-in behavior before I go that route.

Yeah, I think it would be easy to do something like this with perl
(possibly using Git's --format directives to make it easier to parse the
individual pieces). Once upon a time I had a hacky patch to let you
format with lua inside Git, which would have made this trivial. But it
needed quite a bit of polishing.

At any rate, here's a patch for %{}. It seems to work but I think it may
be too hacky within the current system. One thing in particular is that
%w takes effect for the rest of the string, and we apply it
retroactively at the end of the string. I think we'd want to "push" a
new context onto a stack and pop it at the end of the block. But the
current format_commit_context is a bit muddled; some of the things we'd
want to do this with (wrapping, padding, etc) but not others (parsed
elements of the commit we've parsed).

I didn't pursue it further because I think the right solution is doing
an up-front parse of the format string into a true recursive parse tree,
and then walking that tree to format each commit.

But in the meantime, you can hack around it by "popping" the wrap
parameters manually at the end of the block:

  git log --format='* %s%n%+{%w(100,2,2)%b%w(0,0,0)}'

---
diff --git a/pretty.c b/pretty.c
index 28afc701b6..659fc4f3e9 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1146,6 +1146,9 @@ static int format_trailer_match_cb(const struct strbuf *key, void *ud)
 	return 0;
 }
 
+static size_t format_commit_item(struct strbuf *, const char *placeholder,
+				 void *context);
+
 static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 				const char *placeholder,
 				void *context)
@@ -1209,6 +1212,29 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 	case '<':
 	case '>':
 		return parse_padding_placeholder(placeholder, c);
+
+	case '{':
+		{
+			/*
+			 * A real recursive descent parser would allow embedded
+			 * braces, or blocks within blocks. This hacky solution
+			 * just finds a plausible ending brace, but it's
+			 * probably the best we can do using strbuf_expand().
+			 *
+			 * The copy is also ugly and inefficient, since we do
+			 * it for every commit. This would all be much nicer if
+			 * we pre-parsed the format into a tree.
+			 */
+			const char *beg = placeholder + 1;
+			const char *end = strchrnul(beg, '}');
+			char *inner = xmemdupz(beg, end - beg);
+
+			strbuf_expand(sb, inner, format_commit_item, context);
+			free(inner);
+
+			/* consumed whole inner string plus maybe closing brace */
+			return end - placeholder + !!*end;
+		  }
 	}
 
 	/* these depend on the commit */