Re: [PATCH v3] pretty: add %D format specifier

[Junio C Hamano <gitster@pobox.com>]

Harry Jeffery <harry@exec64.co.uk> writes:

> Add a new format specifier, '%D' that is identical in behaviour to '%d',
> except that it does not include the ' (' prefix or ')' suffix provided
> by '%d'.
>
> Signed-off-by: Harry Jeffery <harry@exec64.co.uk>

Thanks.

> @@ -196,20 +198,20 @@ void format_decorations(struct strbuf *sb,
>  	decoration = lookup_decoration(&name_decoration, &commit->object);
>  	if (!decoration)
>  		return;
> -	prefix = " (";
> +	strbuf_addstr(sb, color_commit);
> +	strbuf_addstr(sb, prefix);
>  	while (decoration) {
> -		strbuf_addstr(sb, color_commit);
> -		strbuf_addstr(sb, prefix);
>  		strbuf_addstr(sb, decorate_get_color(use_color, decoration->type));
>  		if (decoration->type == DECORATION_REF_TAG)
>  			strbuf_addstr(sb, "tag: ");
>  		strbuf_addstr(sb, decoration->name);
>  		strbuf_addstr(sb, color_reset);
> -		prefix = ", ";
> +		strbuf_addstr(sb, color_commit);
> +		if (decoration->next)
> +			strbuf_addstr(sb, separator);
>  		decoration = decoration->next;
>  	}

I was kind of found of the nice trick to use a punctuation, which
first points at the prefix " (" and then later points at the
separator ", ", to allow the code that prefixes the punctuation
before showing a new item.  It is now lost.

We can restore it by doing something like this, though:

	if (!decoration)
        	return;
	while (decoration) {
		strbuf_addstr(sb, prefix);
                strbuf_addstr(sb, decoration->name);
                prefix = separator;
                decoration = decoration->next;
	}

> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
> index de0cc4a..38148c1 100755
> --- a/t/t4205-log-pretty-formats.sh
> +++ b/t/t4205-log-pretty-formats.sh
> @@ -457,4 +457,15 @@ EOF
>  	test_cmp expected actual1
>  '
>  
> +test_expect_success 'clean log decoration' '
> +	git log --no-walk --tags --pretty="%H %D" --decorate=full >actual &&
> +	cat <<EOF >expected &&
> +$head1 tag: refs/tags/tag2
> +$head2 tag: refs/tags/message-one
> +$old_head1 tag: refs/tags/message-two
> +EOF

You could indent the here-doc if you wanted to, like this:

	cat >expected <<-EOF &&
        $head1 tag: ...
	...
	EOF

and the end result may look easier on the eyes.

> +	sort actual >actual1 &&

Hmph.  I actually think the part that prepares the history makes
sure that the output order of the commits is predictable by using
test_commit and test_tick.  I see existing tests at the end (which
is a sign that they were added more recently than the rest of the
test script, and can indicate a careless addition) already has
"sort", but we shouldn't have to sort.

> +	test_cmp expected actual1
> +'

Thanks.