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

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:53:08

Luc Pionchon [off-list ref] 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)?

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

From: Luc Pionchon <hidden>
Date: 2016-06-15 22:53:08

On Thu, Feb 23, 2012 at 21:53, Junio C Hamano [off-list ref] wrote:
Luc Pionchon [off-list ref] writes:
quoted
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.
quoted
+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?

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

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:35

On Thu, Feb 23, 2012 at 9:53 PM, Junio C Hamano [off-list ref] wrote:
Luc Pionchon [off-list ref] writes:
quoted
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"?
quoted
+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:
--- 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help