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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 16, 2014, 18:37 UTC
Message-ID: <xmqqegvb2yza.fsf@gitster.dls.corp.google.com>
In-Reply-To: <541875ef.6153c20a.4ef5.ffff8cbb@mx.google.com>

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

Show 5 quoted lines

> 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.

Show 21 quoted lines

> @@ -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;
	}

Show 15 quoted lines

> 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.