threads / patch / 29708

patchpretty: add '*' modifier to add LF after non-empty

Subject: [PATCH] pretty: add '*' modifier to add LF after non-empty

## tl;dr

4 messages between Feb 23, 2012 and Apr 14, 2012. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Luc Pionchon· Feb 23, 2012, 13:10 UTC · lore

Add the '*' modifier, similar to the '+' modifier, to add a line-feed after a non-empty placeholder.

Allow to print a head-line which content can be empty. For example for decorations:

    $ git log --graph --pretty=format:"%C(green)%*d %C(reset)%s"
                                                ^^^

    *  (HEAD, origin/master, origin/HEAD, master)
    |  Update draft release notes to 1.7.10
    *    Merge branch 'jc/maint-request-pull-for-tag'
    |\  
    | *  (origin/jc/maint-request-pull-for-tag)
    | |  request-pull: explicitly ask tags/$name to be pulled
    * |    Merge branch 'bl/gitweb-project-filter'
    |\ \  
    | * |  (origin/bl/gitweb-project-filter)
    | | |  gitweb: Make project search respect project_filter

Signed-off-by: Luc Pionchon <pionchon.luc@gmail.com>
---
 Documentation/pretty-formats.txt |    4 ++++
 pretty.c                         |    6 ++++++
 t/t6006-rev-list-format.sh       |   10 ++++++++++
 3 files changed, 20 insertions(+), 0 deletions(-)

Hi,

I now started to use git.  When formatting my 'log' output, I have been looking for a modifier to add a line feed after a (potentially empty) placeholder.  Git allows to add a line feed before, but not after a placeholder.  This is a small patch that adds the feature.  I hope it is useful to others.

Show changes to 3 files +20 −0

Documentation/pretty-formats.txt, pretty.c, t/t6006-rev-list-format.sh

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 880b6f2..9114d49 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -159,6 +159,10 @@ If you add a `{plus}` (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.
 
+If you add a `*` (asterisk sign) after '%' of a placeholder, a line-feed
+is inserted immediately after the expansion if and only if the
+placeholder expands to a non-empty string.
+
 If you add a `-` (minus sign) after '%' of a placeholder, line-feeds that
 immediately precede the expansion are deleted if and only if the
 placeholder expands to an empty string.
diff --git a/pretty.c b/pretty.c
index 8688b8f..5ebaf88 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1132,6 +1132,7 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 		NO_MAGIC,
 		ADD_LF_BEFORE_NON_EMPTY,
 		DEL_LF_BEFORE_EMPTY,
+		ADD_LF_AFTER_NON_EMPTY,
 		ADD_SP_BEFORE_NON_EMPTY
 	} magic = NO_MAGIC;
 
@@ -1142,6 +1143,9 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 	case '+':
 		magic = ADD_LF_BEFORE_NON_EMPTY;
 		break;
+	case '*':
+		magic = ADD_LF_AFTER_NON_EMPTY;
+		break;
 	case ' ':
 		magic = ADD_SP_BEFORE_NON_EMPTY;
 		break;
@@ -1162,6 +1166,8 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 	} else if (orig_len != sb->len) {
 		if (magic == ADD_LF_BEFORE_NON_EMPTY)
 			strbuf_insert(sb, orig_len, "\n", 1);
+		else if (magic == ADD_LF_AFTER_NON_EMPTY)
+			strbuf_addstr(sb, "\n");
 		else if (magic == ADD_SP_BEFORE_NON_EMPTY)
 			strbuf_insert(sb, orig_len, " ", 1);
 	}
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 4442790..c692061 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -208,6 +208,16 @@ test_expect_success 'add LF before non-empty (2)' '
 	grep "^$" actual
 '
 
+test_expect_success 'add LF after non-empty (1) (empty)' '
+	git show -s --pretty=format:"%*d%s%nfoo%n" HEAD^^ >actual &&
+	test $(wc -l <actual) = 2
+'
+
+test_expect_success 'add LF after non-empty (2) (non empty)' '
+	git show -s --pretty=format:"%*d%s%nfoo%n" HEAD >actual &&
+	test $(wc -l <actual) = 3
+'
+
 test_expect_success 'add SP before non-empty (1)' '
 	git show -s --pretty=format:"%s% bThanks" HEAD^^ >actual &&
 	test $(wc -w <actual) = 2

-- 
1.7.4.1

Junio C Hamano· Feb 23, 2012, 19:53 UTC · re: Luc Pionchon · lore

Re: [PATCH] pretty: add '*' modifier to add LF after non-empty

Luc Pionchon <pionchon.luc@gmail.com> writes:

> Add the '*' modifier, similar to the '+' modifier,
> to add a line-feed after a non-empty placeholder.

Hrm, I thought I designed the plus and minus fairly carefully so that nobody needs to add this later.

Wouldn't it be sufficient to write

        Foo%n%-d

that says "We usually have LF after Foo, and write %d after that, but we might not have anything interesting in %d at all, in which case we don't add that %n"?

> +test_expect_success 'add LF after non-empty (1) (empty)' '
> +	git show -s --pretty=format:"%*d%s%nfoo%n" HEAD^^ >actual &&

Shouldn't this be equivalent to "%n%-d%s%nfoo%n", which in turn is covered by one of the previous tests (del LF before empty)?

Luc Pionchon· Feb 24, 2012, 10:58 UTC · re: Junio C Hamano · lore

Re: [PATCH] pretty: add '*' modifier to add LF after non-empty

On Thu, Feb 23, 2012 at 21:53, Junio C Hamano <gitster@pobox.com> wrote:

Show 17 quoted lines

>
> Luc Pionchon <pionchon.luc@gmail.com> writes:
>
> > Add the '*' modifier, similar to the '+' modifier,
> > to add a line-feed after a non-empty placeholder.
>
> Hrm, I thought I designed the plus and minus fairly carefully so that
> nobody needs to add this later.
>
> Wouldn't it be sufficient to write
>
>        Foo%n%-d
>
>
> that says "We usually have LF after Foo, and write %d after that, but we
> might not have anything interesting in %d at all, in which case we don't
> add that %n"?

What I want is a LF after the non empty, rather than before. It seems to me that %n%-d is equivalent to %+d, or did I miss something?

See the 3 examples below, with %n%-d, %+d and %*d

$ ./git log -5 --graph --pretty=format:"%C(yellow bold)%n%-d %C(reset)%s"
*
|\   (HEAD, origin/master, origin/HEAD, master) Sync with 1.7.9.2
| *
| |  (v1.7.9.2, origin/maint) Git 1.7.9.2
| *  completion: use tabs for indentation
| *  completion: remove stale "to submit patches" documentation
* |  git-p4: the option to specify 'host' is -H, not -h

$ ./git log -5 --graph --pretty=format:"%C(yellow bold)%+d %C(reset)%s"
*
|\   (HEAD, origin/master, origin/HEAD, master) Sync with 1.7.9.2
| *
| |  (v1.7.9.2, origin/maint) Git 1.7.9.2
| *  completion: use tabs for indentation
| *  completion: remove stale "to submit patches" documentation
* |  git-p4: the option to specify 'host' is -H, not -h

$ ./git log -5 --graph --pretty=format:"%C(yellow bold)%*d %C(reset)%s"
*    (HEAD, origin/master, origin/HEAD, master)
|\   Sync with 1.7.9.2
| *  (v1.7.9.2, origin/maint)
| |  Git 1.7.9.2
| *  completion: use tabs for indentation
| *  completion: remove stale "to submit patches" documentation
* |  git-p4: the option to specify 'host' is -H, not -h

Also as a side note, I noticed that color is lost after new lines.

Show 6 quoted lines

>
> > +test_expect_success 'add LF after non-empty (1) (empty)' '
> > +     git show -s --pretty=format:"%*d%s%nfoo%n" HEAD^^ >actual &&
>
> Shouldn't this be equivalent to "%n%-d%s%nfoo%n", which in turn is covered
> by one of the previous tests (del LF before empty)?

I think the later is equivalent to %+d%s%nfoo%n, am I wrong?

Felipe Contreras· Apr 14, 2012, 20:01 UTC · re: Junio C Hamano · lore

Re: [PATCH] pretty: add '*' modifier to add LF after non-empty

On Thu, Feb 23, 2012 at 9:53 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 21 quoted lines

> Luc Pionchon <pionchon.luc@gmail.com> writes:
>
>> Add the '*' modifier, similar to the '+' modifier,
>> to add a line-feed after a non-empty placeholder.
>
> Hrm, I thought I designed the plus and minus fairly carefully so that
> nobody needs to add this later.
>
> Wouldn't it be sufficient to write
>
>        Foo%n%-d
>
> that says "We usually have LF after Foo, and write %d after that, but we
> might not have anything interesting in %d at all, in which case we don't
> add that %n"?
>
>> +test_expect_success 'add LF after non-empty (1) (empty)' '
>> +     git show -s --pretty=format:"%*d%s%nfoo%n" HEAD^^ >actual &&
>
> Shouldn't this be equivalent to "%n%-d%s%nfoo%n", which in turn is covered
> by one of the previous tests (del LF before empty)?

Nope.

Try this:

Show changes to t/t6006-rev-list-format.sh +19 −0

--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -208,6 +208,25 @@ test_expect_success 'add LF before non-empty (2)' '
        grep "^$" actual
 '

+test_expect_success 'add LF after non-empty (1) (empty)' '
+       git show -s --pretty=format:"%n%-d%s%nfoo%n" HEAD^^ >actual &&
+       cat >expect <<-EOF &&
+       added foo
+       foo
+       EOF
+       test_cmp expect actual
+'
+
+test_expect_success 'add LF after non-empty (2) (non empty)' '
+       git show -s --pretty=format:"%n%-d%s%nfoo%n" HEAD >actual &&
+       cat >expect <<-EOF &&
+        (HEAD, master)
+       Test printing of complex bodies
+       foo
+       EOF
+       test_cmp expect actual
+'
+
 test_expect_success 'add SP before non-empty (1)' '
        git show -s --pretty=format:"%s% bThanks" HEAD^^ >actual &&
        test $(wc -w <actual) = 2

Luc's patch makes these tests work.

I don't know if there's any way to have a format string that works
with the current code, but your suggestion doesn't seem to do the
trick.

Cheers.

-- 
Felipe Contreras